Looks good. Take your time. I won't get to it, but I will at least timely merge it.

The test passes but there may be some things that the tests didn't catch (eg. 3f6932b), I will try to find these later.

The fetchers still use only rev (or only version_number sometimes?), adding tag support for them is out of the scope of this PR. This leaves the code base in a bit inconsistent state where sometimes we check version.rev or version.tag when the version came from the nix eval, but for versions coming from fetchers we still only do new_version.rev.

Another weird thing I found is: this might be already fixed in nixpkgs/master, I couldn't reproduce my issue. I'll investigate soon.
Edit: Apparently after the fetchgit tag PR was merged, nix-update was not crashing, I was on a wrong base when I tested it. However if we only update the lockfile, the commit message has issues like saying the diff is from None to a version.

I did some refactors to make it clearer when we are talking about a "rev or a tag" for the old/not-updated package and called it old_rev_tag. I think I caught all instances that previously checked for a rev (excluding versions coming from fetchers as explained above).

I think this is ready, but more manual testing wouldn't hurt :)

@gepbird Can you add both tag and rev support?

For example:

{
  buildGoModule,
  fetchFromGitHub,
  lib,
  nix-update-script,
}:
buildGoModule rec {
  pname = "lipo-go";
  version = "0.9.2";
  env = {
    GIT_VERSION = version;
    GIT_REVISION = "b7b34565565e3cde8037d1b5ee95dd2bb3579ef1";
  };
  src = fetchFromGitHub {
    owner = "konoui";
    repo = "lipo";
    tag = "v${version}";
    rev = env.GIT_REVISION;
    hash = "sha256-FW2mOsshpXCTTjijo0RFdsYX883P2cudyclRtvkCxa0=";
  };

  passthru.updateScript = nix-update-script { };

  vendorHash = "sha256-7M6CRxJd4fgYQLJDkNa3ds3f7jOp3dyloOZtwMtCBQk=";

  postPatch = ''
    # remove the test that requires access permit to /bin
    sed -i '/bin := filepath.Join/a info, err := os.Stat(bin);if err != nil || info.Mode().Perm()&0444 != 0444 { continue }' pkg/lipo/archs_test.go
  '';

  buildPhase = ''
    make build VERSION=$GIT_VERSION REVISION=$GIT_REVISION BINARY=$out/bin/lipo
  '';

  meta = {
    description = "This lipo is designed to be compatible with macOS lipo, written in golang.";
    homepage = "https://github.com/konoui/lipo";
    license = lib.licenses.mit;
    maintainers = with lib.maintainers; [ xiaoxiangmoe ];
  };
}

@xiaoxiangmoe can you explain that use case? I don't see a reason for using both, because if they differ, which should be fetched?

And there's an eval failure on (a 4 day old) nixpkgs:

       error: fetchFromGitHub requires one of either `rev` or `tag` to be provided (not both).                                                                                                                    

Some app build will need both GIT_VERSION and GIT_REVISION for log in sentry, show version and so on. So we need get both tag and rev.

For example https://github.com/konoui/lipo

./result/bin/lipo -version                                       
0.9.2/b7b34565565e3cde8037d1b5ee95dd2bb3579ef1

error: fetchFromGitHub requires one of either `rev` or `tag` to be provided (not both).    

I think we should support both for it.

Maybe someday tag will change (there are precedents for this in history) and rev is always immutable and relyable.

If tag and rev is not same, we can put a warning for users.

Here is another example
https://github.com/NixOS/nixpkgs/blob/a3484e1559a2e63670aef012b0afd1217a9e011b/pkgs/by-name/af/affine/package.nix#L46

I can't use nix-update-script because I can't update version with GITHUB_SHA both. And sentry relies on the env GITHUB_SHA.

Some app build will need both GIT_VERSION and GIT_REVISION for log info, show version and so on. So we need get both tag and rev.

For example konoui/lipo

./result/bin/lipo -version                                       
0.9.2/b7b34565565e3cde8037d1b5ee95dd2bb3579ef1

In this package some REV (COMMIT) and TAG is being patched from the version and src.rev attributes. Or this go package seems like a better example that overrides the commit (rev) and version (tag).

error: fetchFromGitHub requires one of either `rev` or `tag` to be provided (not both).    

I think we should support both for it.

Maybe someday tag will change (there are precedents for this in history) and rev is always immutable and relyable.

If tag and rev is not same, we can put a warning for users.

I doubt it would change, any fetcher like fetchFromGitHub should only have one source, either a tag (that is just a named rev) or a rev. And validating whether the rev and the tag is the same would add extra overhead.

Here is another example NixOS/nixpkgs@a3484e1/pkgs/by-name/af/affine/package.nix#L46

I can't use nix-update-script because I can't update version with GITHUB_SHA both. And sentry relies on the env GITHUB_SHA.

This makes sense, but seems like a rare use case and implementing it wouldn't be easy, so it's out of the scope of this PR, sorry. After fetching the new version, we'd need to find the related hard coded commit SHA (this step would probably lead to a lot of false positives and breaking packages), then fetch the new commit SHA and replace the old one.

If you really need to update the commit SHA automatically, you can write an update script that utilises nix-update to update the version, src.hash and vendorHash, then you can probably obtain the commit SHA with GitHub's API and replace it with sed.

Feel free to open an issue on nixpkgs or start a discourse thread for help with packaging or making the update script and ping me :)