Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

try to document c_stdlib_version in knowledge base #2206

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

minrk
Copy link
Member

@minrk minrk commented Jun 19, 2024

I tried to write down my understanding of the consequences of using a newer c_stdlib_version than the default, since this seems to be tripping up some folks (myself included). Please correct anything that's not right.

The gist I tried to get across:

  • c_stdlib_version is not a transitive dependency, but you need to make sure $LDFLAGS are respected. Mention -Wl,--allow-shlib-undefined by name.
  • old c_stdlib_version 2.12 is still the default both within conda-forge and for users at runtime. c-compiler/gcc_linux-64 set $LDFLAGS on activation, not gcc, which is relevant for user compilation outside conda-build.

Related issues:

@minrk minrk requested a review from a team as a code owner June 19, 2024 08:05
Copy link

netlify bot commented Jun 19, 2024

Deploy Preview for conda-forge-previews ready!

Name Link
🔨 Latest commit c05bab6
🔍 Latest deploy log https://app.netlify.com/sites/conda-forge-previews/deploys/66ab497ebad02600084879ef
😎 Deploy Preview https://deploy-preview-2206--conda-forge-previews.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@minrk
Copy link
Member Author

minrk commented Jun 19, 2024

@jaimergp
Copy link
Member

@h-vetinari if you have spare cycle, could you help review this? I'm afraid I lack the expertise 😬 Thanks! 🙏

@jaimergp jaimergp added the Docs label Jul 11, 2024
@h-vetinari
Copy link
Member

@h-vetinari if you have spare cycle, could you help review this? I'm afraid I lack the expertise 😬 Thanks! 🙏

I think this PR (and the docs generally) need an update now that cos7 has become the default. It's still reasonable to keep the content the same (modulo the exact name of the missing symbols), because the situation will be the same with 2.17 -> 2.28

the _runtime_ requirement on `__glibc` is applied
but it does not mean downstream dependencies will (or should) be built against the same version.
The default `c_stdlib_version` is still `2.12` on linux-64.
**A dependency on a newer `c_stdlib_version` does not mean dependents must also use that version**.
Copy link
Member

@h-vetinari h-vetinari Jul 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that's true in a practical sense (but best to double-check with @isuruf). We can ignore memcpy@GLIBC_2.14, but AFAIU that just means if the code-path requiring that symbol gets hit while compiling something else against some package, it'll crash.

To me, the issue boils down to the fact that we have two distinct ways to talk about the glibc version: __glibc (what's present on the system) and sysroot_<target> (what we're compiling against).

The {{ stdlib("c") }} metapackage on linux correctly generates the runtime constraint on __glibc, but does not constrain sysroot_<target>. IMO it should, though @isuruf was against this because basically that would mean that every compiled package on linux gets such a constraint.

"Morally", that'd be the right approach to me, but I don't know what issues could arise from having this constraint be so wide-spread.

The work-around (for not having this strong_constrains in our general infrastructure) would be to add the constraint to packages (like openmpi) that know their consumers will compile against them. So something like

      run_constrained:
        # prevent downstream packages from building with older sysroot
        - {{ c_stdlib }}_{{ target_platform }} >={{ c_stdlib_version }}  # [linux]

as in conda-forge/openmpi-feedstock#160

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me, the issue boils down to the fact that we have two distinct ways to talk about the glibc version: __glibc (what's present on the system) and sysroot_<target> (what we're compiling against).

This split between compiling and executing complicates the discussion in another way as well, because it's not just a question of the metadata, but also about what actually happens in a given feedstock build (i.e. if we're just compiling stuff requiring the newer glibc, or actually running something)

Let's say we've built libfoo that comes with a tool foo.exe (picture libprotobuf and protoc for example), and they now depend on glibc 2.28 in a world where our baseline is 2.17.

That means that:

Use case sys
root
glibc
in img.
Build Run
bar compiled
against libfoo
2.17 2.17 ✅ Compiles (with
-Wl,--allow-shlib-undefined)
❌ if bar calls symbol from
libfoo that needs newer glibc
bar compiled
against libfoo
2.28 2.17 ✅ Compiles ✅ symbols present due
to run-export from sysroot
bar built
using foo.exe
2.17 2.17 ❌ missing symbols for foo.exe -
bar built
using foo.exe
2.17 2.28 ✅ Builds ✅ (assuming bar does not
use anything from libfoo)

The ❌ under "Run" is not guaranteed, in the sense that it's conceivable that bar will not actually hit any of the paths from libfoo that require the newer symbols, but AFAIU we cannot rule this out.

If I made a mistake somewhere in the above, I'll happily stand corrected, I'm having a hard time wrapping my head around all the different scenarios myself 😅

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your work-around will not help in either of the failures above. The only solution is to use the alma8 image as the default which is analogous to using centos 7 image as the default when we had centos 6 as the baseline

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, there's another distinction between "run in feedstock" (e.g. for testing), and "run in end-user environment" (my table describes the latter). But compilation alone should work also on an older image, because the sysroot has the required libc?

In any case, I'm in favour of replicating the setup we've had (building for cos6 on cos7 images), only now with cos7 & alma8.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, there's another distinction between "run in feedstock" (e.g. for testing), and "run in end-user environment" (my table describes the latter). But compilation alone should work also on an older image, because the sysroot has the required libc?

Can you fix the table with what you mean? It doesn't make any sense to me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The table should look like the following IMO

Use case sys
root
glibc
in img.
Build Run
bar compiled
against libfoo
2.17 2.17 ❌ Cannot install libfoo -
bar compiled
against libfoo
2.28 2.17 ❌ Cannot install libfoo -
bar built
using foo.exe
2.17 2.17 ❌ Cannot install libfoo -
bar built
using foo.exe
2.17 2.28 ✅ Compiles (with
-Wl,--allow-shlib-undefined)
✅ Runs

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think the key point here is that libfoo having a requirement on newer glibc means libfoo won't install in the build environment if the available glibc is too old. isuruf's table looks right to me. Runtime errors are prevented by the glibc dependency, it's only the link issue that's coming up, I think.

I think it works just fine to have libfoo build against 2.28 and bar build against 2.17, though bar effectively has a requirement on 2.28, since it depends on libfoo which depends on glibc>=2.28, so there is no solution for bar with glibc<2.28 at runtime. That's I think the biggest argument to have run_constrained for sysroot: once a dependency builds with a given sysroot, there is no reason for downstream packages to keep building with older, since they can never be installed with a glibc older than the newest version supported by any given dependency anyway.

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

Successfully merging this pull request may close these issues.

4 participants