jqlang / jq

Command-line JSON processor
https://jqlang.github.io/jq/
Other
29.59k stars 1.54k forks source link

return version even if not using git sources #3099

Open jonathanspw opened 2 months ago

jonathanspw commented 2 months ago

Downloading the source tarball from GitHub lacks .git structure and the version will be blank. This fixes it by using the directory name which contains the version.

This is currently impacting Fedora packaging, and potentially other distros if they use GitHub source tarballs.

Ref: https://bugzilla.redhat.com/show_bug.cgi?id=2277743

wader commented 2 months ago

Hi, could the problem be that the build downloads the github auto generated tag artifact "Source code (tar.gz)" instead of the proper source distribution "jq-1.7.1.tar.gz"?

$ curl -sL https://github.com/jqlang/jq/releases/download/jq-1.7.1/jq-1.7.1.tar.gz | tar t | grep version.h
jq-1.7.1/src/version.h
$ curl -sL https://github.com/jqlang/jq/archive/refs/tags/jq-1.7.1.tar.gz | tar t | grep version.h
$
jonathanspw commented 2 months ago

Yes, "Source code (tar.gz)" is precisely what we use for sources in the Fedora package.

wader commented 2 months ago

Ok, could you try use "jq-1.7.1.tar.gz" instead? the other one is automatically added by github and it is currently not possible to disable or hide :(

jonathanspw commented 2 months ago

Is there any real reason to do so other than this bug? Generally speaking I try to use the auto generated source tarballs where possible, and other than this issue, it works fine for jq.

wader commented 2 months ago

I see, no reason really, so a fallback would be fine with me. Let's see what @nicowilliams, @itchyny other maintainers think about it.

jonathanspw commented 2 months ago

I believe even if using those sources that an autoreconf might wipe out the version stuff, so I'm going to test that and potentially expand this fallback to properly support directory names jq-<version> and jq-jq-<version> if necessary.

Please do not merge yet.

wader commented 2 months ago

👍 no worries, will wait for at least one more maintainer agrees and has reviewed

jonathanspw commented 2 months ago

The issue indeed exists even in the CI-generated tarballs that if you autoreconf for whatever reason it wipes out the version info.

$ ./jq --version
jq-

Committed a fix to deal with this as well, just updated regex.

wader commented 2 months ago

If you fix that i guess we can also remove the "skip the autoreconf step" from https://github.com/jqlang/jq?tab=readme-ov-file#instructions?

jonathanspw commented 2 months ago

We do autoreconf in Fedora without any other known side effects, so without it breaking the version stuff I guess it should be safe.

itchyny commented 2 months ago

The auto-generated tarball does not include the oniguruma submodule, so it requires system oniguruma library for regex filters support. Since we distribute archives (both tarball and zip) for distributors including the version and the oniguruma submodule (version of which we test), we recommend to use them. Also, using the directory name for version hint looks a bit weird to me. And /bin/sh is not always Bash.