Skip to content

tracker: Unconditionally add checkInputs to nativeBuildInputs#154096

Merged
jtojnar merged 1 commit intoNixOS:stagingfrom
zhaofengli:tracker-gi-buildinput
Jan 25, 2022
Merged

tracker: Unconditionally add checkInputs to nativeBuildInputs#154096
jtojnar merged 1 commit intoNixOS:stagingfrom
zhaofengli:tracker-gi-buildinput

Conversation

@zhaofengli
Copy link
Member

@zhaofengli zhaofengli commented Jan 9, 2022 •
edited
Loading

Motivation for this change

gi is required by the main meson.build and checked at the start of the build process, regardless of whether tests are enabled or not. As a result, builds are broken when doCheck = false.

Also removing tappy which is no longer needed: https://gitlab.gnome.org/GNOME/tracker/-/merge_requests/385

Ref: #101651

[snip]
Run-time dependency libsoup-3.0 found: YES 3.0.2
Library m found: YES
Library dl found: YES
Program asciidoc found: YES (/nix/store/cs2zga4mgm4r6jcrh8szqdd54mwbqnzh-asciidoc-9.1.0/bin/asciidoc)
Program xsltproc found: YES (/nix/store/xigx9nnnq4j5bx13sc6xh1154anjrqs7-libxslt-1.1.34-bin/bin/xsltproc)
Program python3 (gi) found: NO

meson.build:83:0: ERROR: python3 is missing modules: gi

A full log can be found at /build/tracker-3.2.1/build/meson-logs/meson-log.txt
builder for '/nix/store/dxfi43by4vh08pks154hl90gh3jhd5sx-tracker-3.2.1.drv' failed with exit code 1
Previous description

It is required in the main meson.build and checked at the start of the build process. On riscv64-linux, simply having it in checkInputs doesn't add it to PYTHONPATH but it somehow does on x86_64-linux.

Things done
  • Built on platform(s)
    • x86_64-linux
    • riscv64-linux
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 22.05 Release Notes (or backporting 21.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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot requested review from amaxine, dasj19, hedning and jtojnar January 9, 2022 03:11
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Jan 9, 2022
jtojnar
jtojnar reviewed Jan 9, 2022
View reviewed changes
pkgs/development/libraries/tracker/default.nix Outdated
Copy link
Member

@jtojnar jtojnar Jan 9, 2022

Choose a reason for hiding this comment

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

Looks like the check was added in https://gitlab.gnome.org/GNOME/tracker/-/commit/8569c99d2926d55419affa6c5269053d5c4dc97a. And also it appears tappy is not needed any more https://gitlab.gnome.org/GNOME/tracker/-/merge_requests/385.

@jtojnar
Copy link
Member

jtojnar commented Jan 9, 2022

Maybe something is disabling doCheck? checkInputs should be normally passed to nativeBuildInputs.

nixpkgs/pkgs/stdenv/generic/make-derivation.nix

Line 150 in cf26d7d

++ lib.optionals doCheck checkInputs

@zhaofengli
Copy link
Member Author

zhaofengli commented Jan 9, 2022

Maybe something is disabling doCheck? checkInputs should be normally passed to nativeBuildInputs.

nixpkgs/pkgs/stdenv/generic/make-derivation.nix

Line 150 in cf26d7d

++ lib.optionals doCheck checkInputs

Oh my, you are right - I do have a local change that disables doCheck for this package that I have forgotten about 🙈 But regardless, the package should still build with tests disabled (e.g., cross compilation).

Let me remove tappy from checkInputs.

@zhaofengli zhaofengli force-pushed the tracker-gi-buildinput branch from 2ded819 to 3251fa0 Compare January 9, 2022 03:43
@ofborg ofborg bot requested a review from jtojnar January 9, 2022 03:56
@jtojnar
Copy link
Member

jtojnar commented Jan 9, 2022

I would probably go about it by adding checkInputs to nativeBuildInputs unconditionally (with an explanatory comment). Alternately, we could add a meson option to disable tests, which would also get rid of the check.

@zhaofengli
tracker: Unconditionally add checkInputs to nativeBuildInputs
f8fa83f 
`gi` is required by the main `meson.build` and checked at the start
of the build process, regardless of whether tests are enabled or not.
As a result, builds are broken when `doCheck = false`.

Also removing tappy which is no longer needed: https://gitlab.gnome.org/GNOME/tracker/-/merge_requests/385
@zhaofengli zhaofengli force-pushed the tracker-gi-buildinput branch from 3251fa0 to f8fa83f Compare January 9, 2022 20:37
@zhaofengli
Copy link
Member Author

zhaofengli commented Jan 9, 2022

Alternately, we could add a meson option to disable tests, which would also get rid of the check.

I don't think it can be disabled, since it got moved to the main meson.build from the tests that you linked in https://gitlab.gnome.org/GNOME/tracker/-/commit/51ca035697a4cdbf8bf6849cc67dc8d462358a05.

I would probably go about it by adding checkInputs to nativeBuildInputs unconditionally (with an explanatory comment).

For the reason above, I think this is the correct choice. Let me amend the commit.

@zhaofengli zhaofengli changed the title tracker: Add pygobject3 to buildInputs tracker: Unconditionally add checkInputs to nativeBuildInputs Jan 9, 2022
@SFrijters SFrijters mentioned this pull request Jan 13, 2022
Closed
13 tasks
@jtojnar jtojnar merged commit b2f8a5e into NixOS:staging Jan 25, 2022
@zhaofengli zhaofengli deleted the tracker-gi-buildinput branch January 26, 2022 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@0x4A6F 0x4A6F 0x4A6F approved these changes

@amaxine amaxine Awaiting requested review from amaxine

@dasj19 dasj19 Awaiting requested review from dasj19

@hedning hedning Awaiting requested review from hedning

@jtojnar jtojnar Awaiting requested review from jtojnar

Assignees

No one assigned

Labels

10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@zhaofengli @jtojnar @0x4A6F