stdenv: only log hooks if $NIX_DEBUG >= 4 by philiptaron · Pull Request #331560 · NixOS/nixpkgs

philiptaron · 2024-08-01T17:17:09Z

Motivation for change

Make the logging output by default from the stdenv less verbose.

Description of changes

Sourcing setup hook *default hook* spam in writers/trivial builders #328229

Due to feedback about the amount of logging output in the above issue, this PR moves the hook logging added in PRs #290081 and #310387 under $NIX_DEBUG, specifically at "talkative" level.

As a part of this commit, nixLog is no more, replaced with these eight available logging functions:

nixErrorLog (logs at $NIX_DEBUG>=0 level, which is always)
nixWarnLog (logs at $NIX_DEBUG>=1 level)
nixNoticeLog (logs at $NIX_DEBUG>=2 level)
nixInfoLog (logs at $NIX_DEBUG>=3 level)
nixTalkativeLog (logs at $NIX_DEBUG>=4 level)
nixChattyLog (logs at $NIX_DEBUG>=5 level)
nixDebugLog (logs at $NIX_DEBUG>=6 level)
nixVomitLog (logs at $NIX_DEBUG>=7 level)

All of these names are from the Nix source code.

Setting $NIX_DEBUG is left as an exercise for the reader.

{nix,lix}: add withNixDebug overrideable #331794

Things done

emilazy

A little nit. Don’t feel like building stdenv to test this out, but the basic temporary compromise seems sound to me, even if it will probably satisfy nobody.

I do think that this should probably be a higher value than 2 so that we have room to add things that are less noisy than this, but it doesn’t really matter that much.

pkgs/stdenv/generic/setup.sh

philiptaron · 2024-08-03T20:48:35Z

To test this, btw, I use nix-build --arg pkgs 'import ./. {}' ./lib/tests/release.nix, and then again with a custom Nix that sets NIX_DEBUG to see the other side.

pkgs/stdenv/generic/setup.sh

Nix filters out messages with level ≥ 4 by default as of this commit.

tie · 2024-08-06T08:08:01Z

pkgs/stdenv/generic/setup.sh

+# $NIX_DEBUG must be a documented integer level, if set, so we can use it safely as an integer.
+# See the `Verbosity` enum in the Nix source for these levels.
+if ! [[ -z ${NIX_DEBUG-} || $NIX_DEBUG == [0-7] ]]; then
+    printf 'The `NIX_DEBUG` environment variable has an unexpected value: %s\n' "${NIX_DEBUG}"


I think it’s OK to use echo in this case since we know that the argument will not be interpreted as a flag.

Suggested change

printf 'The `NIX_DEBUG` environment variable has an unexpected value: %s\n' "${NIX_DEBUG}"

echo "The `NIX_DEBUG` environment variable has an unexpected value: ${NIX_DEBUG@Q}"

Note: ${parameter@Q} quotes the string, similar to printf %q. See https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter-Expansion.html

marcusramberg

This change looks good to me. Ran the tests over night and it seems to work well in practice as well, although I didn't test with debugging enabled. I agree with keeping it simple with the cost of some duplication. Would be very nice to get this merged as the current logging of hooks seems to the be causing some real world problems.

Mindavi · 2024-08-09T07:07:40Z

Not sure if this is an issue with the logging infra or nix, but I'm noticing some issues in nix develop mode:

mindavi@nixos-asus:~/nixpkgs$ nix develop -f default.nix xenPackages.xen_4_17
mindavi@nixos-asus:~/nixpkgs$ unpackPhase 
unpacking source archive /nix/store/hwh8vvj30w09w328hmpbkmmhjn034njn-xen-d530627
bash: "$NIX_LOG_FD": Bad file descriptor
source root is xen-d530627
bash: "$NIX_LOG_FD": Bad file descriptor

tie · 2024-08-09T08:13:30Z

@Mindavi, this should have been fixed in #330830 👀

Mindavi · 2024-08-09T08:17:21Z

Ah ok, thanks. Staging is not on master yet and my working tree is normally not staging. So probably with a next staging merge it'll be good.

Atemu · 2024-08-09T09:11:25Z

@Mindavi note that the PR is in staging-next already if you need it urgently.

Mindavi · 2024-08-09T11:42:14Z

No worries, not in a rush 🙂

philiptaron requested a review from Ericson2314 as a code owner August 1, 2024 17:17

github-actions bot added the 6.topic: stdenv Standard environment label Aug 1, 2024

philiptaron linked an issue Aug 1, 2024 that may be closed by this pull request

Sourcing setup hook *default hook* spam in writers/trivial builders #328229

Closed

philiptaron requested review from Ericson2314, Qyriad, emilazy and tie and removed request for Ericson2314 August 1, 2024 17:17

philiptaron mentioned this pull request Aug 1, 2024

{nix,lix,stdenv}: log hook information only if NIX_DEBUG >= 1; provide means to set it #331383

Closed

13 tasks

emilazy reviewed Aug 1, 2024

View reviewed changes

pkgs/stdenv/generic/setup.sh Outdated Show resolved Hide resolved

philiptaron marked this pull request as draft August 1, 2024 18:01

philiptaron force-pushed the issue-328229/stdenv-spam-less-only branch from 0f6c60c to 1938735 Compare August 1, 2024 18:51

philiptaron changed the title ~~stdenv: only log hooks if $NIX_DEBUG >= 2~~ stdenv: only log hooks if $NIX_DEBUG >= 3 Aug 1, 2024

philiptaron force-pushed the issue-328229/stdenv-spam-less-only branch from 1938735 to c48d19f Compare August 1, 2024 18:53

tie reviewed Aug 1, 2024

View reviewed changes

pkgs/stdenv/generic/setup.sh Outdated Show resolved Hide resolved

tie reviewed Aug 1, 2024

View reviewed changes

pkgs/stdenv/generic/setup.sh Outdated Show resolved Hide resolved

philiptaron force-pushed the issue-328229/stdenv-spam-less-only branch from c48d19f to a7363f6 Compare August 2, 2024 23:15

philiptaron marked this pull request as ready for review August 2, 2024 23:17

tie reviewed Aug 3, 2024

View reviewed changes

pkgs/stdenv/generic/setup.sh Outdated Show resolved Hide resolved

philiptaron force-pushed the issue-328229/stdenv-spam-less-only branch from a7363f6 to 916aedb Compare August 3, 2024 17:39

philiptaron changed the title ~~stdenv: only log hooks if $NIX_DEBUG >= 3~~ stdenv: only log hooks if $NIX_DEBUG >= 4 Aug 3, 2024

philiptaron requested review from emilazy and tie August 3, 2024 20:47

philiptaron requested a review from RossComputerGuy August 3, 2024 22:55

philiptaron mentioned this pull request Aug 5, 2024

Sourcing setup hook *default hook* spam in writers/trivial builders #328229

Closed

tie reviewed Aug 5, 2024

View reviewed changes

pkgs/stdenv/generic/setup.sh Outdated Show resolved Hide resolved

pkgs/stdenv/generic/setup.sh Outdated Show resolved Hide resolved

pkgs/stdenv/generic/setup.sh Outdated Show resolved Hide resolved

philiptaron force-pushed the issue-328229/stdenv-spam-less-only branch from 916aedb to 2c93603 Compare August 5, 2024 23:18

philiptaron added 5 commits August 5, 2024 18:35

stdenv: set the phase in showPhaseHeader

bbb9f2f

stdenv: introduce specific logging functions

6244633

stdenv: log hooks at nixTalkativeLog level

465dbd2

Nix filters out messages with level ≥ 4 by default as of this commit.

stdenv: replace other $NIX_DEBUG log statements

e844424

stdenv: change the logging in _allFlags to talkative

d8fbb16

philiptaron force-pushed the issue-328229/stdenv-spam-less-only branch from 2c93603 to d8fbb16 Compare August 6, 2024 03:23

tie reviewed Aug 6, 2024

View reviewed changes

marcusramberg approved these changes Aug 6, 2024

View reviewed changes

philiptaron merged commit 3f3dd53 into NixOS:staging Aug 6, 2024

philiptaron deleted the issue-328229/stdenv-spam-less-only branch August 6, 2024 17:03

philiptaron mentioned this pull request Aug 6, 2024

treewide: support structuredAttrs in setup hooks #318614

Merged

13 tasks

philiptaron mentioned this pull request Aug 13, 2024

installShellFiles: rework and add installBin function #332612

Merged

13 tasks

AndersonTorres mentioned this pull request Aug 14, 2024

rustPlatform: improve composability when the other builder defines a phase #334476

Draft

15 tasks

emilazy mentioned this pull request Aug 19, 2024

[2024-2025] Nixpkgs committer nominations NixOS/nixpkgs-committers#50

Closed

limeytexan mentioned this pull request Oct 29, 2024

Remove noise about hooks from pure build output flox/flox#2127

Closed

philiptaron mentioned this pull request Jan 22, 2025

stdenv: refactor nix logging functions #370742

Merged

13 tasks

	printf 'The `NIX_DEBUG` environment variable has an unexpected value: %s\n' "${NIX_DEBUG}"
	echo "The `NIX_DEBUG` environment variable has an unexpected value: ${NIX_DEBUG@Q}"

Uh oh!

Conversation

philiptaron commented Aug 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation for change

Description of changes

Things done

Uh oh!

emilazy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

philiptaron commented Aug 3, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tie Aug 6, 2024

Choose a reason for hiding this comment

Uh oh!

marcusramberg left a comment

Choose a reason for hiding this comment

Uh oh!

Mindavi commented Aug 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tie commented Aug 9, 2024

Uh oh!

Mindavi commented Aug 9, 2024

Uh oh!

Atemu commented Aug 9, 2024

Uh oh!

Mindavi commented Aug 9, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

philiptaron commented Aug 1, 2024 •

edited

Loading

Mindavi commented Aug 9, 2024 •

edited

Loading