-
-
Notifications
You must be signed in to change notification settings - Fork 281
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
minrk
wants to merge
2
commits into
conda-forge:main
Choose a base branch
from
minrk:try-doc-c-stdlib-version
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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) andsysroot_<target>
(what we're compiling against).The
{{ stdlib("c") }}
metapackage on linux correctly generates the runtime constraint on__glibc
, but does not constrainsysroot_<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 likeas in conda-forge/openmpi-feedstock#160
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 toolfoo.exe
(picturelibprotobuf
andprotoc
for example), and they now depend on glibc 2.28 in a world where our baseline is 2.17.That means that:
root
in img.
bar
compiledagainst
libfoo
-Wl,--allow-shlib-undefined
)bar
calls symbol fromlibfoo
that needs newer glibcbar
compiledagainst
libfoo
to run-export from sysroot
bar
builtusing
foo.exe
foo.exe
bar
builtusing
foo.exe
bar
does notuse 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 fromlibfoo
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 😅
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you fix the table with what you mean? It doesn't make any sense to me.
There was a problem hiding this comment.
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
root
in img.
bar
compiledagainst
libfoo
bar
compiledagainst
libfoo
bar
builtusing
foo.exe
bar
builtusing
foo.exe
-Wl,--allow-shlib-undefined
)There was a problem hiding this comment.
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.