nixpkgs-review result

Generated using nixpkgs-review-gha

Command: nixpkgs-review pr 418147

Logs: https://github.com/guylamar2006/nixpkgs-review-gha/actions/runs/15758131262

nixpkgs-review result

Generated using nixpkgs-review-gha

Command: nixpkgs-review pr 418147

Logs: https://github.com/liberodark/nixpkgs-review-gha/actions/runs/15758223202

I always prefer rev over tag since a tag can be reassigned to another commit. Are you sure it's okay from security perspective?

I always prefer rev over tag since a tag can be reassigned to another commit. Are you sure it's okay from security perspective?

Yes, let's say you create a tag that has the same value as a revision. In that case, doing

rev = version;

or

tag = version;

Will not be different, and that is clearly an issue. This is why, people started to use:

rev = "refs/tags/${version}";

To explicitly use a tag and not a revision.

This is precisely why the tag attribute has been created. It's actually a shortcut to ref = refs/tags/${value};, e.g.:

I always prefer rev over tag since a tag can be reassigned to another commit. Are you sure it's okay from security perspective?

Yes, let's say you create a tag that has the same value as a revision. In that case, doing

rev = version;

or

tag = version;

Will not be different, and that is clearly an issue. This is why, people started to use:

rev = "refs/tags/${version}";

To explicitly use a tag and not a revision.

This is precisely why the tag attribute has been created. It's actually a shortcut to ref = refs/tags/${value};, e.g.:

nixpkgs/pkgs/build-support/fetchgithub/default.nix

Line 110 in dfcf351

revWithTag = if tag != null then "refs/tags/${tag}" else rev;

thank you for explaining! but changes in this PR are rev -> tag, not ref = ref/tags/ -> tag =. I understand the convenience this changes providing, but still I think keeping rev makes more sense due to the fact that upstream can reassign tags, but not rev.

Yes, but here the revs are tags actually, so, what's in this PR is legit.

Yes, but here the revs are tags actually, so, what's in this PR is legit.

I really doubt that this is a good move. in fact one of the reasons I like nixos is that it's encouraging using hashes to refer to source codes. is there any documentation or design behind these changes?

I forgot to mention that this move is on the opposite direction of forcing reproducibility principle! reproducibility was the goal of nix and as far as i know it still is. if we go to this path we basically will add a new class of bugs, which i use nix to prevent them!

Could you please make your point? I still don't get it.

Why would you prefer:

rev = version;

over

tag = version;

?

Could you please make your point? I still don't get it.

Why would you prefer:

rev = version;

over

tag = version;

?

I checked the changes again. All of them have sha-256 hash, so I guess I am okay! thank you for bearing with me! 😄

I checked the changes again. All of them have sha-256 hash, so I guess I am okay! thank you for bearing with me! 😄

That's precisely wrong. If the version attribute is not a human readable version (aka, a hash), then switching to tag is wrong. Can you please clarify again? I guess you meant that all the versions attributes are human readable tags (aka, not a git commit id)?

That's precisely wrong. If the version attribute is not a human readable version (aka, a hash), then switching to tag is wrong. Can you please clarify again? I guess you meant that all the versions attributes are human readable tags (aka, not a git commit id)?

yes I confirm the versions are human readable, but what makes me sure the tags didn't move is the existence of sha-256 beside the tags. I already approved. 🙂

yes I confirm the versions are human readable, but what makes me sure the tags didn't move is the existence of sha-256 beside the tags. I already approved. 🙂

Of course, but a hash is mandatory if you have a rev as well! :)

nixpkgs-review result

Generated using nixpkgs-review-gha

Command: nixpkgs-review pr 418147

Logs: https://github.com/JohnRTitor/nixpkgs-review-gha/actions/runs/15807346321