Can we please change this script so that it only accepts generating a version out of the data in the git tree?
Here is the rationale polished from comments made recently in a PR:
We want the source code to know its version, instead of expecting the person that builds the tarball to keep its version in mind and pass it in via a parameter. That latter approach is error prone will definitely result in binaries reporting their version incorrectly, because one can be updated without the other. Besides providing information wrong, this can easily result in major bugs because there’s logic inside snapd that takes those versions into account.
With that background, we don’t want environment variable, positional parameters, or flag parameters that provide a version into this script. We want this script to tell the version based on the git data, and we want this version to be statically saved into the filesystem so that it can be packed.
If we have vendor tarballs that do not contain their own version today, these tarballs are broken and need to be fixed ASAP so that they hold their version correctly. Let’s please fix any such broken tarballs now, while it’s early and we haven’t suffered any major pain due to that.
I completely disagree with this. This assumes that the version-setting wouldn’t be automated. In my case, it quite literally comes from the version declaration for the whole package. It can’t be wrong or otherwise it wouldn’t build anyway.
It’s flat-out dangerous for mkversion.sh to be pulling in random Git data, as that assumes the Git data is sane, which everyone knows it is not. Git is highly mutable and all kinds of problems can come up with using Git data.
I also do not use the vendor tarball because I don’t want the vendored source tree. I get my tarballs from GitHub using the tar.gz.
If you rip this out, I will patch the functionality back in, because I want to be able to declare my package’s version as the snapd version. That information is useful from your side so that you can map what users are using to errors/issues. If you aren’t already using the version information in this manner, then you should just remove the whole version requirement thing and just set it immutably in the source tree, because it’s pointless otherwise.
The fact we’re discussing this openly and that I invited you to this conversation is hopefully enough proof that I’m trying to solve a problem without actually causing more problems.
I’d appreciate if you could read my message again under those eyes, and collaborate with me on trying to solve the actual problem I described instead of just gut-reacting to the action I suggested.
In terms of trusting the git metadata, we pretty obviously need to trust it to a huge degree otherwise we can’t use it. If you have specific issues you’d like to bring up about our use of git, then please raise them so we can also talk about those.
That will initialize git and apply patches through git am. That will mess things up. Or if someone deploys from a branch that’s not on snapcore/snapd. Or any other thing. The nature of Git means you cannot trust it for this information. The fact that people do is a failing in itself.
If you want to have enforced versioning, you should just write the version into the source tree. People have been doing this forever in other projects, there’s literally no reason we shouldn’t either.
In any case, I’ve wanted for a while to have the vendoring overlay be a separate tarball from the main snapd tarball, but that’s another matter and I don’t expect that to happen anytime soon.
