fetchFromGitHub: force re-fetch when rev changes #11
No reviewers
Labels
No labels
Compat
Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: auxolotl/core#11
Loading…
Reference in a new issue
No description provided.
Delete branch "fetchgithub-refetch"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Since we have yet to hit the ground running, we have a golden opportunity to fix some of those long-standing issues in upstream that don't get fixed to avoid disturbing the whole project. Specifically, this is the fact that
fetchFromGitHub
currently outputs a FOD with a very generic name. As a result, if a maintainer changes a version number but forgets about the output hash, it may not get caught. Error messages are also less useful. If we can fix that we will have much better DX moving forward.To illustrate: I tried to rebuild
nixdoc
to see if it was working and I immediately caught a stale hash.Based on https://github.com/NixOS/nixpkgs/pull/294068
Is this https://github.com/NixOS/nixpkgs/commit/c3255fe8ec32#commitcomment-25289921, not a concern which lead the original PR maker to create https://github.com/NixOS/nixpkgs/pull/294329
This is true and those two commits are already included in the first PR as well as the present PR.
This change does not by itself invalidate all hashes. I checked the
python3Packages.build
hash and it was really stale to begin with.Removing the fix hash commit to avoid conflicts
TBH, I'd also change
source
to something likegithub-repo
, just to make it clearer in the name that the derivation comes from a GitHub checkout.nit:
Any specific reason for
overrideAttrs
over//
?Here's a slightly different proposal: append the github base url as provenance like so:
/nix/store/wzvnh3p7gv1qlqhb3iif878bfffzrvaz-source-pypa-build-refs-tags-1.1.1-github.com.drv
. That's a good self-description, leaves thesource
that people might be used to, and generalizes unambiguously to otherfetchFrom
s. We can also put it before theowner
.It's not super clear to me. The original author explains it as:
https://github.com/NixOS/nixpkgs/pull/294329
It's true that if someone re-overrides after the
//
, the meta will be lost. However, sincefetchFromGitHub
overridesoverrideAttrs
anyway (withmakeOverridable
) I don't think this can happen in practice. Better safe than sorry?@ -5,1 +4,4 @@
{ owner, repo, rev
, name ? null # Override with null to use the default value
, pname ? "source-${githubBase}-${owner}-${repo}"
, fetchSubmodules ? false, leaveDotGit ? null
/nix/store/f291banfgj2vjz1qsvmpm09awn2gl4pj-source-github.com-pypa-build-refs-tags-1.1.1.drv
wdyt @Sorixelle ?Only potential issue I see is creating some pretty long path names, but I don't see it being a massive issue. Seems fine to me.
Mhmm. Sounds good to me.