jaimergp
commented
10 months ago One challenging aspect here might be how to migrate folks from existing installations.
Closed jaimergp closed 9 months ago
jaimergp
commented
10 months ago One challenging aspect here might be how to migrate folks from existing installations.
asmeurer
commented
10 months ago If this is not controlled, users might run into hard to diagnose bugs like
This is the sort of error you get from a long package path on Windows. The long prefix path errors look more like
ERROR:root:PaddingError: Placeholder of length '255' too short in package /private/var/folders/wc/dppcpmxs1tlb36nqcw853wkm0000gn/T/pytest-of-aaronmeurer/pytest-12/test_conda_store_register_envi0/conda-store-state/pytest-namespace/35f604188f69ceb5d9e3fae2c93ffd48c2971d192f21c776e3ebfed7e1196868-20231005-205956-556312-1-pytest-name/bin/bunzip2.(see https://github.com/conda-incubator/conda-store/issues/588#issuecomment-1751556643)
And actually, in this case at least, conda-store didn't even print this error message, it swallowed it. I had to dig into the code to extract the failing command and run it manually to get it. So really users might see no error. Meaning it's definitely a good idea to manually test this.
nkaretnikov
commented
10 months ago The only simple way to do it right now, even with a smaller hash, would be adding a runtime check to build_path. The main issue is that our prefix includes the namespace name and a bunch of other variable-lengths things in build_key. I think long-term we should move to a hash-based approach, which would be fixed-sized, see https://github.com/conda-incubator/conda-store/issues/611#issuecomment-1793552395.
For now, I'll just add a size check to build_path.
nkaretnikov
commented
9 months ago Status update: will be fixed by #653 and https://github.com/conda-incubator/conda-store-ui/pull/332, which are being reviewed.
Feature description
Comes from https://github.com/conda-incubator/conda-store/issues/588
condapackages are guaranteed to be relocatable as long as the environment prefix length is <=255 characters.Since we can have control over that, so I am suggesting we add checks to ensure that:
The 255 char value can also be adjusted down depending on the system, because we might need more wiggle room for Windows installations. So I'd say a
MAX_PREFIX_LENGTH=255should be default, but it can be adjusted to a value smaller than that.Value and/or benefit
If this is not controlled, users might run into hard to diagnose bugs like
, or buffer overflows, linking errors, etc.
It will also help palliating the problems with long paths in Windows, in general, where we might want to set an even lower threshold.
Anything else?
Extra explanation about relocatability:
conda packages are relocatable thanks to a neat trick: all packages are built under a 255-char long prefix. This long prefix acts as a "placeholder" that gets replaced at install-time so it matches the destination prefix.
For example: I have a binary that hardcodes (for whatever reason) a path to another executable it needs to do a certain operation. At build time, the long prefix will have something like
/opt/conda-build-workspace/project-1234abcd/envs/_h_env_placeholder_placeholder_placeholder_placeholder_placeholder_placeholder_placeholder_placeholder_placeholder_placeholder_placeholder_placeholder_placeholder_placeholder_placeholder_placeholder_placeho. At install time, after requestingconda create -p /opt/my_envs/hallo, it will be replaced with that path, and the remaining characters will be NULL, and everyone's happy.This trick is guaranteed to work 100% AS LONG AS the target prefix is under 255 characters. In some cases, longer paths might work because the environment doesn't contain binaries (text replacement is not as constrained) that need to be patched, but it's not guaranteed.