Skip to content

Conversation

@LourensVeen
Copy link
Contributor

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

Closes #193.

This implements the fix proposed in the discussion for #193, setting the present compiler version as the minimum and removing the upper bound.

I'd love a 4.x series build with this fix too, for my current project 5.x seems to be working fine, but I've run into issues with 5.x in another context. I guess I'm asking for #192 really, but that's going a bit beyond my conda-fu I fear.

@LourensVeen
Copy link
Contributor Author

@conda-forge-admin, please rerender

@conda-forge-admin
Copy link
Contributor

conda-forge-admin commented Apr 15, 2025

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe/meta.yaml) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe/meta.yaml:

  • ℹ️ The recipe is not parsable by parser conda-recipe-manager. The recipe can only be automatically migrated to the new v1 format if it is parseable by conda-recipe-manager.
  • ℹ️ The feedstock is lowering the image versions for one or more platforms: {'linux_64': 'cos7', 'linux_aarch64': 'cos7', 'linux_ppc64le': 'cos7'} (the default is alma9). Unless you are in the very rare case of repackaging binaryartifacts, consider removing these overrides from conda-forge.yml in the top feedstock directory.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/14471976960. Examine the logs at this URL for more detail.

@dalcinl
Copy link
Contributor

dalcinl commented Apr 15, 2025

@LourensVeen Any chance that something like {{ compiler('c').replace(' ', '>=', 1) }} would work?

EDIT: Maybe this way?

{{ compiler('c') | replace(' ', '>=', 1) }}

@LourensVeen
Copy link
Contributor Author

Can't hurt to try :)

@LourensVeen LourensVeen force-pushed the issue-193-compiler-version-dependency branch from 0089d69 to 70a0d36 Compare April 15, 2025 15:45
@conda-forge-admin
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I failed to even lint the recipe, probably because of a conda-smithy bug 😢. This likely indicates a problem in your meta.yaml, though. To get a traceback to help figure out what's going on, install conda-smithy and run conda smithy recipe-lint --conda-forge . from the recipe directory. You can also examine the workflow logs for more detail.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/14473713111. Examine the logs at this URL for more detail.

@LourensVeen LourensVeen force-pushed the issue-193-compiler-version-dependency branch from 70a0d36 to 354ec4d Compare April 15, 2025 15:48
@conda-forge-admin
Copy link
Contributor

conda-forge-admin commented Apr 15, 2025

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe/meta.yaml) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe/meta.yaml:

  • ℹ️ The recipe is not parsable by parser conda-recipe-manager. The recipe can only be automatically migrated to the new v1 format if it is parseable by conda-recipe-manager.
  • ℹ️ The feedstock is lowering the image versions for one or more platforms: {'linux_64': 'cos7', 'linux_aarch64': 'cos7', 'linux_ppc64le': 'cos7'} (the default is alma9). Unless you are in the very rare case of repackaging binaryartifacts, consider removing these overrides from conda-forge.yml in the top feedstock directory.

This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/14594495764. Examine the logs at this URL for more detail.

@LourensVeen
Copy link
Contributor Author

Looks like it's working now, except that one of the tests failed because a download failed.

Is there any way I could access the built packages to see if the dependency is now actually what we want?

@dalcinl
Copy link
Contributor

dalcinl commented Apr 15, 2025

You can store artifacts using azure/store_build_artifacts in conda-config.yml.
https://conda-forge.org/docs/maintainer/conda_forge_yml/#azure

@LourensVeen
Copy link
Contributor Author

LourensVeen commented Apr 16, 2025

@conda-forge-admin, please rerender

@LourensVeen
Copy link
Contributor Author

LourensVeen commented Apr 22, 2025

Okay, I have stored and downloaded the alien artifacts, then installed them locally with dependencies from conda-forge, and I've compiled and run C, C++ and Fortran test programs with gcc 11.4.0, 12.4.0, 13.3.0, and 14.2.0, which works fine at least on linux-64.

Just for fun, I also tried clang-20 and clangxx-20 with OMPI_CC and OMPI_CXX, and that works too in case someone wants it.

The dependencies now look like this:

openmpi-mpicc-5.0.7-hc43e4ee_101/info$ cat index.json 
{
  "arch": "x86_64",
  "build": "hc43e4ee_101",
  "build_number": 101,
  "depends": [
    "__glibc >=2.17,<3.0.a0",
    "gcc_linux-64 >=11.*",
    "openmpi 5.0.7 hb85ec53_101"
  ],
  "license": "BSD-3-Clause",
  "license_family": "BSD",
  "name": "openmpi-mpicc",
  "platform": "linux",
  "subdir": "linux-64",
  "timestamp": 1744797675375,
  "version": "5.0.7"
}

which is what we intended.

There are still a couple of build failures due to network timeouts, those seem to be fairly common. Do I keep retrying until it passes?

@LourensVeen
Copy link
Contributor Author

LourensVeen commented Apr 22, 2025

@conda-forge-admin, please restart ci

@dalcinl
Copy link
Contributor

dalcinl commented Apr 22, 2025

Before merging, I think we have to revert/remove 32aaadc.

EDIT: Actually, maybe just set it to "false", such that the key is there in the config file if we ever need to turn that thing on again.

@dalcinl
Copy link
Contributor

dalcinl commented Apr 22, 2025

@LourensVeen Are we all set? Should I merge once CI finishes?

@leofang
Copy link
Member

leofang commented Apr 22, 2025

@conda-forge-admin, please rerender

@LourensVeen
Copy link
Contributor Author

I think this is ready to go.

It's a bit scary because I'm not used to working on stuff that's this widely used, but it works as far as I can test it, passes all the built-in tests as well, and has a positive review, so let's go for it 😄

@leofang leofang merged commit 016c5bc into conda-forge:main Apr 22, 2025
11 checks passed
@minrk
Copy link
Member

minrk commented Apr 23, 2025

It's a bit scary because I'm not used to working on stuff that's this widely used

Not to worry, and thanks for your contribution! The compiler wrapper packages aren't particularly widely used, since they are really empty wrappers for two packages, e.g. mamba install openmpi c-compiler gets you the same thing as openmpi-mpicc. I'm not sure we should have added them in #35, but that was before the more convenient c-compiler packages were added (just one week later, I now realize), so users had to know their platform and type gcc_linux-64, etc. which isn't very nice. After adding c-compiler, I think the only reason to have these wrappers now would be:

  1. more or more restrictive dependencies to make sure the wrappers work
  2. some amount of activation to set e.g. CC=mpicc

but we currently don't do either of those, and I'm not sure we want to.

@LourensVeen
Copy link
Contributor Author

Ah, I never realised that the actual mpicc command was in the openmpi package, not openmpi-mpicc. I figured the idea was that a package could have a dependency on openmpi and only use openmpi-mpicc while building. And that I would need openmpi-mpicc to build anything. Oh well 😄.

@minrk
Copy link
Member

minrk commented Apr 23, 2025

It was a long time ago, but I think we were thinking of being able to use something like compiler("mpicc") or c_compiler = 'openmpi-mpicc' or some such to set $CC and friends for packages that build with the compiler wrappers, but that never materialized.

@LourensVeen
Copy link
Contributor Author

There's still an open issue (#110) about compiler('mpi') actually.

Feeding MPI support into a build system isn't as standardised as $CC though. The $AX_MPI macro for autoconf uses MPICC and CMake has its own different set of environment variables. Setting $CC may work if the build system is just a Makefile, but with something more complicated it could also mislead it into thinking there's no MPI. I've found just using mpicc, mpicxx and mpifort pretty reliable, I think that works across MPICH, OpenMPI, IMPI and Cray MPI.

For background, I'm working to get AMUSE into conda-forge. This is an astrophysics toolbox with some 60 codes in it that do a variety of calculations, written in C, C++, Fortran and CUDA, with MPI and OpenMP and OpenCL, and of varying vintage and quality, all wrapped in Python. In short, the perfect packaging problem 😄.

Of course none of it was designed with packaging or binary distribution in mind, but it's also old and crufty and hard to compile, making packaging more important. On the other hand, the codes aren't always written to be runtime configurable either, and the retrofit of that isn't always complete, so sometimes you want to modify the code, which can be tricky if, like many of the users, you don't really know what a compiler is.

So I'm trying to have the users set up a more or less standardised build environment using conda-forge which then also matches conda-forge's build environment closely, so that mixing binary packages from conda-forge with locally-built packages for some of the codes can work, and also so that any contributions from users are less likely to build the conda-forge builds. When I get around to submitting some feedstocks that is.

For some of these codes setting CC to the MPI compiler is just what's needed, but most of them need something more complicated, and I fear it's probably similar across the computational science/HPC domain...

@minrk
Copy link
Member

minrk commented Apr 23, 2025

Yeah, that's why I don't really think compiler("mpi") or setting CC=mpicc is really worth doing. There are too many ways to use mpi compiler wrappers, and it's far easier to do the right thing in a per-package way. openmpi has everything and there's no real need for any more packages.

The only real problem openmpi-mpicxx was meant to solve is that clangxx_osx-64 was hard for users to remember and type (they were never meant to be used in recipes, where this already isn't a problem), and cxx-compiler already solves the same problem without needing a new package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Compiler wrappers require gcc <= 12

5 participants