Skip to content

Fix test evaluation in restrict-eval #50342

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

Closed
Ekleog wants to merge 2 commits into from
Closed

Fix test evaluation in restrict-eval #50342

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
32dfb74
release: reset `revCount` to a dummy placeholder
Ekleog Nov 14, 2018
b832c46
release: do not cleanSource when evaluating in restrict-eval
Ekleog Nov 14, 2018
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
2 changes: 1 addition & 1 deletion lib/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ let
appendToName mapDerivationAttrset lowPrio lowPrioSet hiPrio
hiPrioSet;
inherit (sources) pathType pathIsDirectory cleanSourceFilter
cleanSource sourceByRegex sourceFilesBySuffices
cleanSource cleanSourceForImport sourceByRegex sourceFilesBySuffices
commitIdFromGitRepo cleanSourceWith pathHasContext
canCleanSource;
inherit (modules) evalModules closeModules unifyModuleSyntax
Expand Down
12 changes: 12 additions & 0 deletions lib/sources.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,18 @@ rec {
# cleanSource ./.
cleanSource = src: cleanSourceWith { filter = cleanSourceFilter; inherit src; };

# Cleans the source if not operating in restrict-eval. This will make
# it possible to import from the cleaned source.
#
# Example:
# cleanSourceForImport ./..
cleanSourceForImport = src:
# This is using a very ugly hack to know whether we're evaluating in restrict-mode.
# Please don't do this at home.
if builtins.pathExists "/"
then cleanSource src
else src;

# Like `builtins.filterSource`, except it will compose with itself,
# allowing you to chain multiple calls together without any
# intermediate copies being put in the nix store.
Expand Down
2 changes: 1 addition & 1 deletion nixos/release-combined.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# and nixos-14.04). The channel is updated every time the ‘tested’ job
# succeeds, and all other jobs have finished (they may fail).

{ nixpkgs ? { outPath = (import ../lib).cleanSource ./..; revCount = 56789; shortRev = "gfedcba"; }
{ nixpkgs ? { outPath = (import ../lib).cleanSourceForImport ./..; revCount = 654321; shortRev = "gfedcba"; }
, stableBranch ? false
, supportedSystems ? [ "x86_64-linux" ]
, limitedSupportedSystems ? [ "i686-linux" ]
Expand Down
2 changes: 1 addition & 1 deletion nixos/release-small.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# small subset of Nixpkgs, mostly useful for servers that need fast
# security updates.

{ nixpkgs ? { outPath = (import ../lib).cleanSource ./..; revCount = 56789; shortRev = "gfedcba"; }
{ nixpkgs ? { outPath = (import ../lib).cleanSourceForImport ./..; revCount = 654321; shortRev = "gfedcba"; }
, stableBranch ? false
, supportedSystems ? [ "x86_64-linux" ] # no i686-linux
}:
Expand Down
4 changes: 3 additions & 1 deletion nixos/release.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
with import ../lib;

{ nixpkgs ? { outPath = cleanSource ./..; revCount = 130979; shortRev = "gfedcba"; }
# `revCount` and `shortRev` are placeholders, that should be filled
# automatically for releases by passing the `nixpkgs` argument
{ nixpkgs ? { outPath = cleanSourceForImport ./..; revCount = 654321; shortRev = "gfedcba"; }
Copy link
Member

@grahamc grahamc Nov 23, 2018

Choose a reason for hiding this comment

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

This revcount isn't a placeholder, it is updated when new channels are cut.

Copy link
Member Author

@Ekleog Ekleog Nov 26, 2018

Choose a reason for hiding this comment

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

@grahamc Are you sure about that? I ran git log -p nixos/release.nix before writing this comment, and it was set to 56789 since May 2014 before 18.03 cut-off (and before that it was set to 5678 since Oct 2013, and before that to a mix of 1234 for nixosSrc and 5678 for nixpkgs), and it hasn't been updated when cutting out 18.09. This is the reason why I thought @vcunat's updating it when 18.03 was cut was potentially a mistake, but…?

Copy link
Member Author

@Ekleog Ekleog Dec 8, 2018

Choose a reason for hiding this comment

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

@grahamc I'd like to try and move this forward. Could you either confirm, infirm or say you don't have time to evaluate my statement above? :)

Copy link
Member

@grahamc grahamc Dec 8, 2018

Choose a reason for hiding this comment

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

I don't have time to dive deeper, however, I thought:

Update versionSuffix in nixos/release.nix, use git log --format=%an|wc -l to get the commit count

was supposed to include updating this count. (https://nixos.org/nixos/manual/index.html#at-beta-release-time)

Maybe I'm wrong, and I trust you to do the digging, and I don't feel like I need to be part of making the decision if this is true.

, stableBranch ? false
, supportedSystems ? [ "x86_64-linux" "aarch64-linux" ]
, configuration ? {}
Expand Down