Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(nix): fix flake lock refresh #33991

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

SuperSandro2000
Copy link
Contributor

@SuperSandro2000 SuperSandro2000 commented Feb 2, 2025

Changes

This PR fixes flake.lock refreshes which didn't fully work in #31921 because of a testing oversight:

DEBUG: currentDigest not found in string to replace (repository=NuschtOS/ixx, branch=renovate/flake-inputs)
       "stringToReplace": "nixos-unstable",
       "currentDigest": "5df43628fdf08d642be8ba5b3625a6c70731c19c"
DEBUG: Starting search at index 55 (repository=NuschtOS/ixx, packageFile=flake.nix, branch=renovate/flake-inputs)
       "depName": "nixpkgs"
DEBUG: Found match at index 55 (repository=NuschtOS/ixx, packageFile=flake.nix, branch=renovate/flake-inputs)
       "depName": "nixpkgs"
DEBUG: Digest is not updated (repository=NuschtOS/ixx, packageFile=flake.nix, branch=renovate/flake-inputs)
       "depName": "nixpkgs",
       "manager": "nix",
       "expectedValue": "9d3ae807ebd2981d593cddd0080856873139aa40",
       "foundValue": "5df43628fdf08d642be8ba5b3625a6c70731c19c"
 WARN: Error updating branch: update failure (repository=NuschtOS/ixx, branch=renovate/flake-inputs)
DEBUG: syncBranchState() (repository=NuschtOS/ixx, branch=renovate/crates)

I used this to generate NuschtOS/ixx#54

The underlying problem is that renovate checks for the deps digest in the fileMatch which is for nix flakes the flake.lock. We need to keep the flake.nix otherwise the nixpkgs updater breaks. currentValue needed to be dropped because otherwise renovate would only search in there.

@mfenniak do you have a better idea?

Context

#31921 (comment)

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

@SuperSandro2000 SuperSandro2000 marked this pull request as ready for review February 2, 2025 05:15
@@ -93,7 +100,6 @@ export async function extractPackageFile(
case 'github':
deps.push({
depName,
currentValue: flakeOriginal.ref,
Copy link
Contributor Author

@SuperSandro2000 SuperSandro2000 Feb 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find a way to tell the updater here to check against the flake.lock file instead of the matched flake.nix file

@SuperSandro2000 SuperSandro2000 force-pushed the fix-nix branch 4 times, most recently from 6e39464 to de0b6fb Compare February 2, 2025 06:35
@mfenniak
Copy link
Contributor

mfenniak commented Feb 2, 2025

I think this approach of matching on both the flake.nix and flake.lock file in the manager is a little ugly... the practical "problem" it will cause is that extractPackageFile is executed twice on a repo with both files, which you've aborted here, but not before it does work like reading the flake.lock file. That's pretty minor though. So as a minor code review, you could move packageLockFile & lockContents definitions to after the regex match.

I think one alternative approach would be something like this (main...mfenniak:renovate:fix-nix-alternative):

  • match on just flake.nix as previously done
  • if the regex match is hit and nixpkgs gets inserted into deps, then, skip depName == "nixpkgs" in the flake.lock loop

I think that would avoid the problem, if I've understood your change correctly -- there won't be any duplicate nixpkgs deps in this case -- and it might be a little easier to follow the code flow with this branch.

It would probably have a bug though: if nixpkgsRegex is matched on flake.nix, but it doesn't represent nixpkgs's input (could match a comment in the file, other content, or could match an input with another name eg. nixpkgs_24_04.input.url = "github:nixos/nixpkgs-24.04"), then... I'm not sure what the behavior would be actually. The depName and the actual dependency wouldn't match, and then the real nixpkgs dep (if present in the flake.lock) wouldn't be included... but I guess this is more of a latent bug with the existing regex match rather than a new problem. 🙁

@SuperSandro2000 SuperSandro2000 force-pushed the fix-nix branch 2 times, most recently from 2820cb4 to 6df6f7e Compare February 2, 2025 22:14
@SuperSandro2000
Copy link
Contributor Author

That didn't work out as expected :/

now other flake dependencies where detected twice
image

and somehow the artifactUpdate function is no longer run...

@SuperSandro2000
Copy link
Contributor Author

and somehow the artifactUpdate function is no longer run...

I think that was because I dropped the currentValue and somehow created ghost updates that didn't really where there...

Oh boy... :notlikethis:


I have deployed the latest commit as of writing. This "ghost" PR got marked as abandon. NuschtOS/ixx#55 . Before with the commit from yesterday with this branch it pushed the entire day nonsense. See that PRs history. This dashboard is created with it NuschtOS/ixx#3 , too.

Also NuschtOS/search.nuschtos.de#44

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.

2 participants