-
-
Notifications
You must be signed in to change notification settings - Fork 37
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
Adapt distribution of etc/profile.d/mamba.sh
#283
base: main
Are you sure you want to change the base?
Adapt distribution of etc/profile.d/mamba.sh
#283
Conversation
c1ed21b
to
a771c6a
Compare
@conda-forge-admin, please rerender |
Hi! This is the friendly automated conda-forge-webservice. I tried to rerender for you, but it looks like there was nothing to do. This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/12596639646. Examine the logs at this URL for more detail. |
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 ( I do have some suggestions for making it better though... For recipe/meta.yaml:
This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/12634644016. Examine the logs at this URL for more detail. |
Hi! This is the friendly automated conda-forge-webservice. I tried to rerender for you, but it looks like there was nothing to do. This message was generated by GitHub Actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/12596662575. Examine the logs at this URL for more detail. |
Signed-off-by: Julien Jerphanion <[email protected]>
a771c6a
to
b81f865
Compare
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.
Hey @jjerphan, sorry that #282 was confusing. (What I put under "Installed packages" in that report were the commands that I used to achieve a broken environment with a mambaorg/micromamba
container.)
By the way, I downloaded the 2.0.5 mamba
packages from conda-forge
for Windows and Linux. The Linux version doesn't seem to include any activation scripts (presumably they are currently provided transitively by libmamba). In contrast, the Windows version of mamba
does include activation scripts in Scripts/
and condabin/
🤔.
Another thought: would it be possible for someone on Linux to end up with build 2 of libmamba
without the mamba.sh
included in libmamba
but with build 1 of mamba
that also excludes mamba.sh
? Answer: All good, this potential problem seems impossible because the pin on libmamba
seems to include the build number.
- test -d ${PREFIX}/include/mamba # [unix] | ||
- test -f ${PREFIX}/include/mamba/version.hpp # [unix] | ||
- test -f ${PREFIX}/lib/cmake/libmamba/libmambaConfig.cmake # [unix] | ||
- test -f ${PREFIX}/lib/cmake/libmamba/libmambaConfigVersion.cmake # [unix] | ||
- test -e ${PREFIX}/lib/libmamba${SHLIB_EXT} # [unix] | ||
- if exist %LIBRARY_PREFIX%\etc\profile.d\mamba.sh (exit 1) # [win] |
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.
Should be .bat
on Windows. Also suggesting to use the identical path from the mamba
section below.
- if exist %LIBRARY_PREFIX%\etc\profile.d\mamba.sh (exit 1) # [win] | |
- if exist %PREFIX%\condabin\mamba.bat (exit 1) # [win] |
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 tests that the file is also not present on Windows (this can be misread at first).
I think we could move this check at the top with the one for [unix]
with a comment giving the motivation and to remove the confusion.
# ============ | ||
|
||
-install(TARGETS ${mambaexe_targets}) | ||
+# The script are only needed for mamba and not for micromamba |
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.
Note that I'm only claiming is that the mamba.sh
/mamba.bat
activation script should not be included with libmamba
.
Probably your claim is correct, but it caught my eye. (It's been a long time since I've looked at the activation scripts for micromamba, and indeed the latest micromamba
package doesn't seem to include any activation scripts.)
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.
For reference, this change was added based on this test in micromamba-feedstock
independently from your remark.
Signed-off-by: Julien Jerphanion <[email protected]>
Checklist
0
(if the version changed)conda-smithy
(Use the phrase@conda-forge-admin, please rerender
in a comment in this PR for automated rerendering)Fix #282 with a patch of mamba-org/mamba#3723.