Conversation
|
@juliankuners @tothtamas28 Full disclosure, claude did write this. I have read over it and feel it makes sense but of course, easy to make a mistake |
|
I think that the snippets are inverted in the PR description |
| OVERRIDES_FILE = os.path.join(BaseDirectory.save_config_path('kup'), 'overrides.json') | ||
|
|
||
|
|
||
| def load_overrides() -> dict[str, list[list[str]]]: | ||
| """Load persisted override info for all packages.""" | ||
| if os.path.exists(OVERRIDES_FILE): |
There was a problem hiding this comment.
Please use pathlib instead. Also, pathlib allows directly reading and writing the file contents without with open().
| installed_packages: list[str] = [] | ||
| pinned_package_cache: dict[str, str] = {} | ||
|
|
||
| OVERRIDES_FILE = os.path.join(BaseDirectory.save_config_path('kup'), 'overrides.json') |
There was a problem hiding this comment.
This is now the second place where BaseDirectory.save_config_path('kup') is being used. Please make this an outer scope final variable that ensures that save_config_path is always called with the same directory name.
| """Load persisted override info for all packages.""" | ||
| if os.path.exists(OVERRIDES_FILE): | ||
| with open(OVERRIDES_FILE) as f: | ||
| return json.loads(f.read()) |
There was a problem hiding this comment.
Please use dataclasses or pydantic instead of json->dict parsing. Having dict[str, list[list[str]]] as a type, e.g., is non-verbose.
| inputs = get_package_metadata(listed_package) | ||
| rich.print(package_metadata_tree(inputs, show_status=show_status)) | ||
| pkg_overrides = load_overrides().get(package_name, []) | ||
| override_map = {ov[0]: ov[1] for ov in pkg_overrides} if pkg_overrides else {} |
There was a problem hiding this comment.
Please use a json struct instead of a json list for theses two values.
| all_overrides = load_overrides() | ||
| if all_overrides.pop(package_name, None) is not None: | ||
| save_overrides(all_overrides) |
There was a problem hiding this comment.
Packages can also be uninstalled from outside kup using nix. Therefore, this state can go out of sync. Instead, check whether a nix derivation hash matches the respective hash in the override state. This also needs to be added to the state.
| # Persist override info so `kup list --inputs` can display it | ||
| all_overrides = load_overrides() | ||
| if package_overrides: | ||
| all_overrides[package_name.base] = package_overrides |
There was a problem hiding this comment.
This saves the relative override path used during install in the override state, e.g., ../haskell-backend. However, the relative address requires a starting point to be useful.
This PR includes a note when a dependency is overridden in the input graph printed with
kup list <PKG> --inputsWithout override
With override