-
Notifications
You must be signed in to change notification settings - Fork 248
Determine if the tag is a ref or a rev #1665
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
Determine if the tag is a ref or a rev #1665
Conversation
Test the `tag` attribute in the `cabal.project` file. If it looks like a git hash, then assign it to `rev`. Otherwise, assign it to `ref`.
Some notes while I was digging around
Goal: Provide the branch name in a
|
d6a53db
to
8bbf7dc
Compare
lib/call-cabal-project-to-nix.nix
Outdated
@@ -228,9 +228,9 @@ let | |||
then throw "${inputMap.${repoData.url}.rev} may not match ${repoData.ref} for ${repoData.url} use \"${repoData.url}/${repoData.ref}\" as the inputMap key if ${repoData.ref} is a branch or tag that points to ${inputMap.${repoData.url}.rev}." | |||
else inputMap.${repoData.url}) | |||
else if repoData.sha256 != null | |||
then fetchgit { inherit (repoData) url sha256; rev = repoData.ref; } | |||
then fetchgit { inherit (repoData) url sha256 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.
What happens here if we have a repoData.ref
but no repoData.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.
Good question!
Looks like fetchgit
's rev
attribute can either be a commit or a ref
-like attribute, so I should do rev = repoData.rev or repoData.ref
instead of inheriting.
lib/call-cabal-project-to-nix.nix
Outdated
else | ||
let drv = builtins.fetchGit { inherit (repoData) url ref; }; | ||
let drv = builtins.fetchGit { inherit (repoData) url ; rev = repoData.rev or repoData.ref; ref = repoData.ref or null; }; | ||
in __trace "WARNING: No sha256 found for source-repository-package ${repoData.url} ${repoData.ref} download may fail in restricted mode (hydra)" |
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.
This __trace
that uses ${repoData.ref}
, is that still safe? Should it use ${repoData.rev or repoData.ref}
?
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.
Good catch. I think a complication is that we may have a ref and a rev (which is part of what this is intending to allow). ie, I was trying to fetch a repo with no master
, so it didn't fetch the right branch because ref
was specified to be something else.
I'll update the trace message to include both.
bors try |
tryBuild succeeded: |
Test the
tag
attribute in thecabal.project
file. If it looks like a git hash, then assign it torev
. Otherwise, assign it toref
.This should fix the problem where
source-repo-override
cannot be used to set arev
orref
, which blocks #1663 from usingsource-repo-override
.