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

nixpkgs-review doesn't differentiate between build and store errors. #415

Closed
CyberShadow opened this issue Sep 8, 2024 · 12 comments · Fixed by #416
Closed

nixpkgs-review doesn't differentiate between build and store errors. #415

CyberShadow opened this issue Sep 8, 2024 · 12 comments · Fixed by #416

Comments

@CyberShadow
Copy link
Contributor

CyberShadow commented Sep 8, 2024

Hi, I'm a relatively new nixpkgs contributor. nixpkgs-review is advertised very visibly in the PR template, so I've been trying to use it for my own PRs.

However, one issue I have is that when nixpkgs-review reports that something failed to build, it doesn't provide any other information.

Today, while working on NixOS/nixpkgs#276662, I tried it on my own PR. I ran:

$ nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
$ git -c fetch.prune=false fetch --no-tags --force https://github.com/NixOS/nixpkgs master:refs/nixpkgs-review/0
remote: Enumerating objects: 200, done.
remote: Counting objects: 100% (139/139), done.
remote: Compressing objects: 100% (57/57), done.
remote: Total 92 (delta 73), reused 48 (delta 32), pack-reused 0 (from 0)
Unpacking objects: 100% (92/92), 13.61 KiB | 240.00 KiB/s, done.
From https://github.com/NixOS/nixpkgs
   31a69c3d855b..61ddb09cfaa7  master     -> refs/nixpkgs-review/0
$ git worktree add /home/vladimir/.cache/nixpkgs-review/rev-0752b57db97bb02e23eb0880855baa841ccee4b0-4/nixpkgs 61ddb09cfaa7424d7fc8e3040ccd5c8c6f875b15
Preparing worktree (detached HEAD 61ddb09cfaa7)
Updating files: 100% (43112/43112), done.
HEAD is now at 61ddb09cfaa7 Revert "syncthing-relay: 1.27.9 -> 1.27.10" (#340552)
$ nix-env --extra-experimental-features no-url-literals --option system x86_64-linux -f <nixpkgs> --nix-path nixpkgs=/home/vladimir/.cache/nixpkgs-review/rev-0752b57db97bb02e23eb0880855baa841ccee4b0-4/nixpkgs nixpkgs-overlays=/tmp/tmpjeha_iv6 -qaP --xml --out-path --show-trace --no-allow-import-from-derivation
$ git merge --no-commit --no-ff 0752b57db97bb02e23eb0880855baa841ccee4b0
Automatic merge went well; stopped before committing as requested
$ nix-env --extra-experimental-features no-url-literals --option system x86_64-linux -f <nixpkgs> --nix-path nixpkgs=/home/vladimir/.cache/nixpkgs-review/rev-0752b57db97bb02e23eb0880855baa841ccee4b0-4/nixpkgs nixpkgs-overlays=/tmp/tmpjeha_iv6 -qaP --xml --out-path --show-trace --no-allow-import-from-derivation --meta
5 packages added:
blivet-gui (init at 2.5.0) python311Packages.bytesize (init at 2.11) python312Packages.bytesize (init at 2.11) python311Packages.blivet (init at 3.10.1) python312Packages.blivet (init at 3.10.1)

133 packages updated:
adapta-gtk-theme almanah ayatana-indicator-datetime bubblemail budgie-control-center calls cantata cheese cinnamon-common cinnamon-gsettings-overrides cinnamon-screensaver clementine coreaction corearchiver corefm coregarage corehunt coreimage coreinfo corekeyboard corepad corepaint corepdf corepins corerenamer coreshot corestats corestuff coreterminal coretime coretoppings coreuniverse dde-device-formatter dde-file-manager deepin-anything deepin-compressor deepin-music elementary-calendar elementary-greeter elementary-mail elementary-session-settings elementary-tasks endeavour enlightenment eos-installer evolution evolution-data-server evolution-data-server evolution-ews evolution-with-plugins exaile folks geary gfbgraph gitg gnome-applets gnome-browser-connector gnome-calendar gnome-contacts gnome-control-center gnome-disk-utility gnome-flashback gnome-gsettings-overrides gnome-multi-writer gnome-music gnome-notes gnome-online-accounts gnome-online-accounts-gtk gnome-panel gnome-panel-with-modules gnome-photos gnome-recipes gnome-session gnome-shell gnome-shell-extension-EasyScreenCast gnome-shell-extension-gsconnect gnome-tweaks grilo-plugins gvfs gvfs libblockdev libbytesize libcsys libgdata libmsgraph libzapojit lomiri lomiri-session lomiri-system-settings lomiri-system-settings-unwrapped mailnag mate-control-center mate-panel-with-applets mate-settings-daemon-wrapped mate-utils matrix-gtk-theme mediascanner2 mediawriter mojave-gtk-theme monitor nemo nemo-fileroller nemo-python nemo-with-extensions nixos-gsettings-desktop-schemas ns-usbloader phosh phosh-mobile-settings planify plasticity psensor rapid-photo-downloader spacefm SwayNotificationCenter switchboard-plug-about switchboard-plug-onlineaccounts switchboard-with-plugs tokyonight-gtk-theme totem udiskie udisks udisks2-qt5 usbimager usermount util-dfm valent vifm-full vimix-gtk-themes whitesur-gtk-theme wingpanel-applications-menu wingpanel-indicator-datetime wingpanel-with-indicators xmonad-log-applet-gnomeflashback-unstable

warning: ignoring the client-specified setting 'system', because it is a restricted setting and you are not a trusted user
$ nix build --nix-path nixpkgs=/home/vladimir/.cache/nixpkgs-review/rev-0752b57db97bb02e23eb0880855baa841ccee4b0-4/nixpkgs nixpkgs-overlays=/tmp/tmpjeha_iv6 --extra-experimental-features nix-command no-url-literals --no-link --keep-going --no-allow-import-from-derivation --option build-use-sandbox relaxed -f /home/vladimir/.cache/nixpkgs-review/rev-0752b57db97bb02e23eb0880855baa841ccee4b0-4/build.nix
warning: ignoring the client-specified setting 'sandbox', because it is a restricted setting and you are not a trusted user
3 packages marked as broken and skipped:
mailnagWithPlugins mailnagWithPlugins.dist xmonad_log_applet

2 packages failed to build:
libblockdev.python libbytesize

184 packages built:
CuboCore.coreaction CuboCore.corearchiver CuboCore.corefm CuboCore.coregarage CuboCore.corehunt CuboCore.coreimage CuboCore.coreinfo CuboCore.corekeyboard CuboCore.corepad CuboCore.corepaint CuboCore.corepdf CuboCore.corepins CuboCore.corerenamer CuboCore.coreshot CuboCore.corestats CuboCore.corestuff CuboCore.coreterminal CuboCore.coretime CuboCore.coretoppings CuboCore.coreuniverse CuboCore.libcsys adapta-gtk-theme almanah ayatana-indicator-datetime blivet-gui blivet-gui.dist bubblemail bubblemail.dist budgie-control-center budgie-control-center.debug calls calls.devdoc cantata cheese cheese.devdoc cheese.man cinnamon-common cinnamon-gsettings-overrides cinnamon-screensaver clementine deepin.dde-device-formatter deepin.dde-file-manager deepin.dde-gsettings-schemas deepin.deepin-anything deepin.deepin-compressor deepin.deepin-music deepin.udisks2-qt5 deepin.util-dfm endeavour enlightenment.enlightenment eos-installer evolution evolution-data-server evolution-data-server-gtk4 evolution-data-server-gtk4.dev evolution-data-server.dev evolution-ews evolutionWithPlugins exaile folks folks.dev folks.devdoc geary gfbgraph gfbgraph.dev gfbgraph.devdoc gitg gnome-applets gnome-browser-connector gnome-calendar gnome-contacts gnome-control-center gnome-control-center.debug gnome-disk-utility gnome-flashback gnome-multi-writer gnome-music gnome-notes gnome-online-accounts gnome-online-accounts-gtk gnome-online-accounts.debug gnome-online-accounts.dev gnome-online-accounts.devdoc gnome-online-accounts.man gnome-panel gnome-panel-with-modules gnome-panel.dev gnome-panel.man gnome-photos gnome-photos.installedTests gnome-recipes gnome-session gnome-session.debug gnome-session.sessions gnome-shell gnome-shell.debug gnome-shell.devdoc gnome-tweaks gnome.gvfs gnome.gvfs.debug gnome.nixos-gsettings-overrides gnomeExtensions.easyScreenCast gnomeExtensions.gsconnect gnomeExtensions.gsconnect.installedTests grilo-plugins gvfs gvfs.debug libblockdev libblockdev.dev libblockdev.devdoc libbytesize.dev libbytesize.devdoc libbytesize.man libgdata libgdata.dev libgdata.installedTests libmsgraph libmsgraph.dev libmsgraph.devdoc libzapojit libzapojit.dev lomiri.lomiri lomiri.lomiri-session lomiri.lomiri-system-settings lomiri.lomiri-system-settings-unwrapped lomiri.lomiri-system-settings-unwrapped.dev lomiri.mediascanner2 lomiri.mediascanner2.dev mate.mate-control-center mate.mate-panel-with-applets mate.mate-settings-daemon-wrapped mate.mate-utils matrix-gtk-theme mediawriter mojave-gtk-theme monitor nemo nemo-fileroller nemo-python nemo-with-extensions nemo.dev ns-usbloader pantheon.elementary-calendar pantheon.elementary-greeter pantheon.elementary-mail pantheon.elementary-session-settings pantheon.elementary-tasks pantheon.switchboard-plug-about pantheon.switchboard-plug-onlineaccounts pantheon.switchboard-with-plugs pantheon.wingpanel-applications-menu pantheon.wingpanel-indicator-datetime pantheon.wingpanel-with-indicators phosh phosh-mobile-settings planify plasticity psensor python311Packages.blivet python311Packages.blivet.dist python311Packages.bytesize python311Packages.bytesize.dev python311Packages.bytesize.devdoc python311Packages.bytesize.man python312Packages.blivet python312Packages.blivet.dist rapid-photo-downloader rapid-photo-downloader.dist spaceFM swaynotificationcenter tokyonight-gtk-theme totem udiskie udiskie.dist udisks udisks.dev udisks.devdoc udisks.man usbimager usermount valent vifm-full vimix-gtk-themes whitesur-gtk-theme

$ /nix/store/11glc2yk4jmycvk42q6kikpdcpbnbf6w-nix-2.17.1/bin/nix-shell /home/vladimir/.cache/nixpkgs-review/rev-0752b57db97bb02e23eb0880855baa841ccee4b0-4/shell.nix --nix-path nixpkgs=/home/vladimir/.cache/nixpkgs-review/rev-0752b57db97bb02e23eb0880855baa841ccee4b0-4/nixpkgs nixpkgs-overlays=/tmp/tmpjeha_iv6
this derivation will be built:
  /nix/store/a4n6wsfldnncgk59gwgwvfljlr607apk-env.drv
building '/nix/store/a4n6wsfldnncgk59gwgwvfljlr607apk-env.drv'...
warning: collision between `/nix/store/cv7hzga05rnshw31hnqi0hjd67r9dbld-nixos-gsettings-desktop-schemas/share/gsettings-schemas/nixos-gsettings-overrides/glib-2.0/schemas/org.gnome.desktop.a11y.applications.gschema.xml' and `/nix/store/rx9zy1ffbrx0rg3hx3ra27rc82hkzbib-cinnamon-gsettings-overrides/share/gsettings-schemas/nixos-gsettings-overrides/glib-2.0/schemas/org.gnome.desktop.a11y.applications.gschema.xml'
[... lots of "warning: collision ..." omitted ...]
created 28896 symlinks in user environment

So, nixpkgs-review says that libblockdev.python and libbytesize failed to build.

But, what can I do with this information, and what should I do about it?

I don't know why they failed to build, because there is no build log or other details.

If I try to build them manually, by running nix-build -A libblockdev.python and nix-build -A libbytesize, that succeeds. So, clearly nixpkgs-review builds them in some other way that causes that build to fail. But, how do I reproduce the problem?

I see that nixpkgs-review printed a command that it ran in order to perform the build (nix build --nix-path ...). But, if I try to copy and run this command, it succeeds as well.

So, I'm stuck. The tool says there's a problem, but it's not giving any explanation, details, or a way to reproduce it.

@CyberShadow
Copy link
Contributor Author

Digging a little deeper, I found that nixpkgs-review thinks the build failed because nix-store --verify-path for these packages failed.

Running that command directly, indeed I get e.g. path '/nix/store/sr9nnsq3ljdwbzliqdab5sn07pzppab2-libblockdev-3.1.1-python' was modified! expected hash 'sha256:01scbfbiy74g4gka763zsx6fdch6g7vknqlhxzbi6r6mc25m9xbx', got 'sha256:1bqmsqvd8c27l4gqm8vh6lqv248b2j1ghvy7lg9yr07bxbjmwhil'.

Running sudo nix-store --repair-path on them fixed the problem, whatever it was.

I think it would be very helpful if nixpkgs-review printed the reason why it thinks the package failed to build.

@SuperSandro2000
Copy link
Collaborator

nixpkgs-review is advertised very visibly in the PR template, so I've been trying to use it for my own PRs.

I've been saying for years that it shouldn't be thrown at every PR especially if you don't know what you want to achieve with it.

But, what can I do with this information, and what should I do about it?

cd into nixpkgs and try to build it

So, clearly nixpkgs-review builds them in some other way that causes that build to fail. But, how do I reproduce the problem?

No. The builds could be flaky or your machine could be overloaded.

You probably want to look in the logs above that where produced and then take a good guess.

Running sudo nix-store --repair-path on them fixed the problem, whatever it was.

So probably that store entry was corrupted for whatever reason.

I think it would be very helpful if nixpkgs-review printed the reason why it thinks the package failed to build.

What should it say that wouldn't be vague and unhelpful? A build can fail for many reasons and only compile errors are obvious. Everything else is needs good knowledge.

@Mic92
Copy link
Owner

Mic92 commented Sep 9, 2024

The reason why build fails should be in the build logs. You can get nicer error reporting by installing nix-output-monitor. nixpkgs-review doesn't have any special build method that is different from running nix-build yourself. If it succeeds the second time, it is likely a flaky build.

@Mic92 Mic92 closed this as completed Sep 9, 2024
@CyberShadow
Copy link
Contributor Author

cd into nixpkgs and try to build it

...

No. The builds could be flaky or your machine could be overloaded.

You probably want to look in the logs above that where produced and then take a good guess.

Hi @SuperSandro2000 , I'm not sure who the above is directed at but as I hope you can see these suggestions don't apply in this situation. As I wrote above, I've already tried to build it, and as you can see in the log I posted, there was no build log that would indicate the source of the error. So, these suggestions are not useful in this situation.

So probably that store entry was corrupted for whatever reason.

I was not using NixOS in this situation, and the program I was testing has a component that runs as root, so I wonder if it was something like automatically creating .pyc files. I'll try this the next time I run into this.

What should it say that wouldn't be vague and unhelpful?

A simple but good improvement would be something like:

- Failed to validate libblockdev.python ("nix-store --verify-path /nix/store/sr9nnsq3ljdwbzliqdab5sn07pzppab2-libblockdev-3.1.1-python" failed)
- Failed to validate libbytesize ("nix-store --verify-path /nix/store/...........-libbytesize-....." failed)

@CyberShadow
Copy link
Contributor Author

The reason why build fails should be in the build logs.

They're not. Sorry, but did you read my follow-up comment?

@CyberShadow
Copy link
Contributor Author

To clarify, the build didn't fail. It was just the method that nixpkgs-review uses to check if the build succeeded causes it to misleadingly report that the build "failed". The reality is that there is a problem, but of a different nature.

@Mic92 Sorry but I think you were hasty to close this.

@Mic92
Copy link
Owner

Mic92 commented Sep 9, 2024

@CyberShadow You seem to use a single-user installation of nix. I would recommend to install a multi-user nix version using the nix-daemon. Than also random programs can no longer modify store paths as you have seen /nix/store/sr9nnsq3ljdwbzliqdab5sn07pzppab2-libblockdev-3.1.1-python because the nix-store is mounted read-only. From the point of nixpkgs-review, there is little we can fix to help your problems with the nix package manager.

@Mic92
Copy link
Owner

Mic92 commented Sep 9, 2024

nixpkgs-review currently uses this to check if a path has been build:

["nix-store", "--verify-path", self.path], stderr=subprocess.DEVNULL

@CyberShadow
Copy link
Contributor Author

You seem to use a single-user installation of nix.

@Mic92 No, I'm using a multi-user installation of Nix.

/nix/store being protected from writes from the root user is a NixOS feature.

From the point of nixpkgs-review, there is little we can fix to help your problems with the nix package manager.

Well, I regret starting the issue with "I'm a relatively new nixpkgs contributor", because all that did was create an invitation to skim over the issue description and hastily classify it as user error.

Let's rephrase the problem: there are situations, other than build errors, which nixpkgs-review currently incorrectly reports as build errors. In these cases, the nixpkgs-review output does not include any details that can allow users to determine the source of the problem.

So, i think we should at least try to avoid incorrectly reporting this situation as a failed build.

nixpkgs-review currently uses this to check if a path has been build:

Indeed, I have discovered this as well.

I noticed that nix store verify has different exit codes depending on the situation. If we can use the nix3 CLI, we could switch over to it and provide better diagnostics.

Are there any other ways we can discern a store path validation error from a legitimate build failure?

@Mic92 Mic92 reopened this Sep 9, 2024
@Mic92 Mic92 changed the title Please explain why packages failed to build nixpkgs-review doesn't differentiate between build and store errors. Sep 9, 2024
@Mic92
Copy link
Owner

Mic92 commented Sep 9, 2024

I would accept a pull request that switches to a different build check command. I currently don't see this as a big priority that I would get to myself soonish, because it's quite rare that these things happen.

@CyberShadow
Copy link
Contributor Author

Happy to provide a PR. Do we have a range of supported Nix versions?

@Mic92
Copy link
Owner

Mic92 commented Sep 9, 2024

2.18 and newer.

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 a pull request may close this issue.

3 participants