velero: 1.5.4 -> 1.6.0

[j4m3s-s]

Motivation for this change

Velero update

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Determined the impact on package closure size (by running nix path-info -S before and after)
• Ensured that relevant documentation is up to date
• Fits CONTRIBUTING.md.

[j4m3s-s]

I see a GitSHA in the buildFlagsArray. Shouldn't be set to the commit of the last version ? Looking at the other updates PR, it hasn't been updated. But maybe it should've been ?

[SuperSandro2000]

I see a GitSHA in the buildFlagsArray. Shouldn't be set to the commit of the last version ? Looking at the other updates PR, it hasn't been updated. But maybe it should've been ?

Yeah and it should be probably be derived from a variable instead of being in the build instructions and have a comment next to the version number to bump it, too.

[bryanasdev000]

Yes, I talked about doing it automatically, at the time I checked packages like K9S and Velero (before adopting it) and others and everyone was without or hardcoded as I did. The last update PRs were all done by bot so I only had to make a mental note of it to update later, but it seems I was slow lol

I am open to understand how to improve.

Looking at the code it seems OK, I will test and report.

[bryanasdev000]

Working & LGTM.

[j4m3s-s]

Not sure that's the right way to do it.
Also, why do some packages in Go have this ? Is this related to inner working of the software or is there any other reason to do it?
I'm a maintainer on some other Go packages, so if I missed anything it'd be nice to know.

[bryanasdev000]

Not sure that's the right way to do it.
Also, why do some packages in Go have this ? Is this related to inner working of the software or is there any other reason to do it?
I'm a maintainer on some other Go packages, so if I missed anything it'd be nice to know.

As far as I know they are purely "cosmetics", I believe it serves for bug reports or something like a form of tracking, so you have not only the release but the commit that generated that binary, in a way it helps in reproducibility.

At least that's what I understood by seeing codes like Velero, Popeye, K9S and others.

I also do not know at the moment a way to deal with this automatically, in a way it is information that Nix has access to when it downloads a certain release, it is the last commit for that release.

It might not be worth it to bump into it.

@SuperSandro2000 do you know any way to access this type of information from Git during build?

[bryanasdev000]

LGTM. Thanks.

[tomberek]

@bryanasdev000 is that GitSHA needed for any functionality or output? I took a quick look and it seems it does impact the output of GetServerStatus and some versioning output: https://github.com/vmware-tanzu/velero/blob/e9c997839ebbb04688a88ffb981eeb74de51b380/pkg/cmd/cli/version/version_test.go#L53

I was hoping the sha would be available in the src attribute, but didn't find it.

[bryanasdev000]

@bryanasdev000 is that GitSHA needed for any functionality or output? I took a quick look and it seems it does impact the output of GetServerStatus and some versioning output: https://github.com/vmware-tanzu/velero/blob/e9c997839ebbb04688a88ffb981eeb74de51b380/pkg/cmd/cli/version/version_test.go#L53

I was hoping the sha would be available in the src attribute, but didn't find it.

As far as I know, no, I also inspected the code looking for this type of information and did some tests without it, I had no problems without it.

In the end I believe that we can really go without it or demonstrate the commit as in K9S (https://github.com/NixOS/nixpkgs/blob/master/pkgs/applications/networking/cluster/k9s/default.nix#L18) using the src.rev .

Informing the version is enough and avoids headaches due to the use of different versions between server and client.