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.

Sign up for GitHub

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

Run 'make installcheck' again #8005

Merged
merged 1 commit into from
Mar 8, 2023
Merged

Run 'make installcheck' again #8005

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions flake.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,7 @@

doInstallCheck = finalAttrs.doCheck;
installCheckFlags = "sysconfdir=$(out)/etc";
installCheckTarget = "installcheck"; # work around buggy detection in stdenv
Copy link
Member

Choose a reason for hiding this comment

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

Is there a Nixpkgs issue for this stdenv buggy detection?

Copy link
Member

Choose a reason for hiding this comment

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

Good question, FWIW I had trouble reproducing it locally.

Copy link
Member

Choose a reason for hiding this comment

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

Not answering your question, but I suspect from past discussions that it's from this + which breaks make -n (because that's magic for “recursive make” and make executes these rules regardless of -n)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it's indeed the + that forces make -n to execute the command anyway. I don't understand why it's there in the first place...

Copy link
Member

Choose a reason for hiding this comment

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

Oh huh. Good eye. That's very spooky magic...

Copy link
Member

Choose a reason for hiding this comment

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

Sounds like some future work may be to remove the + and see what breaks, since it's unknown and can cause subtle breakage itself 👀

Copy link
Member

Choose a reason for hiding this comment

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

Quick blame shows it was introduced (and discussed) here: 2799fe4 (#5813)

Copy link
Member

Choose a reason for hiding this comment

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

Or we just NixOS/rfcs#132 😀

Copy link
Member

Choose a reason for hiding this comment

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

2799fe4#r1099608721

Oh that's very interesting. And arguably Ninja and the LTO stuff should use same jobserver things too so my Meson joke is not the full story.

I wonder if the variable can just expand to put in the + or that doesn't work.

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if the variable can just expand to put in the + or that doesn't work.

Google tells me that we might be able to hack our way out. But that's quite nasty.


separateDebugInfo = !currentStdenv.hostPlatform.isStatic;

Expand Down