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

lib, treewide: introduce repoRevToName and use it to cleanup most fetch* functions #316668

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ let
inherit (self.sources) cleanSourceFilter
cleanSource sourceByRegex sourceFilesBySuffices
commitIdFromGitRepo cleanSourceWith pathHasContext
canCleanSource pathIsGitRepo;
canCleanSource pathIsGitRepo repoRevToName;
inherit (self.modules) evalModules setDefaultModuleLocation
unifyModuleSyntax applyModuleArgsIfFunction mergeModules
mergeModules' mergeOptionDecls mergeDefinitions
Expand Down
60 changes: 60 additions & 0 deletions lib/sources.nix
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,62 @@ let
outPath = builtins.path { inherit filter name; path = origSrc; };
};

# urlToName : (URL | Path | String) -> String
#
# Transform a URL (or path, or string) into a clean package name.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe I can be wrong, but the general term is URI.

Copy link
Member Author

Choose a reason for hiding this comment

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

As a thing more general than a URL? Yes. But here it is literally "a URL, or a path, or a string (a package name, usually)".

urlToName = url:
let
inherit (builtins) stringLength;
base = baseNameOf (lib.removeSuffix "/" (lib.last (lib.splitString ":" (toString url))));
# chop away one git or archive-related extension
removeExt = name: let
matchExt = builtins.match "(.*)\\.(git|tar|zip|gz|tgz|bz|tbz|bz2|tbz2|lzma|txz|xz|zstd)" name;
in if matchExt != null then lib.head matchExt else name;
# apply function f to string x while the result shrinks
shrink = f: x: let v = f x; in if stringLength v < stringLength x then shrink f v else x;
in shrink removeExt base;

# shortRev : (String | Integer) -> String
#
# Given a package revision (like "refs/tags/v12.0"), produce a short revision ("12.0").
shortRev = rev:
let
baseRev = baseNameOf (toString rev);
matchHash = builtins.match "[a-f0-9]+" baseRev;
matchVer = builtins.match "([A-Za-z]+[-_. ]?)*(v)?([0-9.]+.*)" baseRev;
in
if matchHash != null then builtins.substring 0 7 baseRev
else if matchVer != null then lib.last matchVer
else baseRev;

# repoRevToNameFull : (URL | Path | String) -> (String | Integer | null) -> (String | null) -> String
#
# See `repoRevToName` below.
repoRevToNameFull = repo_: rev_: suffix_:
let
repo = urlToName repo_;
rev = if rev_ != null then "-${shortRev rev_}" else "";
suffix = if suffix_ != null then "-${suffix_}" else "";
in "${repo}${rev}${suffix}-source";

# repoRevToName : (Bool | String) -> (URL | Path | String) -> (String | Integer | null) -> String -> String
#
# Produce derivation.name attribute for a given repository URL/path/name and (optionally) its revision/version tag.
#
# This is used by fetch(zip|git|FromGitHub|hg|svn|etc) to generate discoverable
# /nix/store paths.
#
# This uses a different implementation depending on the `pretty` argument:
# false -> name everything as "source"
Copy link
Member

Choose a reason for hiding this comment

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

To be honest I do not like dual-type attributes. This is a serious footgun in a duck-typed language.

And let's remember Nix is not stellar on the error report department:
https://discourse.nixos.org/t/rant-finding-errors-in-module-configs-after-incompatible-upgrades-is-dreadful/46375

Either is everything a string, say format ? "short", "medium", "long" or we stick to only two options (and then we can use a Boolean).

Copy link
Member Author

Choose a reason for hiding this comment

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

But it is not checked by Nix but by the config.nix and NixOS typing infra, and the errors it generates are pretty nice, e.g. with nameSourcesPrettily = "error":

$ nix-instantiate ./default.nix -A xlife
error: A definition for option `nameSourcesPrettily' is not of type `boolean or value "full" (singular enum)'. Definition values:
- In `nixpkgs.config': "error"
(use '--show-trace' to show detailed location information)

Copy link
Member

Choose a reason for hiding this comment

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

Hum, less bad.

Nonetheless, semantically it is a clear case for an enumeration with three possible states, instead of a weird Boolean-plus-an-extra.

# true -> name everything as "${repo}-${rev}-source"
# "full" -> name everything as "${repo}-${rev}-${fetcher}-source"
repoRevToName = pretty:
# match on `pretty` first to minimize the thunk
if pretty == false then (repo: rev: suffix: "source")
else if pretty == true then (repo: rev: suffix: repoRevToNameFull repo rev null)
else if pretty == "full" then repoRevToNameFull
else throw "invalid value of `pretty'";

in {

pathType = lib.warnIf (lib.isInOldestRelease 2305)
Expand All @@ -278,6 +334,10 @@ in {
pathHasContext
canCleanSource

urlToName
shortRev
repoRevToName

sourceByRegex
sourceFilesBySuffices

Expand Down
5 changes: 3 additions & 2 deletions pkgs/build-support/fetchbitbucket/default.nix
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
{ fetchzip, lib }:
{ lib, repoRevToNameMaybe, fetchzip }:

lib.makeOverridable (
{ owner, repo, rev, name ? "source"
{ owner, repo, rev
, name ? repoRevToNameMaybe repo rev "bitbucket"
, ... # For hash agility
}@args: fetchzip ({
inherit name;
Expand Down
22 changes: 10 additions & 12 deletions pkgs/build-support/fetchgit/default.nix
Original file line number Diff line number Diff line change
@@ -1,22 +1,20 @@
{lib, stdenvNoCC, git, git-lfs, cacert}: let
urlToName = url: rev: let
inherit (lib) removeSuffix splitString last;
base = last (splitString ":" (baseNameOf (removeSuffix "/" url)));

matched = builtins.match "(.*)\\.git" base;
{lib, stdenvNoCC, git, git-lfs, cacert}:

short = builtins.substring 0 7 rev;

appendShort = lib.optionalString ((builtins.match "[a-f0-9]*" rev) != null) "-${short}";
in "${if matched == null then base else builtins.head matched}${appendShort}";
let
urlToName = url: rev: let
shortRev = lib.sources.shortRev rev;
appendShort = lib.optionalString ((builtins.match "[a-f0-9]*" rev) != null) "-${shortRev}";
in "${lib.sources.urlToName url}${appendShort}";
in

lib.makeOverridable (
{ url, rev ? "HEAD", sha256 ? "", hash ? "", leaveDotGit ? deepClone
{ url, rev ? "HEAD"
, name ? urlToName url rev
, sha256 ? "", hash ? "", leaveDotGit ? deepClone
, fetchSubmodules ? true, deepClone ? false
, branchName ? null
, sparseCheckout ? []
, nonConeMode ? false
, name ? urlToName url rev
, # Shell code executed after the file has been fetched
# successfully. This can do things like check or transform the file.
postFetch ? ""
Expand Down
5 changes: 3 additions & 2 deletions pkgs/build-support/fetchgithub/default.nix
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
{ lib, fetchgit, fetchzip }:
{ lib, repoRevToNameMaybe, fetchgit, fetchzip }:

lib.makeOverridable (
{ owner, repo, rev, name ? "source"
{ owner, repo, rev
, name ? repoRevToNameMaybe repo rev "github"
, fetchSubmodules ? false, leaveDotGit ? null
, deepClone ? false, private ? false, forceFetchGit ? false
, sparseCheckout ? []
Expand Down
7 changes: 5 additions & 2 deletions pkgs/build-support/fetchgitiles/default.nix
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
{ fetchzip, lib }:
{ fetchzip, repoRevToNameMaybe, lib }:

lib.makeOverridable (
{ url, rev, name ? "source", ... } @ args:
{ url, rev
, name ? repoRevToNameMaybe url rev "gitiles"
, ...
} @ args:

fetchzip ({
inherit name;
Expand Down
5 changes: 3 additions & 2 deletions pkgs/build-support/fetchgitlab/default.nix
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
{ lib, fetchgit, fetchzip }:
{ lib, repoRevToNameMaybe, fetchgit, fetchzip }:

lib.makeOverridable (
# gitlab example
{ owner, repo, rev, protocol ? "https", domain ? "gitlab.com", name ? "source", group ? null
{ owner, repo, rev, protocol ? "https", domain ? "gitlab.com", group ? null
, name ? repoRevToNameMaybe repo rev "gitlab"
, fetchSubmodules ? false, leaveDotGit ? false
, deepClone ? false, forceFetchGit ? false
, sparseCheckout ? []
Expand Down
5 changes: 3 additions & 2 deletions pkgs/build-support/fetchrepoorcz/default.nix
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
{ fetchzip }:
{ lib, repoRevToNameMaybe, fetchzip }:

# gitweb example, snapshot support is optional in gitweb
{ repo, rev, name ? "source"
{ repo, rev
, name ? repoRevToNameMaybe repo rev "repoorcz"
, ... # For hash agility
}@args: fetchzip ({
inherit name;
Expand Down
5 changes: 3 additions & 2 deletions pkgs/build-support/fetchsavannah/default.nix
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
{ fetchzip, lib }:
{ lib, repoRevToNameMaybe, fetchzip }:

lib.makeOverridable (
# cgit example, snapshot support is optional in cgit
{ repo, rev, name ? "source"
{ repo, rev
, name ? repoRevToNameMaybe repo rev "savannah"
, ... # For hash agility
}@args: fetchzip ({
inherit name;
Expand Down
4 changes: 2 additions & 2 deletions pkgs/build-support/fetchsourcehut/default.nix
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{ fetchgit, fetchhg, fetchzip, lib }:
{ lib, repoRevToNameMaybe, fetchgit, fetchhg, fetchzip }:

let
inherit (lib)
Expand All @@ -13,7 +13,7 @@ makeOverridable (
, repo, rev
, domain ? "sr.ht"
, vc ? "git"
, name ? "source"
, name ? repoRevToNameMaybe repo rev "sourcehut"
, fetchSubmodules ? false
, ... # For hash agility
} @ args:
Expand Down
49 changes: 23 additions & 26 deletions pkgs/build-support/fetchsvn/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -2,41 +2,38 @@
, subversion, glibcLocales, sshSupport ? true, openssh ? null
}:

{ url, rev ? "HEAD", sha256 ? "", hash ? ""
, ignoreExternals ? false, ignoreKeywords ? false, name ? null
let
repoToName = url: rev:
let
inherit (lib) removeSuffix splitString reverseList head last elemAt;
base = removeSuffix "/" (last (splitString ":" url));
path = reverseList (splitString "/" base);
repoName =
# ../repo/trunk -> repo
if head path == "trunk" then elemAt path 1
# ../repo/branches/branch -> repo-branch
else if elemAt path 1 == "branches" then "${elemAt path 2}-${head path}"
# ../repo/tags/tag -> repo-tag
else if elemAt path 1 == "tags" then "${elemAt path 2}-${head path}"
# ../repo (no trunk) -> repo
else head path;
in "${repoName}-r${toString rev}";
in

{ url, rev ? "HEAD"
, name ? repoToName url rev
, sha256 ? "", hash ? ""
, ignoreExternals ? false, ignoreKeywords ? false
, preferLocalBuild ? true
}:

assert sshSupport -> openssh != null;

let
repoName = with lib;
let
fst = head;
snd = l: head (tail l);
trd = l: head (tail (tail l));
path_ =
(p: if head p == "" then tail p else p) # ~ drop final slash if any
(reverseList (splitString "/" url));
path = [ (removeSuffix "/" (head path_)) ] ++ (tail path_);
in
# ../repo/trunk -> repo
if fst path == "trunk" then snd path
# ../repo/branches/branch -> repo-branch
else if snd path == "branches" then "${trd path}-${fst path}"
# ../repo/tags/tag -> repo-tag
else if snd path == "tags" then "${trd path}-${fst path}"
# ../repo (no trunk) -> repo
else fst path;

name_ = if name == null then "${repoName}-r${toString rev}" else name;
in

if hash != "" && sha256 != "" then
throw "Only one of sha256 or hash can be set"
else
stdenvNoCC.mkDerivation {
name = name_;
inherit name;
builder = ./builder.sh;
nativeBuildInputs = [ subversion glibcLocales ]
++ lib.optional sshSupport openssh;
Expand Down
12 changes: 8 additions & 4 deletions pkgs/build-support/fetchzip/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,14 @@
# (e.g. due to minor changes in the compression algorithm, or changes
# in timestamps).

{ lib, fetchurl, withUnzip ? true, unzip, glibcLocalesUtf8 }:
{ lib, repoRevToNameMaybe, fetchurl
, withUnzip ? true, unzip
, glibcLocalesUtf8
}:

{ name ? "source"
, url ? ""
{ url ? ""
, urls ? []
, name ? repoRevToNameMaybe (if url != "" then url else builtins.head urls) null "unpacked"
, nativeBuildInputs ? []
, postFetch ? ""
, extraPostFetch ? ""
Expand All @@ -22,7 +25,8 @@
, extension ? null

# the rest are given to fetchurl as is
, ... } @ args:
, ...
} @ args:

assert (extraPostFetch != "") -> lib.warn "use 'postFetch' instead of 'extraPostFetch' with 'fetchzip' and 'fetchFromGitHub' or 'fetchFromGitLab'." true;

Expand Down
3 changes: 3 additions & 0 deletions pkgs/top-level/all-packages.nix
Original file line number Diff line number Diff line change
Expand Up @@ -1090,6 +1090,9 @@ with pkgs;

broadlink-cli = callPackage ../tools/misc/broadlink-cli { };

# this is used by most `fetch*` functions
repoRevToNameMaybe = lib.repoRevToName config.nameSourcesPrettily;

fetchpatch = callPackage ../build-support/fetchpatch {
# 0.3.4 would change hashes: https://github.com/NixOS/nixpkgs/issues/25154
patchutils = buildPackages.patchutils_0_3_3;
Expand Down
33 changes: 33 additions & 0 deletions pkgs/top-level/config.nix
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,39 @@ let
default = false;
};

nameSourcesPrettily = mkOption {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
nameSourcesPrettily = mkOption {
usePrettierSourceNames = mkOption {

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm, but why? nameSourcesPrettily is shorter and means the same thing.

Copy link
Member

Choose a reason for hiding this comment

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

It's shorter because it is more obscure.
verb-adjective-name is clearer than name-name-adverb.

On the other hand, it depends on the previous tristate vs boolean-plus-state discussion.

type = types.uniq (types.either types.bool (types.enum ["full"]));
default = false;
description = lib.mdDoc ''
This controls the default derivation name attribute generated by the
`fetch*` (like `fetchzip`, `fetchFromGitHub`, etc) functions.

Possible values and the resulting `.name`:

- `false` -> `"source"`
- `true` -> `"''${repo}-''${rev}-source"`
- `"full"` -> `"''${repo}-''${rev}-''${fetcherName}-source"`

The default `false` is the best choice for minimal rebuilds, it will
ignore any non-hash changes (like branches being renamed, source URLs
changing, etc).

Setting this to `true` greatly helps with discoverability of sources
in `/nix/store` at the cost of a single mass-rebuild for all `src`
derivations (but not their referrers, as all `src`s are fixed-output)
and occasional rebuild when a source changes some of its non-hash
attributes.

Setting this to `"full"` is similar to setting it to `true`, but the
use of `fetcherName` in the derivation name will force a rebuild when
`src` switches between `fetch*` functions, thus forcing `nix` to check
new derivation's `outputHash`, which is useful for debugging.

Also, `"full"` is useful for easy collection and tracking of
statistics of where the packages you use are hosted.
'';
};

doCheckByDefault = mkMassRebuild {
feature = "run `checkPhase` by default";
};
Expand Down