[bug] Fix parseStorePathFromInstallableOutput #2098
Conversation
| @@ -0,0 +1,23 @@ | |||
| # Test installing a package without outputs in the store path. | |||
| # NOTE: Purposefully using a weird version to ensure it is not already in store. | |||
There was a problem hiding this comment.
once I run this testscript locally, it will then be in my store indefinitely (or until I run nix store gc) So, in that case the testscript won't be testing that its missing from local nix store.
so, the goal of this testscript is then to verify that one can do devbox install (equivalent) of "a package that doesn't have ooutputs in store path" ? Feels reasonable.
There was a problem hiding this comment.
yeah, this test mostly just tests that devbox install/run/shell work if the lockfile is missing store paths. I think it is useful (even though it would not catch this particular case where this function was removing store paths that were not installed)
The unit test you had originally (with valid: false) does catch it! I just misunderstood the API, and broke your test.
## Summary Addresses issue introduced in #2076 and #2098 Basically we have 2 different uses cases when looking up store paths. Sometimes we want all store paths (whether installed or not) and in other cases we only want paths that are in local store. This change fixes that and adds some testing. ## How was it tested? unit tests, manual integration test by installing a package that was not in local store.
Summary
Fix regression introduced in #2076
I misunderstood the API. A null value (in modern API) or a valid: false (in legacy API) occur when store path doesn't exist. Whether the store path itself is a valid store path doesn't matter, obviously if it is not a valid path, it won't be present in the store, but if it's valid and not installed, it will return null value or valid: false. The users of this function do not want to ignore these paths.
Edit: This bug is a bit less bad than initially thought. Because we install non-cached packages in flake, so anything missing would be installed then.
How was it tested?
Unit test.
This is tricky to test. I added a test script that can hopefully catch this.
Edit: This test doesn't actually catch this because the package ends up getting installed later on (in the flake). I still think this test is good to have.