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

siji: init at 2016-05-14 #22283

Merged
merged 1 commit into from Jan 30, 2017
Merged

siji: init at 2016-05-14 #22283

merged 1 commit into from Jan 30, 2017

Conversation

asymmetric
Copy link
Contributor

@asymmetric asymmetric commented Jan 29, 2017

Motivation for this change

Init package siji

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

Copy link
Member

@rasendubi rasendubi left a comment

Choose a reason for hiding this comment

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

Minor issues. Also, builds fine on my NixOS machine with sandboxing.

name = "siji-${date}";
date = "2016-05-14";

src = fetchgit {
Copy link
Member

Choose a reason for hiding this comment

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

fetchFromGitHub is preferred (as it fetches less):

fetchFromGitHub {
  owner = "stark";
  repo = "siji";
  rev = "95369afac3e661cb6d3329ade5219992c88688c1";
  sha256 = "...";
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe fetchFromGitHub only downloads less when there's a release available, which is not the case here:

  fetchFromGitHub = {
    owner, repo, rev, name ? "${repo}-${rev}-src",
    fetchSubmodules ? false,
    ... # For hash agility
  }@args:
  let
    baseUrl = "https://github.com/${owner}/${repo}";
    passthruAttrs = removeAttrs args [ "owner" "repo" "rev" "fetchSubmodules" ];
  in if fetchSubmodules then
    fetchgit ({
      inherit name rev fetchSubmodules;
      url = "${baseUrl}.git";
    } // passthruAttrs)
  else
    # We prefer fetchzip in cases we don't need submodules as the hash
    # is more stable in that case.
    fetchzip ({
      inherit name;
      url = "${baseUrl}/archive/${rev}.tar.gz";
      meta.homepage = "${baseUrl}/";
    } // passthruAttrs) // { inherit rev; };

Copy link
Member

Choose a reason for hiding this comment

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

There is no release-specific code. For any revision of siji, you can download a tarball using the link https://github.com/stark/siji/archive/<rev>.tar.gz.

Try:

rasen@ashmalko ~ % nix-prefetch-url https://github.com/stark/siji/archive/95369afac3e661cb6d3329ade5219992c88688c1.tar.gz                                                          
downloading ‘https://github.com/stark/siji/archive/95369afac3e661cb6d3329ade5219992c88688c1.tar.gz’... [0/0 KiB, 0.0 Ki [0/0 KiB, 0.0 KiB/s]
path is ‘/nix/store/1lmsk8y7mvlcvxq6rxxz2pnx6jjg01ad-95369afac3e661cb6d3329ade5219992c88688c1.tar.gz’
1pz73pl2ylbhdbikh2nrl39xfyc8am71rhqpzx8lr28slz025q8m

Copy link
Member

@rasendubi rasendubi Jan 30, 2017

Choose a reason for hiding this comment

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

There is also another benefit of fetchFromGitHub. If you examine the code of fetchgit, it does not fetch the full repo, but only N last commits. So if the repo receives too many new commits, fetchgit will fail to checkout to the specified commit as it won't be fetched. (There is a workaround, though.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, I had a typo in the author's name and I jumped to conclusions. Fixing!


stdenv.mkDerivation rec {
name = "siji-${date}";
date = "2016-05-14";
Copy link
Member

Choose a reason for hiding this comment

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

The commit was done on May 13, so better change to 2016-05-13.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used the date returned by nix-prefetch-git:

❯ nix-prefetch-git https://github.com/stark/siji
Initialized empty Git repository in /tmp/git-checkout-tmp-UuH7HSt2/siji/.git/
remote: Counting objects: 11, done.
remote: Compressing objects: 100% (9/9), done.
remote: Total 11 (delta 0), reused 6 (delta 0), pack-reused 0
Unpacking objects: 100% (11/11), done.
From https://github.com/stark/siji
 * branch            HEAD       -> FETCH_HEAD
Switched to a new branch 'fetchgit'
removing `.git'...

git revision is 95369afac3e661cb6d3329ade5219992c88688c1
path is /nix/store/qv0p80c5pg53qpsvdjbnzk4sdgqf8gdi-siji
git human-readable version is -- none --
Commit date is 2016-05-14 00:30:21 +0530
hash is 1408g4nxwdd682vjqpmgv0cp0bfnzzzwls62cjs9zrds16xa9dpf
{
  "url": "https://github.com/stark/siji",
  "rev": "95369afac3e661cb6d3329ade5219992c88688c1",
  "date": "2016-05-14T00:30:21+05:30",
  "sha256": "1408g4nxwdd682vjqpmgv0cp0bfnzzzwls62cjs9zrds16xa9dpf",
  "fetchSubmodules": true
}

But you're right that the commit is from the 13th... Any idea why?

Copy link
Member

Choose a reason for hiding this comment

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

timezones

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I think 14 is fine, as that's a day the commit was made (in the author's timezone)

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

Successfully merging this pull request may close these issues.

None yet

2 participants