leil-io / saunafs

SaunaFS is a free-and open source, distributed POSIX file system inspired by Google File System.
https://saunafs.com
GNU General Public License v3.0
62 stars 5 forks source link

chore: Change how PACKAGE_VERSION string is build #232

Open uristdwarf opened 1 month ago

uristdwarf commented 1 month ago

Git commits are not guaranteed to be available or reliable on build time (for example, some packaging systems prefer without a VCS or have their own image of the repository with their own commits). Build time may be useful, but it should not be part of the PACKAGE_VERSION, instead it should be generated seperately and used seperately when -v is called. Also, the version should not depend on an outside variable, the CMakeLists.txt file should be the final source for this.

This commit removes the above git commit and build time variables (build time will be re-introduced in another commit) and simplifies the version string, with one exception: A compile time option called VERSION_SUFFIX, which can be used to identify the build (e.g, 'unofficial', 'dev', 'rc' or nothing at all). By default this will be dev when building from source, or 'unofficial' when using ./configure. Package maintainers may use their own suffix to distinguish building processes or add additional version information.

uristdwarf commented 1 month ago

This will be included in rc3 of v4.6.0

ralcolea commented 1 month ago

I agree with @lgsilva3087. Key information about the packages, such as the commit, should be available in the package name or through an alternative method. This information is essential, particularly for supporting customers, and should not be removed.

uristdwarf commented 1 month ago

You are dropping very useful information for debugging.

I do agree we should preserve some of the info, e.g build date, that is useful for differentiating same versions with different builds.

I agree with @lgsilva3087. Key information about the packages, such as the commit, should be available in the package name or through an alternative method. This information is essential, particularly for supporting customers, and should not be removed.

However we can't rely on git commits for a couple of reasons:

  1. They may only download the source tar, with no git. Even if they get it through git, they may wish to remove the VCS (e.g Debian does this), so what we end up is an empty string.
  2. Even if they do have a git commit, it may very well wrongly point to some other repository, debian for example tracks source code with git also, but they have their own different repostiory

https://salsa.debian.org/med-team/neo https://salsa.debian.org/med-team/neo/-/commits/master?ref_type=heads Compare to upstream https://github.com/NeuralEnsemble/python-neo/commits/master/

  1. Even if they include the right commit (perhaps through a CMake definition), there's no guarantee this commit will be 1:1 with the source code in this repository, as there may have been patches added to it. Example from above

https://salsa.debian.org/med-team/neo/-/tree/master/debian/patches?ref_type=heads

But also see LizardFS

https://salsa.debian.org/debian/lizardfs/-/tree/master/debian/patches?ref_type=heads

This last rule also applies to our own customers, as we may wish to apply customized patches without hosting them here (e.g branding related).

In short, there's no guarantee the source code is not tampered with during the build process, making the commit pointless in terms of reproduction. You need either the binary(ies) or the build process. This suffix should allow builders to distinguish themselves so it can be figured out how it was build (and from what source with what patches) if needed.

Maybe the information should be added to the --version option before dropping it.

I agree that some info should be preserved. This was the minimum needed for the new deployment process (distinguish builds between dev, staging and production builds) but -v should produce the build time at least. I'll mark it as a draft.

While we are discussing this, I think it should also include some of the compile time definitions, e.g those related to Judy, Prometheus and other stuff.

'dev' is a very generic suffix that could be interpreted as it was generated from the 'dev' branch

Perhaps source would be better?

uristdwarf commented 1 month ago

I could add CMake definition to optionally include a commit hash that we use in our official (and unofficial) builds

lgsilva3087 commented 1 month ago

You are dropping very useful information for debugging.

I do agree we should preserve some of the info, e.g build date, that is useful for differentiating same versions with different builds.

I agree with @lgsilva3087. Key information about the packages, such as the commit, should be available in the package name or through an alternative method. This information is essential, particularly for supporting customers, and should not be removed.

However we can't rely on git commits for a couple of reasons:

  1. They may only download the source tar, with no git. Even if they get it through git, they may wish to remove the VCS (e.g Debian does this), so what we end up is an empty string.
  2. Even if they do have a git commit, it may very well wrongly point to some other repository, debian for example tracks source code with git also, but they have their own different repostiory

https://salsa.debian.org/med-team/neo https://salsa.debian.org/med-team/neo/-/commits/master?ref_type=heads Compare to upstream https://github.com/NeuralEnsemble/python-neo/commits/master/

  1. Even if they include the right commit (perhaps through a CMake definition), there's no guarantee this commit will be 1:1 with the source code in this repository, as there may have been patches added to it. Example from above

https://salsa.debian.org/med-team/neo/-/tree/master/debian/patches?ref_type=heads

But also see LizardFS

https://salsa.debian.org/debian/lizardfs/-/tree/master/debian/patches?ref_type=heads

This last rule also applies to our own customers, as we may wish to apply customized patches without hosting them here (e.g branding related).

In short, there's no guarantee the source code is not tampered with during the build process, making the commit pointless in terms of reproduction. You need either the binary(ies) or the build process. This suffix should allow builders to distinguish themselves so it can be figured out how it was build (and from what source with what patches) if needed.

Maybe the information should be added to the --version option before dropping it.

I agree that some info should be preserved. This was the minimum needed for the new deployment process (distinguish builds between dev, staging and production builds) but -v should produce the build time at least. I'll mark it as a draft.

While we are discussing this, I think it should also include some of the compile time definitions, e.g those related to Judy, Prometheus and other stuff.

'dev' is a very generic suffix that could be interpreted as it was generated from the 'dev' branch

Perhaps source would be better?

I was refferring all the time only to our official releases.

uristdwarf commented 1 month ago

No git commit/branch:

image

With git commit/branch

image

The git commit will be extracted in the package.sh script for our own packages, but I will do this when these commits are approved

This will need to be provided with the -DGIT_COMMIT or -DGIT_BRANCH

I've also added missing version options to the rest of the binaries.