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

Log what nix flake check does #8893

Merged
merged 4 commits into from
Jan 24, 2024
Merged

Log what nix flake check does #8893

merged 4 commits into from
Jan 24, 2024

Conversation

9999years
Copy link
Contributor

@9999years 9999years commented Aug 31, 2023

There's still room for improvement, but this produces much more informative output with -v:

$ nix flake check -v
evaluating flake...
checking flake output 'checks'...
checking derivation 'checks.aarch64-darwin.ghcid-ng-tests'...
derivation evaluated to /nix/store/nh7dlvsrhds4cxl91mvgj4h5cbq6skmq-ghcid-ng-test-0.3.0.drv
checking derivation 'checks.aarch64-darwin.ghcid-ng-clippy'...
derivation evaluated to /nix/store/9cb5a6wmp6kf6hidqw9wphidvb8bshym-ghcid-ng-clippy-0.3.0.drv
checking derivation 'checks.aarch64-darwin.ghcid-ng-doc'...
derivation evaluated to /nix/store/8brdd3jbawfszpbs7vdpsrhy80as1il8-ghcid-ng-doc-0.3.0.drv
checking derivation 'checks.aarch64-darwin.ghcid-ng-fmt'...
derivation evaluated to /nix/store/wjhs0l1njl5pyji53xlmfjrlya0wmz8p-ghcid-ng-fmt-0.3.0.drv
checking derivation 'checks.aarch64-darwin.ghcid-ng-audit'...
derivation evaluated to /nix/store/z0mps8dyj2ds7c0fn0819y5h5611033z-ghcid-ng-audit-0.3.0.drv
checking flake output 'packages'...
checking derivation 'packages.aarch64-darwin.default'...
derivation evaluated to /nix/store/41abbdyglw5x9vcsvd89xan3ydjf8d7r-ghcid-ng-0.3.0.drv
checking flake output 'apps'...
checking flake output 'devShells'...
checking derivation 'devShells.aarch64-darwin.default'...
derivation evaluated to /nix/store/bc935gz7dylzmcpdb5cczr8gngv8pmdb-nix-shell.drv
running 5 flake checks...
warning: The check omitted these incompatible systems: aarch64-linux, x86_64-darwin, x86_64-linux
Use '--all-systems' to check all.

In the future, it would be nice to print out the output paths and whether or not they were actually built (sometimes the checks are cached).

Motivation/Context

See #8882:

As of Nix 2.17.0, when I run nix flake check, the output doesn't tell me what was checked:

$ nix flake check
warning: The check omitted these incompatible systems: aarch64-linux, x86_64-darwin, x86_64-linux
Use '--all-systems' to check all.

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

  • agreed on idea
  • agreed on implementation strategy
  • tests, as appropriate
    • functional tests - tests/**.sh
    • unit tests - src/*/tests
    • integration tests - tests/nixos/*
  • documentation in the manual
  • documentation in the internal API docs
  • code and comments are self-explanatory
  • commit message explains why the change was made
  • new feature or incompatible change: updated release notes

@github-actions github-actions bot added the new-cli Relating to the "nix" command label Aug 31, 2023
@9999years
Copy link
Contributor Author

By the way, I'm very confused by the logging code in nix. When logged at lvlChatty, the message is displayed on the current line and erased when another log message is emitted. When logged at lvlInfo, the result is the same, except that when --verbose is used subsequent log messages are printed on new lines. I can't find the code that's responsible for this anywhere — as far as I can tell, the --verbose flag just controls which levels of log messages are shown or not:

.handler = {[]() { verbosity = (Verbosity) (verbosity + 1); }},

if (lvl <= verbosity && !s.empty())
log(lvl, s + "...");

I though this might have something to do with the progress bar logger, but the various --option log-format values didn't seem to have any effect:

Logger * makeDefaultLogger() {
switch (defaultLogFormat) {
case LogFormat::raw:
return makeSimpleLogger(false);
case LogFormat::rawWithLogs:
return makeSimpleLogger(true);
case LogFormat::internalJSON:
return makeJSONLogger(*makeSimpleLogger(true));
case LogFormat::bar:
return makeProgressBar();
case LogFormat::barWithLogs: {
auto logger = makeProgressBar();
logger->setPrintBuildLogs(true);
return logger;
}
default:
abort();
}
}

Any pointers here would be appreciated.

@9999years
Copy link
Contributor Author

I was surprised to notice that nix flake check only builds the checks -- I would have expected it to build the packages, devShells, apps, and so on as well. Perhaps a future PR can address that?

@phip1611
Copy link
Contributor

Any update here? Would love to see this too!

Copy link
Contributor

@tomberek tomberek left a comment

Choose a reason for hiding this comment

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

Looks good. Log level changes are helpful. Needs a conflict rebase.

There's still room for improvement, but this produces much more
informative output with `-v`:

```
$ nix flake check -v
evaluating flake...
checking flake output 'checks'...
checking derivation checks.aarch64-darwin.ghcid-ng-tests...
checking derivation checks.aarch64-darwin.ghcid-ng-clippy...
checking derivation checks.aarch64-darwin.ghcid-ng-doc...
checking derivation checks.aarch64-darwin.ghcid-ng-fmt...
checking derivation checks.aarch64-darwin.ghcid-ng-audit...
checking flake output 'packages'...
checking derivation packages.aarch64-darwin.ghcid-ng...
checking derivation packages.aarch64-darwin.ghcid-ng-tests...
checking derivation packages.aarch64-darwin.default...
checking flake output 'apps'...
checking flake output 'devShells'...
checking derivation devShells.aarch64-darwin.default...
running flake checks...
warning: The check omitted these incompatible systems: aarch64-linux, x86_64-darwin, x86_64-linux
Use '--all-systems' to check all.
```
Also be more consistent with quotes around attribute paths
@9999years
Copy link
Contributor Author

@tomberek Rebased to fix conflicts, PTAL

@9999years
Copy link
Contributor Author

9999years commented Jan 20, 2024

Actually, I'll need to give this some more attention. Even though the logs are emitted at lvlInfo, they seem to only be printed temporarily, even at -v, and are erased when the 'activity' ends. Really weird behavior.

EDIT: Nevermind, I just forgot to make install. 🤦

@tomberek tomberek merged commit 775d59f into NixOS:master Jan 24, 2024
8 checks passed
@9999years 9999years deleted the fix-8882 branch January 24, 2024 02:16
9999years added a commit to 9999years/nix that referenced this pull request Mar 7, 2024
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants