Sh and pkg #6
Sh and pkg #6
Conversation
marcoesters
left a comment
marcoesters
left a comment
There was a problem hiding this comment.
The big item to fix is the way falsy values are handled, which would result in a couple of false negatives.
constructor/preconda.py
Outdated
| # shortcuts | ||
| write_shortcuts_txt(info, env_pkgs, env_config) | ||
| # frozen marker file | ||
| if env_config.get("freeze_env"): |
There was a problem hiding this comment.
You could use the walrus operator here.
constructor/preconda.py
Outdated
|
|
||
|
|
||
| def write_frozen(freeze_info, dst_dir): | ||
| if freeze_info and "conda" in freeze_info: |
There was a problem hiding this comment.
This would result in you checking for freeze_info, once before the call and once during the call.
constructor/preconda.py
Outdated
| if freeze_info and "conda" in freeze_info: | ||
| frozen_path = join(dst_dir, "frozen") | ||
| with open(frozen_path, "w") as ff: | ||
| json.dump(freeze_info, ff, indent=2, sort_keys=True) |
There was a problem hiding this comment.
This will not result in the correct file. If conda is in freeze_info, the resulting file will at least be:
{
"conda": {}
}
Which is not the correct format. Also, why do we need to indent and sort keys? The file is not intended for human consumption.
constructor/shar.py
Outdated
| pre_t.addfile(tarinfo=tarfile.TarInfo("conda-meta/frozen")) | ||
| post_t.add(join(tmp_dir, "conda-meta", "frozen"), "conda-meta/frozen") |
There was a problem hiding this comment.
With these lines, you are adding an empty conda-meta/frozen to the preconda archive and the designated conda-meta/frozen file to the postconda archive. Did you use the same patterns as for the history file?
If so, history is an exception. It must exist so that conda recognizes that the environment it's installing packages into is a conda environment. That's why there is an empty file in preconda. However, installing the packages changes the history file. Since we are installing from an @EXPLICIT file, this can cause package conflicts down the line. The file packaged in postconda is the history file we need after the installation.
For the frozen file, we only need in one archive. I don't think it matters which one it is.
| and check_version(CONDA_EXE_VERSION, min_version="25.5.0", max_version="25.7.0") | ||
| ), | ||
| reason="conda-standalone 25.5.x fails with protected environments", | ||
| reason="conda-standalone 25.5.x fails with protected environments and older versions ignore frozen files", |
Jrice1317
force-pushed
the
sh_and_pkg
branch
from
1b40a9b to
5da5fba
Compare
constructor/shar.py
Outdated
| for env_name in info.get("_extra_envs_info", {}): | ||
| pre_t.addfile(tarinfo=tarfile.TarInfo(f"envs/{env_name}/conda-meta/history")) | ||
| post_t.add( | ||
| join(tmp_dir, "envs", env_name, "conda-meta", "history"), | ||
| f"envs/{env_name}/conda-meta/history", | ||
| ) | ||
| env_config = info["_extra_envs_info"][env_name] |
There was a problem hiding this comment.
| for env_name in info.get("_extra_envs_info", {}): | |
| pre_t.addfile(tarinfo=tarfile.TarInfo(f"envs/{env_name}/conda-meta/history")) | |
| post_t.add( | |
| join(tmp_dir, "envs", env_name, "conda-meta", "history"), | |
| f"envs/{env_name}/conda-meta/history", | |
| ) | |
| env_config = info["_extra_envs_info"][env_name] | |
| for env_name, env_config in info.get("_extra_envs_info", {}).items(): | |
| pre_t.addfile(tarinfo=tarfile.TarInfo(f"envs/{env_name}/conda-meta/history")) | |
| post_t.add( | |
| join(tmp_dir, "envs", env_name, "conda-meta", "history"), | |
| f"envs/{env_name}/conda-meta/history", | |
| ) |
If you use the value, you can just iterate over the items - it's a little cleaner.
There was a problem hiding this comment.
I thought about doing this too when I first decided to move my logic here.
There was a problem hiding this comment.
I ended up not doing this since we're checking for if the files exist instead of using the config logic
constructor/shar.py
Outdated
| pre_t.addfile(tarinfo=tarfile.TarInfo("conda-meta/history")) | ||
| post_t.add(join(tmp_dir, "conda-meta", "history"), "conda-meta/history") | ||
|
|
||
| if info.get("freeze_base"): |
There was a problem hiding this comment.
This isn't 100% sufficient since you also have to look for the conda sub key. I think the easiest way to verify would be to check if tmp_dir / conda-meta / frozen exists.
Description
Enable metadata for SH and PKG installers (INST-636)
Related tickets:
frozen_environmentsmetadata (INST-635)Checklist - did you ...
newsdirectory (using the template) for the next release's release notes?