-
Notifications
You must be signed in to change notification settings - Fork 0
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
Pkg 3792: initial feedstock, v12.3 #1
Conversation
We don't have arm-variant as a package in defaults. It's left in run_constrained to support compatibility when mixed with conda-forge packages.
Linter check found the following problems:ERROR conda.cli.main_run:execute(124): `conda run conda-lint /tmp/abs_877lnzy2z6/clone` failed. (See above for error) The following problems have been found:===== WARNINGS =====
===== ERRORS =====
|
Linter check found the following problems:ERROR conda.cli.main_run:execute(124): `conda run conda-lint /tmp/abs_6ezi4pxqi2/clone` failed. (See above for error) The following problems have been found:===== WARNINGS =====
===== ERRORS =====
|
…ired rpath functionality
Linter check found the following problems:ERROR conda.cli.main_run:execute(124): `conda run conda-lint /tmp/abs_b3m9falo3d/clone` failed. (See above for error) The following problems have been found:===== WARNINGS =====
===== ERRORS =====
|
Linter check found the following problems:ERROR conda.cli.main_run:execute(124): `conda run conda-lint /tmp/abs_4bk6mnet7b/clone` failed. (See above for error) The following problems have been found:===== WARNINGS =====
===== ERRORS =====
|
Linter check found the following problems:ERROR conda.cli.main_run:execute(124): `conda run conda-lint /tmp/abs_c7lix3469g/clone` failed. (See above for error) The following problems have been found:===== WARNINGS =====
===== ERRORS =====
|
Linter check found the following problems:ERROR conda.cli.main_run:execute(124): `conda run conda-lint /tmp/abs_36i7vje2b1/clone` failed. (See above for error) The following problems have been found:===== WARNINGS =====
===== ERRORS =====
|
Linter check found the following problems:ERROR conda.cli.main_run:execute(124): `conda run conda-lint /tmp/abs_f54ap9he2m/clone` failed. (See above for error) The following problems have been found:===== WARNINGS =====
===== ERRORS =====
|
recipe/build.sh
Outdated
# the runtime dependencies are packaged in their ecosystem. | ||
# For defaults, they're in the default system lib location. | ||
# if [[ $j =~ \.so\. ]]; then | ||
# patchelf --set-rpath '$ORIGIN' ${PREFIX}/${targetsDir}/$j |
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 think we should avoid diverging from conda-forge too much. Having $ORIGIN
in RUNPATH
doesn't hurt in this case. NVidia might actually have a reason to prefer RUNPATH
over RPATH
here (because conda-build will set RPATH
to $ORIGIN/.
automatically).
So if anything, we should wait to get an answer on conda-forge#21 before proceeding with this change.
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'll wait for the answer from conda-forge
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.
From conda-forge#21 (and conda-forge/cuda-feedstock#10), we need to keep this. We could create an issue on the cuda feedstock to suggest using --force-rpath
which would silence conda-build.
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. This would also change precedence of $ORIGIN
related to LD_LIBRARY_PATH
so not sure whether that makes sense (it might do).
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 is done
@@ -1,2 +1,5 @@ | |||
arm_variant_type: # [aarch64] | |||
- sbsa # [aarch64] | |||
conda_glibc_ver: | |||
- 2.17 # [not aarch64] |
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 feel like this is the kind of thing we should probably define in aggregate?
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.
it's defined in package cbcs for everything that uses it (various compiler related packages). But yeah I agree. Would prefer this not to be blocked on that though, and move them all out at some later opportunity. Concretely, I'd generate a jira ticket for it, and if needed an epic for cbc reviewing. Would you be ok with that?
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.
You know what, I'm even wondering we should shouldn't just do >=2.17
on both x86_64 and aarch64. If something is compiled with glibc 2.17, it will work on anything greater than that version. Like I don't know why they hard pin glibc there.
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.
Do you know why they specify compilers and sysroot in this feedstock anyway? It's just binary repackaging.
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 had this exact question with AnacondaRecipes/file-feedstock#1 yesterday and the only thing I can think of is that it's to make the DSO check happy.
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.
right yeah
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 asked that here
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 haven't had a reply and this is in line with other feedstocks (example). Would you be ok with keeping it in? Seems less risky if we don't know the reason. Would like to get the PR moving along. Don't think it's critical to unpin.
Linter check found the following problems:ERROR conda.cli.main_run:execute(124): `conda run conda-lint /tmp/abs_2208zcuso_/clone` failed. (See above for error) The following problems have been found:===== WARNINGS =====
===== ERRORS =====
|
Linter check found the following problems:ERROR conda.cli.main_run:execute(124): `conda run conda-lint /tmp/abs_5dtu9q68v1/clone` failed. (See above for error) The following problems have been found:===== WARNINGS =====
===== ERRORS =====
|
Linter check found the following problems:ERROR conda.cli.main_run:execute(124): `conda run conda-lint /tmp/abs_208sijomxy/clone` failed. (See above for error) The following problems have been found:===== WARNINGS =====
===== ERRORS =====
|
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.
LGTM, the rpath story is interesting
Looks like the |
@skupr-anaconda no, this feedstock is not python related. This feedstock is about cudart from the cudatoolkit. |
Linter check found the following problems:ERROR conda.cli.main_run:execute(124): `conda run conda-lint /tmp/abs_6afpp0omz8/clone` failed. (See above for error) The following problems have been found:===== WARNINGS =====
===== ERRORS =====
|
Linter check found the following problems:ERROR conda.cli.main_run:execute(124): `conda run conda-lint /tmp/abs_15jxi0_64s/clone` failed. (See above for error) The following problems have been found:===== WARNINGS =====
===== ERRORS =====
|
Destination channel: defaults
Links
Explanation of changes: