Automatically determine Autoconf package version from cmake package version #1663
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
It seems to me you are pretty good at auto-tools. Just wondering if you can help to fix the issue with libreadline status as well. I tried to fix it last time but it caused issues with macOS terminal mode and I have to revert the following PR. |
|
Other than that, there are other issues as well. Example build log under MSYS2 mingw64. You can see there are many warnings. |
|
But no issues if you do not have the time and only fix the version number issue for avrdude 7.3 release. It is just my wish that you can help fix some of the issues with the auto-tools scripts. |
|
I do have plans for the Autotools buildsystem, but I did not want to distract anybody from the 7.3 release you had already planned out before I started pushing PRs to you a couple of weeks ago. Automatic version number possibly for 7.3, and everything else after 7.3 would be my preferred option. |
This is perfectly fine. Thanks a lot for the help. |
|
Post 7.3 items created: |
ndim
force-pushed
the
autoconf-package-version
branch
from
c63abfa
to
f76afb7
Compare
|
Ideally, this should be done now. I will run some tests later on different operating systems before I declare this "Ready for review" and for merging. I have VMs with Debian, Ubuntu, Arch, FreeBSD in addition to my Fedora development system to test on:
The test procedure is basically: set -e
if test -f src/configure; then grep ^PACKAGE_VERSION= src/configure; fi
autoreconf -vis src
grep ^PACKAGE_VERSION= src/configureOr with a few more checks set -e
if test -f src/configure; then
mkdir _b1
cd _b1
../src/configure
cd ..
fi
autoreconf -vis src
mkdir _b2
cd _b2
../src/configure |
ndim
force-pushed
the
autoconf-package-version
branch
2 times, most recently
from
df2dbad
to
5ed2201
Compare
|
I can test under Windows (MSYS2 mingw64), macOS (Homebrew) and Linux (Ubuntu 20.04). I can also test under FreeBSD/OpenBSD/NetBSD if necessary. |
|
BTW, I am not sure whether you actually want the added CI build. It only checks the consistency of the Autotools build system. I could not easily get the test script to work yet without investing more than a few cursory glances into it. |
To save the efforts and speed up avrdued 7.3 release, I will suggest we do not add this to CI first. We can do this post 7.3 release. |
|
The version number here is a bit different from CMake build -- no git commit ID info. git main CMake build has the data and git commit ID. @ndim Is it possible for you to follow CMake build as well? Thanks. But if it is a bit troublesome, we can live with this. |
|
|
|
Build log of the resulting tarball from |
The current This PR (as of commit 5ed2201) uses Of course it is possible to add the hash there like in the cmake build, but I thought the priority was to avoid manually editing I have already made preparations for adding the hash, but my priority was to have an automatically and robustly generated After a bit more thinking though, adding the hash might not be as complicated as I had originally feared. Let me try a few things. |
|
There's someting wrong with the date/git hash when building using make on macOS 10.14: I've buildt like I always do; |
Ah. So there is something wrong with the sed command which parses As it turns out, FreeBSD's I had only gotten around to test GNU sed and busybox sed. BTW, if you run |
|
That's better, but there are still no commit hash. I'm not the one who decided if that should be there or not: |
Judging from studying the history of the So the missing git commit hash is not a regression. I would prefer not to hasten adding the git commit hash just two days before a release date. That time can be better spent to make the existing automatic |
Completely agree, and this is the main improvement that was needed. The output of the @ndim I noticed SQUASHME directives in commit messages. Will github automagically consider them when it carries out a web-interface merge? I won't be able to manually cherry pick commits for squashing. I normally use the the github web interface and either merge PRs as they are or squash and merge them (depending on what the author wants or how I think it documents the feature nicer in the git main history) . |
ndim
force-pushed
the
autoconf-package-version
branch
from
c000e74
to
f660b61
Compare
|
@stefanrueger SQUASHME are notices to myself. I will squash those commits and force-push the result before I mark this Draft PR as "Ready for review", so that you can use the web interface to merge. |
ndim
force-pushed
the
autoconf-package-version
branch
3 times, most recently
from
5f0ceba
to
780c91b
Compare
Sometimes when building with the Autoconf buildsystem, the avrdude-html/ subdir already exists and therefore renaming the newly built avrdude/ subdir to avrdude-html/ cannot succeed. Removing the old avrdude-html/ subdir first fixes that.
Generate the version number used in the Autotools builds
via a script from the top-level CMakeLists.txt and git
information instead of manual editing.
This script mimics what the avrdude top-level CMakeLists.txt
does for composing AVRDUDE_FULL_VERSION.
Consequently, maintainers do not need to edit the version numbers
in the "src/configure.ac" file's "AC_INIT(...)" line any more.
However, unlike the cmake based builds, this does not print the
commit hash in the "avrdude --help" message or in the "avrdude.conf"
file's "avrdude_conf_version = " line. That will come later.
These are the six build types supported and what avrdude
versions cmake and the autoconf builds actually produce:
cmake autoconf
git clone release 7.2 7.2
git clone non-release 7.2-DATE+HASH 7.2-DATE
git archive release 7.2 7.2
git archive non-release 7.2 7.2
dist tarball release N/A 7.2
dist tarball non-release N/A 7.2-DATE
ndim
force-pushed
the
autoconf-package-version
branch
from
780c91b
to
d15c561
Compare
|
As you can tell by the CI checks now running again, I have removed the simple autoconf based test build from the CI checks again. I do not want to foist a completely untested CI check on you so close to a release. No surprises to be expected there: None of the existing CI workflows actually use an Autotool based build, so changes to the Autotools build system will not change the CI outcomes in any significant way. I have developed on Fedora, tested on Debian and Arch (all using GNU tools) and FreeBSD (using FreeBSD tools), so that should cover many systems. MSYS2 and mingw use GNU tools, OSX is based on FreeBSD tools, so those should work as well. I am mostly confident that the changes to the Autotools build system should work. The basic idea has been working for me in other projects for the last about 15 years, and the adaptations for avrdude have been tested at least somewhat. I hope I have not overlooked too many edge and corner cases with my manual non-CI non systematic testing. Thanks to @mcuee and @MCUdude for testing. Do ping me if anything comes up. |
|
Regarding the git commit hash, I have a basic idea I am confident will work, but I have not used some of the required techniques yet, so that will require a bit of testing, i.e. time. Expect it some time after the 7.3 release. |
|
I just noticed... does it really make sense for the I just copied the command from Lines 71 to 76 in 1a64d38 The author date of that commit could have been last year, but the committer date ( |
I am fine with this. As @stefanrueger mentioned earlier:
Thinking about this, I am okay to even keep this after 7.3 release as it is kind of a differntiation between CMake build and Autotools build. |
Hmm, strange, so far from my experiences, CMake build always display the commit date and not author date. |
I think it is better to use an explicit marker to distinguish cmake and autotools builds. Noted for after 7.3. |
|
Tested the latest git commit under MSYS2 mingw64 and it works fine. |
|
Unfortunately this PR does not seem to work under NetBSD. But I think it is an existing issue -- not able to detect the libraries. Last time I spent some time to figure out using |
Yes, git main has the same issue. So this is not a regression. And it is okay to address the issue after 7.3 release. I have not looked into the details of NetBSD pkgsrc here to see how they build avrdude 6.4 version using Autotools. |
ndim
added a commit
to ndim/avrdude
that referenced
this pull request
Feb 7, 2024
The test(1) command string comparison operator is a single equals sign, not a double equals sign. Pointed out by @mcuee at avrdudes#1663
ndim
added a commit
to ndim/avrdude
that referenced
this pull request
Feb 7, 2024
The test(1) command string comparison operator is a single equals sign, not a double equals sign. Pointed out by @mcuee at avrdudes#1663
|
OpenBSD is better but somehow last step failed. |
Same for git main. So again this is not a regression. git main build last step also failed. I have not looked into OpenBSD port to see how they build avrdude 6.3. |
|
I am happy with this PR. Thanks. We can deal with OpenBSD/NetBSD after 7.3 release as well. |
|
FYI on the git version guess from OpenOCD project. Just for reference. |
This was referenced Feb 7, 2024
23 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR will determine the
PACKAGE_VERSIONwhich the Automake/Autoconf avrdude package uses from the package version which the top levelCMakeLists.txtfile uses.ETA 2024-02-06 23:59:59 UTC.