Skip to content

Add "branch" information for builtins.fetchGit #1019

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

Merged
merged 1 commit into from
Feb 3, 2021

Conversation

ismaelbouyaf
Copy link
Contributor

@ismaelbouyaf ismaelbouyaf commented Jan 25, 2021

The builtins.fetchGit used by haskell.nix clones only the subset of a
repository corresponding to HEAD. So if we have a workflow that has a
branch "development" as HEAD, but the stack.yaml only targets commits
from "master", then it will be unable to fetch any commit.

Fixes #592

@michaelpj
Copy link
Collaborator

So, I was actually hoping we could get away from the magic comments. We have the sha256map argument for looking up the shas for revisions, perhaps we could do something similar to pass this information? That has the advantage of not needing to change nix-tools, it would be a haskell.nix only change.

@michaelpj
Copy link
Collaborator

A minimal solution might be a lookupBranch argument next to lookupSha256.

@ismaelbouyaf
Copy link
Contributor Author

Thanks for your hints @michaelpj . I understand the move, I’ll try to have a look at that. I remember seeing this sha256map in haskell.nix, I shall try to adapt it for branches.

This is the counterpart of nix-tools proposed change:
input-output-hk/nix-tools#99
@ismaelbouyaf
Copy link
Contributor Author

I implemented the suggested change. Now the change in nix-tools is not needed anymore 😌

@michaelpj
Copy link
Collaborator

This should close #592, also.

@immae
Copy link

immae commented Jan 28, 2021

This should close #592, also.

Thanks for the digging of issues, I didn’t do my part of the job before submitting it seems 🙇

@hamishmack
Copy link
Collaborator

bors try

iohk-bors bot added a commit that referenced this pull request Feb 2, 2021
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 3, 2021

@hamishmack hamishmack merged commit 4729034 into input-output-hk:master Feb 3, 2021
@arianvp
Copy link
Contributor

arianvp commented Feb 3, 2021

I just tried out this change on latest master; but this does not fix issue #592 for me.

fatal: not a tree object: 0a5142cd3ba48116ff059c041348b817fb7bdb25

using:

extra-deps:
# https://github.com/hspec/hspec-wai/pull/49
# nix-branch: body-contains
- git: https://github.com/wireapp/hspec-wai
  commit: 0a5142cd3ba48116ff059c041348b817fb7bdb25

Edit:
But maybe I'm misunderstanding how to use it. As I see you moved away from the magic-comment approach. Where exactly should I add the branch information?

@immae
Copy link

immae commented Feb 3, 2021

@arianvp : the "nix-branch comment hack" was discussed and discarded finally, because it doesn’t follow the way that haskell.nix is planned to evolve.
The final version would not require a change of your stack.yml, but rather the way you call haskell.nix:

pkgs.haskell-nix.project {
    (... other fields);
    branchMap = {                                                                                                       
      "https://github.com/wireapp/hspec-wai" = "body-contains";
    };                                                                                                                  
    lookupBranch = { location, ... }: branchMap."${location}" or null;                                                  
}

the modification of the lookupBranch is because the default implementation also looks at the wanted commit.

pkgs.haskell-nix.project {
    (... other fields);
    branchMap = {                                                                                                       
      "https://github.com/wireapp/hspec-wai"."0a5142cd3ba48116ff059c041348b817fb7bdb25" = "body-contains";
    };                                                                                                                  
}

Hope it’s clearer that way?

booniepepper pushed a commit to booniepepper/haskell.nix that referenced this pull request Feb 4, 2022
The builtins.fetchGit used by haskell.nix clones only the subset of a
repository corresponding to HEAD. So if we have a workflow that has a
branch "development" as HEAD, but the stack.yaml only targets commits
from "master", then it will be unable to fetch any commit.

Fixes input-output-hk#592
@parsonsmatt
Copy link
Contributor

Commenting from way in the future, I guess -

The entire selling point of haskell.nix (to me) is that I can use cabal.project or stack.yaml as the source of truth, and I don't have to know or teach nix so that devs can update packages.

Requiring devs to update the nix configuration for something this simple entirely breaks that encapsulation. I'd have strongly prefered the magic comment. Even just syntactically, it's much nicer for my diff to look like:

 -- https://github.com/haskell-servant/servant/pull/1592
 source-repository-package
     --sha256: sha256-DcgeE37DJy6+sUvkbKdnk1Cy9LyATgPHaPZ0ifj/qtg=
+    --nix-branch: ghc-9.4
     type: git
     location: https://github.com/TeofilC/servant.git
     tag: a53d69929cf6fe531bb25aec65e7ab6405278459
     subdir:
         servant
         servant-client
         servant-client-core

than this much noisier and heavily duplicated thing:

+    branchMap = {
+      "https://github.com/TeofilC/servant.git"."a53d69929cf6fe531bb25aec65e7ab6405278459" = 
+        "ghc-9.4";
+    };

Additionally, we now have a non-local addition to the source-of-truth for the build.

OTOH, I understand that as a maintainer, the magic comments are likely Worse. But as a user, the UX of just putting a magic comment in the relevant definition for the in the source of truth for the package. I don't expect y'all to cater your project just to me opinons, but I do want to provide some user feedback on the design decision here.

@immae
Copy link

immae commented Sep 13, 2022

@parsonsmatt I agree on the idea, but the difference here comes from git: since it is not possible to "fetch" (in the git sense) only a commit from a repository, nix and stack have a different way of working around that:

  • stack fetches the whole repository, so the commit you’re looking for is necessarily there
  • nix only fetches the HEAD aka default branch (or a specific branch if specified, which was the subject of the change of this PR), and only the last n (1000 IIRC) commits, so if your commit is not in that list then nix is unhappy

so the only ways of getting what you want (which, again, is a reasonnable wish) is

  • fix git (so that you can "git fetch" only a commit. I think there was work on that)
  • fix nix (to fetch the whole repository, whatever it costs. Again I think there are "some" improvements on that)
  • replace git references in your stack.yml with urls (possible with github, gitlab and most git "web" frontends), either in the stack.yml file directly or by hacking it in your nix file (requires a bit of work but it’s only a oneshot then, you won’t need to remember to update the branchMap anymore)

@immae
Copy link

immae commented Sep 13, 2022

(NB: @ismaelbouyaf and I are the same person, the former one was a company account to which I lost access when the company terminated)

@michaelpj
Copy link
Collaborator

The entire selling point of haskell.nix (to me) is that I can use cabal.project or stack.yaml as the source of truth, and I don't have to know or teach nix so that devs can update packages.

This is a very reasonable desire, but it's slightly different to what I think our working goal is. Our goal is to follow the project configuration as the source of truth so that we do not diverge from it, not so that it can be the only thing you need to touch to configure your project. We are quite far from that. So that's just to explain: I don't think we're optimizing for the same thing.

And aside from that... personally I have the opposite feeling. I want my cabal.project to just be a normal cabal.project and not littered with weird magic for nix. The nix configuration can live in the nix code, thank you. But then I think we just have different preferences.

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.

stackProject git deps fail when rev is not in default branch
6 participants