search

adaszko / complgen

Declarative bash/fish/zsh completions without writing shell scripts
Apache License 2.0
221 stars 7 forks source link

Release 0.2.0 introduces breaking changes when git is not present #52

Closed meator closed 3 months ago

meator commented 3 months ago

Hey, I was updating my complgen package to the newest version and I've noticed that the build now fails for me. The error message is unfortunately pretty cryptic, but through some debugging, I have figured out the cause:

https://github.com/adaszko/complgen/blob/master/build.rs#L5

This line is accompanied by the following comment:

// note: add error checking yourself.

I am not sure whether I understand it correctly. Are you saying that it is the user's responsibility to add error checking?

I do not have experience with Rust, but I have already come across this issue (making the program aware of this version) in other languages. The "standard" way to handle this is to use revision control to retrieve program's version, but provide a sensible fallback when revision control fails for whatever reason. A good choice for the fallback would be the project's version set in the Cargo.toml file, but that is unfortunately out of date: #51

git may not be always available. The code could have been downloaded through a release archive (which many package builders do) instead of cloning.

I also use git describe in a program I maintain. I have it set up so that it prints the same version (for example r3.0) as the fallback version when the repo is checked out to a tag. If the HEAD of the repository isn't tied to a tag, git-describe prints the usual commit hash info. This means that it doesn't matter whether someone downloads a release of my program through git clone or through the release archive, printed version will be the same.

I also believe that this breaking change should be mentioned in the release notes for v0.2.0 it it's intentional.

adaszko commented 3 months ago

Fair points! I added a fallback and more descriptive warning message.

I also believe that this breaking change should be mentioned in the release notes for v0.2.0 it it's intentional.

This isn't something that was introduced in 0.2.0. I believe you just happened to stumble on it at 0.2.0.

meator commented 3 months ago

Thanks for the quick fix!

Fair points! I added a fallback and more descriptive warning message.

The new solution still isn't ideal. complgen will at least compile from release archives, but having the fallback version be literally "fallback" is bad. Package maintainers will have to patch out this logic if they would want to provide a more sensible package version themselves. I am not familiar with the Cargo build system, but I believe it could be possible to use the project's Cargo version as fallback. If the Cargo version is accurate (which might not be the case), this would still provide accurate version information even in environments where git cannot be used.

If that isn't possible, it would be nice if there was at least a way to override the version without patching the code.

I also believe that this breaking change should be mentioned in the release notes for v0.2.0 it it's intentional.

This isn't something that was introduced in 0.2.0. I believe you just happened to stumble on it at 0.2.0.

This issue is not present in the v0.1.8 release.

adaszko commented 3 months ago

Added the fallback to CARGO_PKG_VERSION 👍