fetchgit: Support sparse checkout by azuwis · Pull Request #135881 · NixOS/nixpkgs

azuwis · 2021-08-27T08:47:00Z

Motivation for this change

This allow git checkout small parts of a large repo, and avoid fetching
unnecessary blobs from server.

For example, the nerd-fonts git repo, even with --depth=1, need network transfer > 2G. Using sparse checkout and partialclonefilter "blob:none", we can fetch a subdir for < 20M:

{ lib, stdenvNoCC, fetchgit }:

stdenvNoCC.mkDerivation rec {
  name = "jetbrains-mono-nerdfonts";

  src = fetchgit {
    url = "https://github.com/ryanoasis/nerd-fonts.git";
    rev = "bc4416e176d4ac2092345efd7bcb4abef9d6411e";
    sparseCheckout = ''
      patched-fonts/JetBrainsMono/Ligatures
    '';
    sha256 = "04ag7p8jvs6628qfhkyh7w5584xc2d27iskwl7sq8anzjv5xmrlg";
  };

  installPhase = ''
    mkdir -p $out/share/fonts/truetype
    cp patched-fonts/JetBrainsMono/Ligatures/*/complete/*Complete.ttf $out/share/fonts/truetype
  '';

  meta = with lib; {
    description = "Iconic font aggregator, collection, & patcher. 3,600+ icons, 50+ patched fonts";
    homepage = "https://nerdfonts.com/";
    license = licenses.mit;
  };
}

Things done

r-rmcgibbo · 2021-08-27T09:35:15Z

Result of nixpkgs-review pr 135881 at f48630c1 run on x86_64-linux 1

15 packages built successfully:

bundix
cabal2nix
crate2nix
crystal2nix
dep2nix
go2nix
haskellPackages.cabal2nix-unstable
haskellPackages.nvfetcher
haskellPackages.update-nix-fetchgit
nix-prefetch-git
nix-prefetch-scripts
nix-update-source
nvfetcher
update-nix-fetchgit
vgo2nix

Result of nixpkgs-review pr 135881 at bfebcde8 run on aarch64-linux 1

14 packages built successfully:

bundix
cabal2nix
crate2nix
dep2nix
go2nix
haskellPackages.cabal2nix-unstable
haskellPackages.nvfetcher
haskellPackages.update-nix-fetchgit
nix-prefetch-git
nix-prefetch-scripts
nix-update-source
nvfetcher
update-nix-fetchgit
vgo2nix

Artturin · 2021-09-08T01:49:44Z

Looks useful, could you add documentation for it too? https://nixos.org/manual/nixpkgs/stable/#fetchgit
doc/builders/fetchers.chapter.md doc/builders/fetchers.chapter.md

Should something be added to fetchFromGitHub to make it possible to use this with it?

roberth · 2021-10-02T23:33:58Z

Could you add a test case in pkgs/build-support/fetchgit/tests.nix?

azuwis · 2021-10-07T15:12:22Z

Could you please give some tips about how to write and run tests?

I read https://nixos.org/manual/nixpkgs/stable/#sec-package-tests, but can not figure out.

❯ nix-build -A hello.tests   
these derivations will be built:
  /nix/store/k7xrpk94bz0ny7ql321v0z2lwf4mhfyb-test-version.drv
these paths will be fetched (5.57 MiB download, 29.61 MiB unpacked):
  /nix/store/2yccw7959qx77a5yw612xd54mclmnxag-gmp-6.2.1
  /nix/store/3z7w87pg7xcghgdmw450qc25ws99wpfj-libiconv-50
  ...
building '/nix/store/k7xrpk94bz0ny7ql321v0z2lwf4mhfyb-test-version.drv'...
hello (GNU Hello) 2.10
/nix/store/a29vrm7h17cyxpcpaqa0m2wymidw1d59-test-version

❯ nix-build -A fetchgit.tests
error: cannot auto-call a function that has an argument without a default value ('url')

Artturin · 2021-10-07T15:18:14Z

Package tests are different to fetcher tests https://nixos.org/manual/nixpkgs/unstable/#sec-pkgs-invalidateFetcherByDrvHash

example in this pr #138869

roberth · 2021-10-07T15:22:14Z

Originally I introduced an extra attribute for the purpose of running the tests, but in the end I cut it because it shouldn't be necessary.

You can run them from nix repl .

nix-repl> :b fetchgit.tests.simple

this derivation produced the following outputs:
  out -> /nix/store/j9yzk4f0z66va8qy31wc46a9k0wvsh7r-nix-source-salted-gy6zpkzq0gmx

or nix build .#fetchgit.tests.simple.

Neither of these run all tests though, so maybe packageTests should be reintroduced. See 8651143

azuwis · 2021-11-08T01:40:39Z

Test case added, anything else need to be done?

roberth

Just a question and a comment. Otherwise, LGTM.

roberth · 2021-11-08T10:29:06Z

pkgs/build-support/fetchgit/nix-prefetch-git

Did you consider using git sparse-checkout set --stdin?

roberth · 2021-11-08T10:37:17Z

pkgs/build-support/fetchgit/nix-prefetch-git

This doesn't seem necessary. If you want to make it "correct" you could use printf %q, but the real solution is to replace this code by something simple, like a loop that pops arguments using shift.
I'll ignore it for now.

This is necessary because --sparse-checkout arg may contain "\n", without this change, the following test fails.

~/src/nixpkgs master* ⇣⇡ ❯ git diff diff --git a/pkgs/build-support/fetchgit/nix-prefetch-git b/pkgs/build-support/fetchgit/nix-prefetch-git index f940d10b2ae..be595e59b0c 100755 --- a/pkgs/build-support/fetchgit/nix-prefetch-git +++ b/pkgs/build-support/fetchgit/nix-prefetch-git @@ -98,7 +98,7 @@ for arg; do case $argfun in set_*) var=${argfun#set_} - eval $var="\"$arg\"" + eval $var=$arg ;; esac argfun="" ~/src/nixpkgs master* ⇣⇡ ❯ nix repl . Welcome to Nix version 2.5pre20211007_844dd90. Type :? for help. Loading '.'... Added 15361 variables. nix-repl> :b fetchgit.tests.sparseCheckout error: builder for '/nix/store/cnh4q2g2vmxl6l4cjq28677hqdxrwxbx-nix-source-salted-bydqcp0cva5n.drv' failed with exit code 127; last 2 log lines: > exporting https://github.com/NixOS/nix (rev 9d9dbe6ed05854e03811c361a3380e09183f4f4a) into /nix/store/yw6ycpfg088q7jswqjgjj19724drl3ji-nix-source-salted-bydqcp0cva5n > /nix/store/k4wdi6yqznx8qww12v715sz46vd4p1jf-nix-prefetch-git: line 101: tests: command not found For full logs, run 'nix log /nix/store/cnh4q2g2vmxl6l4cjq28677hqdxrwxbx-nix-source-salted-bydqcp0cva5n.drv'.

The tests comes from:

sparseCheckout = '' src tests '';

eval without quote leads to code injection.

How about something like eval "$var=$(printf %q "$arg")"

It could also append to the previous value, so it can be used multiple times on the command line. I guess that'd be append_* although I think the argument parsing should be turned into a simple loop that consumes one or more arguments per iteration. Much simpler than this weird state machine business.

declare "$var=$arg" ...

azuwis · 2021-11-12T14:02:32Z

Please review the new changes.

roberth

I'll ignore the preexisting problem of the overcomplicated argument parsing and consider this PR an incremental improvement.

doc/builders/fetchers.chapter.md

pkgs/build-support/fetchgit/nix-prefetch-git

SuperSandro2000

Can we forward that option to the various fetchFrom*?

This allow git checkout small parts of a large repo, and avoid fetching unnecessary blobs from server.

azuwis · 2022-01-24T03:33:02Z

Rebase to master, add fetchFromGitHub.

roberth · 2022-01-25T12:53:26Z

@ofborg build tests.fetchgit tests.fetchFromGitHub

nixos-discourse · 2022-03-01T07:19:24Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/extracting-sub-directory-from-fetchgit-or-fetchurl-or-any-derivation/8830/9

github-actions bot added the 6.topic: fetch Fetchers (e.g. fetchgit, fetchsvn, ...) label Aug 27, 2021

ofborg bot added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Aug 27, 2021

azuwis force-pushed the master branch from f48630c to bfebcde Compare August 27, 2021 11:29

roberth mentioned this pull request Aug 28, 2021

Fetcher testing #136022

Merged

12 tasks

github-actions bot added the 8.has: documentation This PR adds or changes documentation label Sep 8, 2021

azuwis force-pushed the master branch from d8a5d66 to 277d335 Compare October 7, 2021 16:18

azuwis force-pushed the master branch from 277d335 to 191fcbc Compare November 8, 2021 01:40

roberth reviewed Nov 8, 2021

View reviewed changes

azuwis force-pushed the master branch from 191fcbc to d6360a1 Compare November 12, 2021 13:58

azuwis force-pushed the master branch from d6360a1 to df2ddaa Compare November 12, 2021 16:20

roberth approved these changes Nov 12, 2021

View reviewed changes

SuperSandro2000 reviewed Nov 12, 2021

View reviewed changes

doc/builders/fetchers.chapter.md Outdated Show resolved Hide resolved

SuperSandro2000 reviewed Nov 12, 2021

View reviewed changes

pkgs/build-support/fetchgit/nix-prefetch-git Outdated Show resolved Hide resolved

SuperSandro2000 reviewed Nov 12, 2021

View reviewed changes

azuwis force-pushed the master branch from df2ddaa to e534336 Compare November 13, 2021 03:31

roberth approved these changes Nov 14, 2021

View reviewed changes

azuwis mentioned this pull request Jan 24, 2022

Noto Fonts CJK fails to build #156180

Closed

azuwis added 4 commits January 24, 2022 11:23

fetchgit: Support sparse checkout

d03a07d

This allow git checkout small parts of a large repo, and avoid fetching unnecessary blobs from server.

fetchgit: Add document for sparseCheckout

8c26b2d

fetchgit: Add test for sparseCheckout

36cc03e

fetchgithub: Support sparseCheckout

57829cc

azuwis force-pushed the master branch from e534336 to 57829cc Compare January 24, 2022 03:32

roberth merged commit 64b5e92 into NixOS:master Jan 25, 2022

midchildan mentioned this pull request Jan 25, 2022

noto-fonts-cjk: use sparse checkouts for smaller downloads #156342

Merged

13 tasks

DeeUnderscore mentioned this pull request Feb 11, 2022

nerd-font-patcher: use upstream repo with sparseCheckout #159298

Merged

13 tasks

rehno-lindeque mentioned this pull request Apr 25, 2022

Flake inputs should be able to do a sparse checkout of a Git repository NixOS/nix#5811

Open

Janik-Haag added the 12.first-time contribution This PR is the author's first one; please be gentle! label Jun 12, 2023

Uh oh!

Conversation

azuwis commented Aug 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation for this change

Things done

Uh oh!

r-rmcgibbo commented Aug 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Artturin commented Sep 8, 2021

Uh oh!

roberth commented Oct 2, 2021

Uh oh!

azuwis commented Oct 7, 2021

Uh oh!

Artturin commented Oct 7, 2021

Uh oh!

roberth commented Oct 7, 2021

Uh oh!

azuwis commented Nov 8, 2021

Uh oh!

roberth left a comment

Choose a reason for hiding this comment

Uh oh!

roberth Nov 8, 2021

Choose a reason for hiding this comment

Uh oh!

azuwis Nov 12, 2021

Choose a reason for hiding this comment

Uh oh!

roberth Nov 8, 2021

Choose a reason for hiding this comment

Uh oh!

azuwis Nov 12, 2021

Choose a reason for hiding this comment

Uh oh!

roberth Nov 12, 2021

Choose a reason for hiding this comment

Uh oh!

azuwis Nov 12, 2021

Choose a reason for hiding this comment

Uh oh!

milahu Apr 26, 2022

Choose a reason for hiding this comment

Uh oh!

azuwis commented Nov 12, 2021

Uh oh!

roberth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

SuperSandro2000 left a comment

Choose a reason for hiding this comment

Uh oh!

azuwis commented Jan 24, 2022

Uh oh!

roberth commented Jan 25, 2022

Uh oh!

nixos-discourse commented Mar 1, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

azuwis commented Aug 27, 2021 •

edited

Loading

r-rmcgibbo commented Aug 27, 2021 •

edited

Loading