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

Don't use the 'nix' command #87182

Merged
merged 3 commits into from May 7, 2020
Merged

Don't use the 'nix' command #87182

merged 3 commits into from May 7, 2020

Conversation

@edolstra
Copy link
Member

edolstra commented May 7, 2020

Motivation for this change

In Nix 2.4, the nix command requires experimental-features = nix-command in nix.conf. So we can't use it in nixos-install or in the activation script yet.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
edolstra added 2 commits May 7, 2020
This reverts commits 9d0de0d,
27d2857. 'nix ping-store' is an
experimental command so it doesn't work in Nix 2.4 unless you set
'experimental-features = nix-command' in nix.conf.
'nix build' is an experimental command so we shouldn't use it
yet. (nixos-rebuild also uses 'nix', but only when using flakes, which
are themselves an experimental feature.)
If a program (e.g. nixos-install) writes more than 1000 lines to
stderr during execute(), then process_serial_output() deadlocks
waiting for the queue to be processed. So use an unbounded queue
instead.

We should probably get rid of the structured log output (log.xml),
since then we don't need the log queue anymore.
@edolstra edolstra requested a review from tfc as a code owner May 7, 2020
@tfc
Copy link
Contributor

tfc commented May 7, 2020

The changes in the python driver look like drive-by changes, or do they fix some kind of concrete problem that you ran into?

@edolstra
Copy link
Member Author

edolstra commented May 7, 2020

Yes, without them the installer test hangs.

@tfc
Copy link
Contributor

tfc commented May 7, 2020

The changes look innocent so i'd have no objections merging them but i am interested in understanding how the pre-allocated queue size or the end-pattern blocked this specific test while they work on more than 200 other tests.

@edolstra
Copy link
Member Author

edolstra commented May 7, 2020

Probably because they don't write more than 1000 lines to the serial console between commands.

@edolstra edolstra merged commit 06b9302 into NixOS:master May 7, 2020
14 checks passed
14 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="78f2a83"; rev="78f2a830291eb19af6306476b186b585f4d9cd37"; } ./pkgs/t
Details
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="78f2a83"; rev="78f2a830291eb19af6306476b186b585f4d9cd37"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="78f2a83"; rev="78f2a830291eb19af6306476b186b585f4d9cd37"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="78f2a83"; rev="78f2a830291eb19af6306476b186b585f4d9cd37"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="78f2a83"; rev="78f2a830291eb19af6306476b186b585f4d9cd37"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="78f2a83"; rev="78f2a830291eb19af6306476b186b585f4d9cd37"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="78f2a83"; rev="78f2a830291eb19af6306476b186b585f4d9cd37"; } ./pkgs/t
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@edolstra edolstra deleted the edolstra:no-experimental branch May 7, 2020
@flokli

This comment has been minimized.

Copy link
Contributor

flokli commented on 78f2a83 May 9, 2020

This still seems to be broken - see the failure in #86486 (comment).

@LnL7
Copy link
Member

LnL7 commented May 10, 2020

Can this also be backported to 19.09 or will it also require fixes for the tests there?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.