-
Notifications
You must be signed in to change notification settings - Fork 418
fix: Resolve ca-certificates installed in the local environment #4101
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4101 +/- ##
==========================================
+ Coverage 63.57% 63.59% +0.01%
==========================================
Files 315 315
Lines 38562 38572 +10
Branches 2950 2951 +1
==========================================
+ Hits 24517 24529 +12
+ Misses 13976 13974 -2
Partials 69 69 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
0cde832 to
091170f
Compare
9193b95 to
97bac5d
Compare
Signed-off-by: Julien Jerphanion <[email protected]>
97bac5d to
6750848
Compare
| // `mamba` or `micromamba` is installed at: | ||
| // - `${PREFIX}/bin/{mamba,micromamba}` on Unix | ||
| // - `${PREFIX}/Library/bin/{mamba,micromamba}.exe` on Windows | ||
| const fs::u8path env_prefix |
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 whole block is extracted from get_root_prefix right?
mamba/libmamba/src/api/configuration.cpp
Lines 699 to 706 in 6f11ca2
| // Find the supposed environment prefix of libmamba. | |
| // `libmamba` is installed at: | |
| // - `${PREFIX}/lib/libmamba${SHLIB_EXT}` on Unix | |
| // - `${PREFIX}/Library/bin/libmamba$.dll` on Windows | |
| const fs::u8path libmamba_env_prefix = fs::weakly_canonical( | |
| util::on_win ? libmamba_path.parent_path().parent_path().parent_path() | |
| : libmamba_path.parent_path().parent_path() | |
| ); |
I would prefer if you factorize that in a function called here and in get_root_prefix.
Also, I think this is a bit fragile: is that supposed to work with micromamba which can be installed in any place (my micromamba never has ../bin as an example) and doesnt have a libmamba install?
Isnt there an env variable already for the env prefix? Or we really want the one from the executable here?
I see we later check the existence of this path so maybe it's not important and it's Ok with micromamba, just asking to be sure. :)
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.
Also, I think this is a bit fragile: is that supposed to work with micromamba which can be installed in any place (my micromamba never has
../binas an example) and doesnt have a libmamba install?
This is only meant to support a new use-case (see conda-forge/mamba-feedstock#362 (comment)).
Isnt there an env variable already for the env prefix? Or we really want the one from the executable here?
I do not think we can expect that an environment variable is set for the use-case given above.
Description
Toward resolving conda-forge/mamba-feedstock#362.
Type of Change
Checklist
pre-commit run --alllocally in the source folder and confirmed that there are no linter errors.