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

kubo-migrator: add migration from 15 to 16 #344265

Merged
merged 2 commits into from
Nov 21, 2024

Conversation

Luflosi
Copy link
Contributor

@Luflosi Luflosi commented Sep 24, 2024

Description of changes

The latest migration is required by the latest Kubo version 0.30.0.
I also rewrote the migration code:

  • Migrate to pkgs/by-name
  • Format with nixfmt-rfc-style
  • Make it possible to remove support for very old migrations in the future by increasing the minRepoVersion parameter
  • Rename kubo-migrator-all-fs-repo-migrations to kubo-fs-repo-migrations since it may no longer include all migrations
  • Add an alias for kubo-migrator-all-fs-repo-migrations to keep backwards compatibility
  • Update descriptions to differentiate between kubo-migrator and kubo-migrator-unwrapped and better describe the purpose of the migrator
  • Add a description to every individual migration
  • Add a description to kubo-fs-repo-migrations
  • Fetch the source code of the individual migrations from their specific Git tags, like upstream intends
  • Enable tests for some migrations
  • Check that the migrations don't crash on startup
  • Mark two broken migrations as broken. They are not compatible with the latest Go versions and upstream is not interested in fixing this
  • Change code to allow most updates to be done by only changing three lines (add new version and change git tag and hash)
  • Add a stub for any disabled or broken migration to prevent downloading unsigned binaries from the internet, see fs-repo-migrations is difficult to package ipfs/fs-repo-migrations#148 (comment) and Implement trustless and verified HTTP retrieval ipfs/fs-repo-migrations#188
  • Use lib.getExe instead of hardcoding the binary name in the kubo NixOS module
  • Use substituteInPlace with --replace-fail instead of --replace

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Sep 24, 2024
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Sep 24, 2024
@Luflosi Luflosi force-pushed the update/kubo-migrator branch from f1f481d to 26c6f53 Compare October 4, 2024 16:08
@Luflosi
Copy link
Contributor Author

Luflosi commented Oct 4, 2024

I decided to now also fetch the source of each migration from their respective git tag, as upstream intended.

@Luflosi Luflosi force-pushed the update/kubo-migrator branch 2 times, most recently from 6cc7f4e to 6f46b53 Compare October 4, 2024 16:17
@ofborg ofborg bot added 8.has: clean-up 8.has: package (new) This PR adds a new package and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Oct 4, 2024
@ofborg ofborg bot requested a review from elitak October 4, 2024 17:15
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Oct 4, 2024
@Luflosi Luflosi force-pushed the update/kubo-migrator branch from 6f46b53 to c47bb3a Compare October 26, 2024 21:36
@Luflosi
Copy link
Contributor Author

Luflosi commented Oct 26, 2024

I resolved the conflict.

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 10, 2024
@Mayeu
Copy link
Contributor

Mayeu commented Nov 13, 2024

Beside a now needed rebase, what is needed for this to be merged to the repo?

Kubo is currently releasing a RC for the v0.32 and this PR is needed since the PR for upgrading to v0.30 has been open (more than 1 month ago now).

@Luflosi I guess you don't have merge right for this?
@elitak do you have merge right? If so, what's needed for this PR to be merged?

@gileri
Copy link
Contributor

gileri commented Nov 18, 2024

Tested to migrate from repo version 15 to 16, works fine, thanks !
Code looks good, but I'm still new, a second look would do well.

Trivial rebase still needed.

- Migrate to pkgs/by-name
- Format with nixfmt-rfc-style
- Make it possible to remove support for very old migrations in the future by increasing the `minRepoVersion` parameter
- Rename kubo-migrator-all-fs-repo-migrations to kubo-fs-repo-migrations since it may no longer include all migrations
- Add an alias for kubo-migrator-all-fs-repo-migrations to keep backwards compatibility
- Update descriptions to differentiate between kubo-migrator and kubo-migrator-unwrapped and better describe the purpose of the migrator
- Add a description to every individual migration
- Add a description to kubo-fs-repo-migrations
- Fetch the source code of the individual migrations from their specific Git tags, like upstream intends
- Enable tests for some migrations
- Check that the migrations don't crash on startup
- Mark two broken migrations as broken. They are not compatible with the latest Go versions and upstream is not interested in fixing this
- Change code to allow most updates to be done by only changing three lines (add new version and change git tag and hash)
- Add a stub for any disabled or broken migration to prevent downloading unsigned binaries from the internet, see ipfs/fs-repo-migrations#148 (comment) and ipfs/fs-repo-migrations#188
- Use `lib.getExe` instead of hardcoding the binary name in the kubo NixOS module
- Use `substituteInPlace` with `--replace-fail` instead of `--replace`
@Luflosi Luflosi force-pushed the update/kubo-migrator branch from c47bb3a to ca26219 Compare November 20, 2024 15:17
@Luflosi
Copy link
Contributor Author

Luflosi commented Nov 20, 2024

Beside a now needed rebase, what is needed for this to be merged to the repo?

I rebased. This should be ready for merging.

I guess you don't have merge right for this?

No.

Code looks good, but I'm still new, a second look would do well.

Thanks.

@Mayeu
Copy link
Contributor

Mayeu commented Nov 21, 2024

I guess you don't have merge right for this?

No.

Then I guess it's time to poke random people to find somebody with write access? :/

@wegank: I see that you are a member of the NixOS org, would you be able to merge this PR?

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 21, 2024
@wegank wegank merged commit 02287a8 into NixOS:master Nov 21, 2024
28 of 29 checks passed
@Mayeu
Copy link
Contributor

Mayeu commented Nov 21, 2024

Thank you @wegank 🙏

@Luflosi Luflosi deleted the update/kubo-migrator branch November 21, 2024 14:20
@Luflosi
Copy link
Contributor Author

Luflosi commented Nov 21, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: clean-up 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants