Skip to content

Quality: Lockfile entry is discarded for matching URLs due to inverted comparison#1106

Open
Nam0101 wants to merge 1 commit intonix-community:mainfrom
Nam0101:contribai/improve/quality/lockfile-entry-is-discarded-for-matching
Open

Quality: Lockfile entry is discarded for matching URLs due to inverted comparison#1106
Nam0101 wants to merge 1 commit intonix-community:mainfrom
Nam0101:contribai/improve/quality/lockfile-entry-is-discarded-for-matching

Conversation

@Nam0101
Copy link
Copy Markdown

@Nam0101 Nam0101 commented Mar 30, 2026

✨ Code Quality

Problem

Repo.__init__ only keeps locked_version when locked_version.url != url.geturl(). This is inverted: lock data is retained when URLs differ and dropped when they match. That can silently clear valid lock state and lead to unnecessary re-prefetch/re-evaluation or incorrect update behavior.

Severity: high
File: ci/nur/manifest.py

Solution

Flip the URL comparison so lock state is preserved only when it matches the repo definition: if ( locked_version is not None and locked_version.url == url.geturl() and locked_version.submodules == submodules ): self.locked_version = locked_version

Changes

  • ci/nur/manifest.py (modified)

The following points apply when adding a new repository to repos.json

  • I ran ./bin/nur format-manifest after updating repos.json (We will use the same script in github actions to make sure we keep the format consistent)
  • By including this repository in NUR, I confirm that any copyrightable content in the repository (other than built derivations or patches, if applicable) is licensed under the MIT license
  • I confirm that meta.license and meta.sourceProvenance have been set correctly for any derivations for unfree or not built from source packages

Additionally, the following points are recommended:

  • All applicable meta fields have been filled out. See https://nixos.org/manual/nixpkgs/stable/#sec-standard-meta-attributes for more information. The following fields are particularly helpful and can always be filled out:
    • meta.description, so consumers can confirm that that your package is what they're looking for
    • meta.license, even for free packages
    • meta.homepage, for tracking and deduplication
    • meta.mainProgram, so that nix run works correctly

Closes #1105

…ed comparison

`Repo.__init__` only keeps `locked_version` when `locked_version.url != url.geturl()`. This is inverted: lock data is retained when URLs differ and dropped when they match. That can silently clear valid lock state and lead to unnecessary re-prefetch/re-evaluation or incorrect update behavior.


Affected files: manifest.py

Signed-off-by: Nguyen Van Nam <nam.nv205106@gmail.com>
@Pandapip1
Copy link
Copy Markdown
Member

Correct me if I'm wrong, but I'm pretty sure the logic here is right. If we're passing a repository and the URL has changed, we want to copy over the new locked version.

@Nam0101
Copy link
Copy Markdown
Author

Nam0101 commented Mar 30, 2026

Thanks for taking a look at this.

I might be missing some intended behavior here, but from reading manifest.py, my understanding is that locked_version at this point is still the previous entry loaded from the lock file, rather than a newly resolved one.

If that's right, then the current condition seems a little counter-intuitive: it keeps the old lock when the URL changed, and drops it when the URL still matches. My gut feeling is that reusing the old lock only makes sense when it still points to the same source (url + submodules), and that carrying it over after a source change may be a bit risky.

So to me, != -> == still looks reasonble here, unless there's some other flow later on that refreshes locked_version before it gets written back.

Happy to be corrcted if I'm overlooking that part.

@Pandapip1
Copy link
Copy Markdown
Member

Sounds reasonable. Could you figure out what's causing the failed test?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: lockfile entry is discarded for matching urls due to inverted comparison

2 participants