-
-
Notifications
You must be signed in to change notification settings - Fork 15k
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
nixos/modules/flake/source-info: new module #179772
base: master
Are you sure you want to change the base?
Conversation
}; | ||
|
||
config = { | ||
system.configurationRevision = mkIf (sourceInfo ? rev) sourceInfo.rev; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we set it to "dirty"
when we see that sourceInfo
is from git but does not have rev
?
We don't have a type
-like attribute in there (yet?), but the presence of submodules
should give it away.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The intention of setting it to dirty
, to my understanding, is to help identify the state of the configuration better. To answer this question well, I checked the relevant code, and have two proposals:
-
We can do it for all supported VCSes, not just git. For instance, mercurial is also supported (albeit immature). This is partly due to only git getting special handling while examining local paths, and we may improve that too to enable support for other VCSes. This also implies a standardized, fine-grained specification of source information, as will be discussed in
sourceInfo
does not tell us whether it's git or not nix#6747. -
To further tell differences among
dirty
configurations, we can fetch the latest update time of a repository and set something likedirty-<timestamp>
. The latest update time can be defined as the most recent modification time of all files in the repository.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@roberth Do we solve this later as a part of NixOS/rfcs#125?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can scope this discussion out regardless.
We could avoid storing the rev in the derivation output and make the non-derivation parts of NixOS responsible for propagating this information. NixOS/rfcs#125 (comment) |
nixos/modules/flake/source-info.nix
Outdated
type = types.attrsOf types.anything; | ||
default = {}; | ||
example = "self.sourceInfo"; | ||
description = '' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could do this and then write markdown if you like
description = '' | |
description = lib.mdDoc '' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated. The example also gets improved.
nixos/modules/flake/source-info.nix
Outdated
The source information of a flake-based NixOS configuration. | ||
If set, the attribute <literal>configurationRevision</literal> | ||
will appear properly in the output of | ||
<literal>nixos-version --json</literal>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't it also occur in the grub and systemd-boot boot menus?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does not. Currently, the menus contain only the shortRev
of nixpkgs
. Should we add the configuration revision to versionSuffix
too? If both revisions are to be included, they should be made distinctive.
Note that the nixpkgs
revision can be dirty
too; we may develop a uniform way to obtain revisions, and apply it to both occasions. What do you think about my earlier proposals on setting the dirty
tag? I can start to work on a prototype after we discuss and reach an agreement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does not.
Ok. That can be done in another PR if desirable.
we may develop a uniform way to obtain revisions
Sounds good, but these are all quite independent problems. It's best to resolve them in separate PRs, so that maintainers don't need to keep (in the limit) all of NixOS in their head in order to approve them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, there will be at least three PRs: this one, one for obtaining revisions, and another one for applying the feature. I will try to write a detailed design later.
<literal>nixos-version --json</literal>. | ||
|
||
Caution: Setting this option may result in unnecessary | ||
re-deployments if the flake repository changes but the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re-deployments if the flake repository changes but the | |
boot loader entries or re-deployments if the flake repository changes but the |
Implementation lgtm. I think we'll need to create a section in the NixOS manual as well. There currently isn't one, but that's going to be a problem when flakes are released. If we're going to write those docs last minute, they'll be incomplete, and probably skip over this feature. |
I just checked that the missing docs were not only flakes for NixOS but also flakes for Nix. But, unfortunately, writing a whole bunch of such docs is out of my reach right now. Hence I added a section for this feature but did not integrate it into the current NixOS manual. This way, someone will notice that it is there in the future. |
Description of changes
This PR introduces a new module that improves
configurationRevision
handling as discussed in #178837.This option can be set as in the following example:
See more details in the module description.
Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)nixos/doc/manual/md-to-db.sh
to update generated release notes