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

treewide: fix hard-coded sourceRoot prefix for fetchgit-based src #294334

Merged
merged 1 commit into from
Mar 11, 2024

Conversation

ShamrockLee
Copy link
Contributor

@ShamrockLee ShamrockLee commented Mar 8, 2024

Description of changes

According to Nixpkgs manual and NixOS 23.11 Release Note, the sourceRoot attribute passed to stdenv.mkDerivation should be specified as "${src.name}" or "${src.name}/subdir" when src is produced using fetchgit-based fetchers.

sourceRoot = "source" or sourceRoot = "source/subdir" is based on the assumption that the name attribute of these pre-unpacked fetchers are always "source", which is not the case. Expecting constant name also makes the source FODs prone to irrelevent hashes during version bumps.

This PR should cause no rebuild. As the issue occurs also on the stable branch, we could also backport this fix to the stable branch.

This is a continuation of #245388 and an initial effort for #294068.

Things done

  • Built on platform(s)
    • x86_64-linux (no rebuild)
    • 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.05 Release Notes (or backporting 23.05 and 23.11 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.

@ShamrockLee
Copy link
Contributor Author

Result of nixpkgs-review run on x86_64-linux 1

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Mar 8, 2024
According to Nixpkgs manual[1] and NixOS 23.11 Release Note[2], the
`sourceRoot` attribute passed to `stdenv.mkDerivation` should be
specified as `"${src.name}"` or `"${src.name}/subdir"` when `src` is
produced using `fetchgit`-based fetchers.

`sourceRoot = "source"` or `sourceRoot = "source/subdir"` is based on
the assumption that the `name` attribute of these pre-unpacked fetchers
are always `"source"`, which is not the case. Expecting constant `name`
also makes the source FODs prone to irrelevent hashes during version
bumps.

[1]: https://nixos.org/manual/nixpkgs/unstable/#var-stdenv-sourceRoot
[2]: https://nixos.org/manual/nixos/stable/release-notes#sec-release-23.11
Copy link
Contributor

@drupol drupol left a comment

Choose a reason for hiding this comment

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

Hi!

Just to stay on the safe side, could you please call @ofborg build with all the derivations that were updated?

Thx

@ShamrockLee
Copy link
Contributor Author

Just to stay on the safe side, could you please call @ofborg build with all the derivations that were updated?

Is there a way to programmatically get the list of packages? The nixpkgs-review output goes too large to copy and paste from the terminal.

If not, I'll just log the output of nixpkgs-review and select them out manually.

@drupol
Copy link
Contributor

drupol commented Mar 10, 2024

I don't know any way of automatizing this, I guess manually doing it is the only way, there not that much.

@ShamrockLee
Copy link
Contributor Author

@drupol I have confused this PR with #294068 in my previous reply. nixpkgs-review reports no rebuild in this PR, and great chance are that the build command will just download existing packages from Hydra. Anyway, I'll tag them by hand.

@ofborg build vimPlugins awesomebump geph localsend autodock-vina openrefine anilibria-winmaclinux c2fmzq frankenphp intiface-central lanzaboote-tool liana linien-gui mercure monophony rustdesk-flutter snippetexpanderx svix-server syn2mas torrentstream flutter coqPackages_8_16.vcfloat coqPackages_8_17.vcfloat mpfi opentelemetry-collector opentelemetry-collector-contrib opentelemetry-cpp python3Packages.bootstrap.flit-core python3Packages.daqp python3Packages.flit-core python3Packages.kanidm python3Packages.linien-client python3Packages.linien-common python3Packages.openllm-client python3Packages.openllm-core python3Packages.openllm python3Packages.pendulum_3 python3Packages.prophet sudachidict-core sudachidict-full sudachidict-small electron-source electron-bin gendef opensnitch tailor-gui openobserve SP800-90B_EntropyAssessment mpremote

@ShamrockLee
Copy link
Contributor Author

ShamrockLee commented Mar 10, 2024

As for the potentially affected packages, #294068 has shown more than 5000+ rebuild on Linux and stdenv rebuild on Darwin. nixpkgs-review shows 55040 packages on my x86_64-linux NixOS laptop. I can paste them here if need be.

Update: Not as many for this PR.

@ShamrockLee ShamrockLee requested a review from drupol March 10, 2024 22:49
@ShamrockLee
Copy link
Contributor Author

ShamrockLee commented Mar 10, 2024

It seems that OfBorg actually rebuilds the specified packages, which is great for testing purpose.

@drupol drupol merged commit 54c52cb into NixOS:master Mar 11, 2024
24 checks passed
@ShamrockLee ShamrockLee deleted the sourceroot-fix branch March 11, 2024 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: python 6.topic: vim 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants