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

feat(nix): rework to work with all flake inputs #31921

Merged
merged 62 commits into from
Jan 31, 2025

Conversation

SuperSandro2000
Copy link
Contributor

@SuperSandro2000 SuperSandro2000 commented Oct 11, 2024

Changes

Before nix flakes where only updated if a github:nixos/nixpkgs input was used. Now all flake inputs are treated like a dependency and updated.

Context

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 October 11, 2024 16:52
@viceice viceice added the breaking Breaking change, requires major version bump label Oct 12, 2024
@SuperSandro2000
Copy link
Contributor Author

I am not sure if this is really breaking. Before updating flake locks just didn't really work and wasn't doing what people would expect.

@SuperSandro2000
Copy link
Contributor Author

❌ Error: More files have been added to '__fixtures__' directories than deleted.

I don't really have an idea how to test the lockfiles other than using fixtures.

@rarkins
Copy link
Collaborator

rarkins commented Oct 17, 2024

❌ Error: More files have been added to '__fixtures__' directories than deleted.

I don't really have an idea how to test the lockfiles other than using fixtures.

This is a valid use when lockfiles are long, we can ignore this test and merge anyway.

@SuperSandro2000
Copy link
Contributor Author

Can we finally get this merged?

The current flake support in renovate is practically none existing and doesn't align with any standard nix flake workflows.

* upstream/main: (73 commits)
  fix(cargo): Skip "replace" range upgrade if new version still matches (renovatebot#33773)
  feat(presets): Add nestjs/config to nest monorepo (renovatebot#33785)
  chore(deps): update python:3.13 docker digest to 6ee7975 (renovatebot#33781)
  chore(deps): update dependency @types/node to v22.10.7 (renovatebot#33780)
  feat: Drop older `timeStamp` field support (renovatebot#33734)
  feat: Switch to `Timestamp` type (renovatebot#33648)
  chore(deps): update github/codeql-action action to v3.28.3 (renovatebot#33767)
  build(deps): update dependency @renovatebot/osv-offline to v1.5.12 (renovatebot#33766)
  build(deps): update dependency fs-extra to v11.3.0 (renovatebot#33765)
  chore(deps): update codecov/codecov-action action to v5.2.0 (renovatebot#33763)
  build(deps): update dependency cronstrue to v2.53.0 (renovatebot#33762)
  docs: added list of valid namespace codes for the cacheTtlOverride config (renovatebot#33754)
  test(npm): use fs test utils in extract/pnpm.spec.ts (renovatebot#33756)
  docs: remove platform information from Mend-hosted App config page (renovatebot#33724)
  feat(presets): add monorepo html-eslint (renovatebot#33730)
  feat(presets): add arrow-kt monorepo (renovatebot#33740)
  build(deps): update dependency better-sqlite3 to v11.8.0 (renovatebot#33750)
  chore(deps): update dependency eslint-plugin-jest to v28.11.0 (renovatebot#33749)
  chore(deps): update dependency eslint-plugin-jest to v28.10.1 (renovatebot#33747)
  chore(deps): update otel/opentelemetry-collector-contrib docker tag to v0.118.0 (renovatebot#33746)
  ...
@h0nIg

This comment was marked as spam.

@viceice
Copy link
Member

viceice commented Jan 24, 2025

Please verify on forks of these repos that the new implementation works as intended:

The current implementation fails on the first repo.

@viceice
Copy link
Member

viceice commented Jan 24, 2025

you can easily find more repos for tersting https://github.com/search?q=nixpkgs.url+path%3Aflake.nix&type=code

@SuperSandro2000
Copy link
Contributor Author

The current implementation fails on the first repo.

It no longer bumps nixpkgs-21.11 to nixpkgs-24.11 as I already described multiple times before. I lack the typescript knowledge to implement support for both updating this and general lock file updates. After bumping that manual once SuperSandro2000/31921-nix-rework@6b394af this PR is created which is the intended feature SuperSandro2000/31921-nix-rework#1

@ojsef39
Copy link

ojsef39 commented Jan 25, 2025

you can easily find more repos for tersting https://github.com/search?q=nixpkgs.url+path%3Aflake.nix&type=code

Im adding mine to the list since my config is bit weird and gets all its inputs from a base config im not expecting it to work but would be cool:

https://github.com/ojsef39/nix-personal

@SuperSandro2000
Copy link
Contributor Author

Below tests where run with 5b12fed (#31921)

Im adding mine to the list since my config is bit weird and gets all its inputs from a base config im not expecting it to work but would be cool:

ojsef39/nix-personal

SuperSandro2000/nix-personal#1

renovate-reproductions/31921-nix-rework

https://github.com/SuperSandro2000/31921-nix-rework/pull/1/files

Simon-Boyer/nixos-config

That repo has no renovate config.

you can easily find more repos for tersting github.com/search?q=nixpkgs.url+path%3Aflake.nix&type=code

I have testing repos myself (NuschtOS/nixos-modules#130 NuschtOS/search#97) where I created PRs in the past and I have the latest version of this branched deployed to my renovate instance.

@elikoga
Copy link

elikoga commented Jan 27, 2025

The current implementation fails on the first repo.

It no longer bumps nixpkgs-21.11 to nixpkgs-24.11 as I already described multiple times before. I lack the typescript knowledge to implement support for both updating this and general lock file updates. After bumping that manual once SuperSandro2000/31921-nix-rework@6b394af this PR is created which is the intended feature SuperSandro2000/31921-nix-rework#1

I think there's a bit of confusion here. The existing manager is a nixpkgs manager that has domain-knowledge of github:nixos/nixpkgs release channels (structure and versioning scheme) and makes the potentially breaking change for you. Additionally (as a feature) it updates the flake lockfile so that this is a complete integration.

We additionally need a nix integration that updates flake.lock in general, which this PR tackles as far as I can see.

I believe that we need to preserve both behaviors and distinguish that these are two different managers.

@SuperSandro2000 is my assessment correct or am I missing something

@elikoga
Copy link

elikoga commented Jan 27, 2025

See this discussion and line change.

https://github.com/renovatebot/renovate/pull/31921/files#diff-ba688fc899a85e87e072cb5408f08fa6a579a14f608d75da4f711fc02505fa8aL12-R12

I think there is one obvious change. Change the docs to reflect this. Renovate has a nixpkgs manager, for the nix ecosystem. Renovate does not have a flake.lock manager. I think I'll try a PR sometime.

@SuperSandro2000
Copy link
Contributor Author

I believe that we need to preserve both behaviors and distinguish that these are two different managers.

In the current form the PR does both. I quickly added the old behaviour back after realizing that I can just append the list with the old logic.

I don't really want to split them in two managers at this point 😅

@h0nIg
Copy link

h0nIg commented Jan 30, 2025

i don't see a need to split the manager as well. With the old and new behaviour, the "breaking" label can get removed?

@elikoga
Copy link

elikoga commented Jan 30, 2025

In the current form the PR does both. I quickly added the old behaviour back after realizing that I can just append the list with the old logic.

Seems like I missed that. I'm looking forward to when it's merged then! 🤞

Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

thankyou for being patient and react on my concerns. so now we've a non breaking feature 🎉

@viceice viceice removed the breaking Breaking change, requires major version bump label Jan 31, 2025
@viceice viceice changed the title feat(nix)!: rework to work with all flake inputs feat(nix): rework to work with all flake inputs Jan 31, 2025
@viceice viceice added this pull request to the merge queue Jan 31, 2025
Merged via the queue into renovatebot:main with commit 441a30f Jan 31, 2025
39 checks passed
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 39.147.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

ivankatliarchuk added a commit to ivankatliarchuk/renovate that referenced this pull request Jan 31, 2025
* main:
  docs(versioning): fix types and tests (renovatebot#33958)
  feat(gitlab): add branch status check attemps (renovatebot#32692)
  feat(versioning): aws-eks-addon versioning added (renovatebot#33301)
  feat(nix): rework to work with all flake inputs (renovatebot#31921)
  fix(deps): update dependency mkdocs-material to v9.6.1 (renovatebot#33956)
  fix(deps): update dependency mkdocs-material to v9.6.0 (renovatebot#33955)
  fix(deps): update ghcr.io/renovatebot/base-image docker tag to v9.38.3 (renovatebot#33954)
  fix(deps): update ghcr.io/renovatebot/base-image docker tag to v9.38.2 (renovatebot#33953)
  feat(logger): Add `withMeta` helper (renovatebot#33948)
  feat(schema): Add logging utilities for catch calls (renovatebot#33950)
@SuperSandro2000 SuperSandro2000 deleted the rework-nix branch January 31, 2025 10:48
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.