Skip to content

Make functional tests depend on nix binary so they auto recompile#13683

Merged
xokdvium merged 1 commit intoNixOS:masterfrom
fzakaria:fzakaria/meson-improvement
Aug 8, 2025
Merged

Make functional tests depend on nix binary so they auto recompile#13683
xokdvium merged 1 commit intoNixOS:masterfrom
fzakaria:fzakaria/meson-improvement

Conversation

@fzakaria
Copy link
Contributor

@fzakaria fzakaria commented Aug 3, 2025

Trivial improvement to Meson.
I noticed plenty of tests don't depend on nix itself which is frustrating since I forget to do meson compile.

Slowly making the build graph more "correct".
I'm not an expert in Meson so not sure if this is ultimately correct but it does what I expect.


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@fzakaria fzakaria requested a review from edolstra as a code owner August 3, 2025 22:33
@fzakaria
Copy link
Contributor Author

fzakaria commented Aug 3, 2025

@xokdvium ask and you shall receive.

@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Aug 3, 2025
@xokdvium

This comment was marked as outdated.

@xokdvium

This comment was marked as resolved.

@fzakaria
Copy link
Contributor Author

fzakaria commented Aug 3, 2025

This works for me locally... not sure about this nix build why it's complaining.
I'm trying to see what the "correct" approach is with declare_dependency or what not.

@fzakaria fzakaria force-pushed the fzakaria/meson-improvement branch from 5ff41ee to b4cd2cf Compare August 4, 2025 00:57
@fzakaria
Copy link
Contributor Author

fzakaria commented Aug 4, 2025

The meson setup is pretty convoluted.
Maybe @Ericson2314 can recommend the "right way".

Folks on #mesonbuild didn't give me much in the way of advice.

@Ericson2314
Copy link
Member

It's supposed to already do this with the existing "find program", IIRC. If it's not working, a bug should be submitted to Meson.

@Ericson2314
Copy link
Member

You also don't want to change the main suite, you want to change the loop at the bottom to include nix in addition to the suite-specific deps.

@fzakaria
Copy link
Contributor Author

fzakaria commented Aug 4, 2025

It does not work for me @Ericson2314 -- just change any source line and run meson test -C build fetchGit for instance.
Can you confirm you see the same issue?

I couldn't find documentation claiming that findProgram should be causing rebuilds properly. If so, I can open up a ticket with our project as a reproducer.

@Ericson2314
Copy link
Member

Ericson2314 commented Aug 4, 2025

@fzakaria so to be clear, my first comment is maybe not quite right, I think the find_program return value should cause proper rebuilds, but only if it is used. If we do

diff --git a/tests/functional/meson.build b/tests/functional/meson.build
index 8f2b1ff59a..4a7dfae5d5 100644
--- a/tests/functional/meson.build
+++ b/tests/functional/meson.build
@@ -249,7 +249,7 @@ foreach suite : suites
       # them more time than the default of 30 seconds.
       timeout : 300,
       # Used for target dependency/ordering tracking, not adding compiler flags or anything.
-      depends : suite['deps'],
+      depends : [nix] + suite['deps'],
       workdir : workdir,
     )
   endforeach

(what my second comment proposes), then it should work. If it still doesn't work, then I think that is a Meson bug.

@fzakaria
Copy link
Contributor Author

fzakaria commented Aug 4, 2025

@Ericson2314 the build was failing with depends having nix because of the type:

ERROR: test keyword argument 'depends' was of type array[ExternalProgram]
but should have been array[BuildTarget | CustomTarget | CustomTargetIndex]

@Ericson2314
Copy link
Member

ah ok @fzakaria. Then yes it might be a feature request, but it should still be a Meson issue:

  1. When the program is truly external, this should just be ignored. Truly external things don't change by definition.
  2. When the program is overridden with an internal program, it should just work via the underlying target.

@Ericson2314
Copy link
Member

I'll just open the issue :)

@Ericson2314
Copy link
Member

mesonbuild/meson#14874 OK there is the issue

With this I'm able to do a fresh config + meson test with all dependencies
correctly propagated.

Co-authored-by: Sergei Zimmerman <sergei@zimmerman.foo>
@xokdvium xokdvium force-pushed the fzakaria/meson-improvement branch from b4cd2cf to bf32046 Compare August 8, 2025 00:22
@github-actions github-actions bot added the new-cli Relating to the "nix" command label Aug 8, 2025
@xokdvium
Copy link
Contributor

xokdvium commented Aug 8, 2025

@fzakaria, @Ericson2314 this should now work perfectly locally as well as in CI. Also needed some fixes for perl dependency tracking.

@xokdvium xokdvium merged commit 5451ad4 into NixOS:master Aug 8, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants