only set wrapper args for cross compilation, keeping subset of LDFLAGS necessary for sysroot < 2.17 #159
Conversation
defaults are not likely to be wrong for native compilation, don't mess with them and don't passthrough $LDFLAGS, etc., set the actual correct values from the wrappers with the right prefix
minrk
requested review from
astrofrog,
beckermr,
bekozi,
dalcinl,
leofang,
msarahan and
ocefpaf
as code owners
|
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 ( |
This comment was marked as outdated.
This comment was marked as outdated.
updates c_stdlib reliably
…nda-forge-pinning 2024.06.08.16.24.57
appears to be required for proper sysroot linkage on linux but these don't belong in the compiler wrappers themselves
fixes glibc lookup when building dependents with sysroot 2.12 avoids need for excessive flags in test env
| @@ -23,6 +24,9 @@ fi | |||
| build_with_ucx="" | |||
| if [[ "$target_platform" == linux-* ]]; then | |||
| build_with_ucx="--with-ucx=$PREFIX" | |||
| # allow-shlib-undefined required for dependencies to link against older sysroot | |||
| # avoids undefined | |||
| wrapper_ldflags="${wrapper_ldflags} -Wl,--allow-shlib-undefined" | |||
There was a problem hiding this comment.
This is actually the change needed to keep downstream packages from running into the glibc version, as verified by the fact that the tests still pass and still run with 3.12, and do not pass the conda-forge $LDFLAGS.
This change should actually fix #143
minrk
changed the title
Co-authored-by: jakirkham <[email protected]>
|
I'd really like to land this to unbreak things caused by #158. If folks are still uncomfortable with including the link flag from $LDFLAGS, I can remove it and isolate that to its own PR. |
bash is the only shell that will take these steps, but it won't be the only shell to process the file as a whole
|
I will merge this today unless there are objections, to prevent more conflicts like #162, since all questions raised so far have been addressed. To reiterate, this PR applies a strict subset of flags than the currently published build, it does not add any. It's just that one (the one that's necessary for building in conda-forge default environments) is now called out by name. |
minrk
changed the title
fixes #4474 #4496 and related PRs introduced a ceiling on `openmpi`, a dependency that's only pulled in at test time, because `cugraph`'s builds were struggling to find it. This proposes removing that pin, as the fixes in conda-forge/openmpi-feedstock#159 should allow the package to again be found by e.g. `find_package(MPI)` in CMake scripts. Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Chuck Hastings (https://github.com/ChuckHastings) - Ray Douglass (https://github.com/raydouglass) URL: #4496
defaults are not likely to be wrong for native compilation, don't mess with them, I think #158 went a bit too far.
and don't passthrough $LDFLAGS, etc. from compiler activation, set the actual correct values from the wrappers with the right prefix.
closes #163