Buildsystem cleanups #17
Conversation
ndim
force-pushed
the
buildsystem-cleanups
branch
2 times, most recently
from
73f9539
to
fa6ff74
Compare
|
Hi Hans, thanks for the contribution! It is nice to get the build system cleaned up.
I will look at this in detail over the next few days and let you know if I have any other questions or comments. By the way, what motivated your interest in the build system cleanup? Thanks again! -Eric |
|
See above, too, but clearly you are more familiar with autoconf and its interaction with Question: Is it best practice to distribute the files in |
Why require automake >= 1.13?Because you have already been using Also note that automake-1.13 was released in 2012-12-28, which is quite close to the 2012-04-25 release of autoconf-2.69 from a 2022 perspective - and you have had Side note: I have found either libtool or gettext to not properly handle Why distribute the files inside the
|
|
Awesome, I appreciate it! Thanks for clarifying my various questions. About |
|
All of my systems are based on CentOS 7 and probably will be for at least the next year or so, which is the reason for the Autoconf 2.69 target. I tried running autogen.sh./configureThen running According to Red Hat, they do not ship Do you have a work-around for |
|
Quick note: This is not ready for merge, for a number of reasons. That is why I filed this as a Draft PR. I am still clarifying some details regarding the auto dependencies for the resource compilation. Regarding Regarding And let me try building on CentOS 7 (my Fedora 37 system has a mock config for CentOS 7) and a few more systems. |
|
As to PKG_CHECK_VAR, that can be worked around easily if necessary. |
|
I'm guessing that I inherited the code as the new maintainer from Neoklis (the original developer) and I think he was using a default autoconf stack way back in 2008. Looking at the header it appears that autogen.sh was canned and copied by Neoklis for this project since the original authors have nothing to do with xnec2c: The only exception to to this is that I've added some checks for missing packages that if [ ! -x "$(command -v gettext)" ]; then
echo "gettext does not exist: install the gettext package."
exit 1
fi
if [ ! -x "$(command -v gettextize)" ]; then
echo "gettextize does not exist: install the gettext package."
exit 1
fi
if [ ! -x "$(command -v autopoint)" ]; then
echo "autopoint does not exist: install the autopoint or gettext-devel package."
exit 1
fi
if [ ! -x "$(command -v glib-compile-resources)" ]; then
echo "glib-compile-resources does not exist: install the libglib2.0-dev-bin or glib2-devel package."
exit 1
fi
if [ ! -x "$(command -v pkg-config)" ]; then
echo "pkgconfig does not exist: install the pkgconfig or pkg-config package."
exit 1
fi... so if we can fix whatever is triggering the What do you think? |
|
Regarding mock builds for el7: there is an xnec2c.spec in the git tree if that is helpful. Beware of version tagging issues: There is a Someday I would like to make xnec2c.spec.in as suggested on SE so the I've not gotten around to cleaning up the auto-version stuff yet; right now these are the files that have the version hard-coded that would benefit from a
|
|
Quick note on build tool versions: xnec2c requires gettext version 0.19.7 from 2015-12, which is 3 years younger than automake 1.13 and 3.5 years younger than autoconf 2.69. And that is even without any translations. |
Do not rush into changing the git release tags. github releases (which happen via git tags) versus GNU buildsystem dist tarball releases. First,
Unfortunately, you cannot disable the repo dump tarballs on Github releases, but you can attach other binaries to a release, either manually or with a Github workflow. See e.g. the https://github.com/gphoto/libgphoto2/releases/tag/v2.5.30 release: While there is a "Source Code" tarball with the URL v2.5.30.tar.gz which is just the git source tree dump, there are also dist tarballs available in three compression flavours: These days, dist tarballs are starting to become less significant as a method for distributing software releases, with Github's automatic source tree dump tarballs or even "just checkout our release branch" beginning to replace dist tarballs. Both methods have their advantages and disadvantages. Anyway, I would keep using the git tags for the git releases at
That is easy.
Agreed about those two. They can take their version information from BTW, are you married to the recursive make
I do not see what you would want to accomplish with a What is possible is to have
However, I do not know whether you want to go that route. I personally am using such a script in my more or less personal projects like https://github.com/ndim/ndim-git-utils https://github.com/ndim/ndim-utils: They both use two number release tags like e.g. |
That is indeed fixed by the "Remove $(shell) parts from src/Makefile.am" commit.
[...]
There is no need to check for You also do not need the A standard build will need I guess I can take a look through past xnec2c github issues and see what can be done with an autoreconf based buildsystem initialization.
The exact error messages could maybe be better for some situations, but the exact name of the missing packages and how to install them is highly system specific, so you cannot be expected to prepare error messages with detailed instructions suitable to all operating systems. I would just use |
ndim
force-pushed
the
buildsystem-cleanups
branch
2 times, most recently
from
fac2018
to
7814f8e
Compare
|
Remark on Centos 7 having an old However, if you have users who build from git on Centos 7, the alternative implementation of |
ndim
force-pushed
the
buildsystem-cleanups
branch
from
d645b0a
to
a0e201d
Compare
Great information. I'll stick with
For now manually editing the configure.ac with the latest version is the way to go in case I miss a tag or have some other tagging problem.
Maybe shipping our own pkg.m4 (or even pkg-check-var.m4 with just the missing bits) because my desk will be el7 for a while. I plan to update my desk to Oracle Linux 8 or Oracle Linux 9 but it might still be a while and all my builds are still on el7 for the near feature. Also I agree with dropping the binary checks in
Either is fine, very rarely do I do something like |
Consistency (i.e. continuing the existing theme) will also make any future automation and communication MUCH simpler.
An But that can always be changed later, we do not need to deal with that now.
Shipping pkg.m4 is a brilliant idea. Why have I not thought about that? With aclocal's It is also important information that you as the main dev run EL7. I have checked EL7 tool version availability within a mock centos-7 chroot and remarked on what tool versions mean for xnec2c in a comment in
I still want to go through past github build issues for ideas there.
Given that you are still on EL7, and that EL7 only has automake 1.13.4 which does not know |
ndim
force-pushed
the
buildsystem-cleanups
branch
from
a0e201d
to
8ffaf18
Compare
ndim
force-pushed
the
buildsystem-cleanups
branch
4 times, most recently
from
d35ad7d
to
04153e8
Compare
|
Are you still planning to replace I tried Aside from the warning, your branch builds and runs fine. Has anything changed that would effect |
Require at least automake 1.13.2 which made AC_CONFIG_MACRO_DIRS work properly, and xnec2c has been using AC_CONFIG_MACRO_DIRS for ages. This also happens to require CVE-2012-3386 to have been fixed in the automake version used (1.12.2 or later). While automake-1.13.2 was released *after* the autoconf-2.69 release xnec2c has been requiring since 2013, both automake-1.13.2 and autoconf-2.69 happend in 2012/2013, so requiring at least automake-1.13.2 does not change the age of systems significantly when viewed from 2022. Also note that Centos 7 comes with automake-1.13.4.
This anchors patterns like the po/foo patterns at the location
of the .gitignore file, and ignores Makefile and Makefile.in
anywhere.
Also ignores some more patterns created by the buildsystem like
.deps/
/compile
/depcomp
etc.
PKG_CHECK_MODULES already makes the _CFLAGS and _LIBS vars an AC_ARG_VAR which in turn AC_SUBST()s them.
Use more appropriate GTK_(CFLAGS|LIBS) var names for gtk+-3.0 instead of the generic nondescript PACKAGE_(CFLAGS|LIBS).
Catch missing m4 macros when generating the configure script instead of generating a malformed configure script and then executing that just to have it fail.
Remove the $(shell ...) parts from src/Makefile.am as not all
make implementations support them and Automake does not support
them either.
* Generating dependency information has been moved into the
actual $(GLIB_COMPILE_RESOURCES) recipe to make it a side
effect of compilation.
* Finding pkg-config and determining GLIB_COMPILE_RESOURCES
from the gio-2.0.pc file's glib_compile_resource variable
has been moved to configure.ac.
Our auto-dependency generation works just like Automakes
auto-dependency generation, but it but does not directly rely
on any of Automake's make recipes, files, or directories.
* We use our own dependency file directory (analog to .deps)
* We use our own dep dir variable (analog to DEPDIR)
* We include the dep file into Makefile.am
* We generate the dummy dep file as empty like Automake
will after release 2.71
* We generate the actual dep file as a side effect of compilation
* We do not distribute the generated *.c file in the dist tarball
To allow running autoreconf on a system without the PKG_CHECK_VAR
macro in pkg.m4 (e.g. CentOS 7 with older pkg-config), we ship a
copy of a modern pkg.m4 file in m4-include/. aclocal will then
pick the latest (by "serial" number) version of pkg.m4 it can
find in m4/ m4-include/ /usr/share/aclocal etc.
Also fixes out of source tree builds by using the proper $(top_srcdir)
variable to refer to the source files in the make rule recipe.
Also add ax_check_compile_flag.m4 to dist tarball. Someone might
want to autoreconf from a dist tarball after all.
Verify glib-compile-resources works as intended. This comprises
two steps:
* run glib-compile-resources on a resource file *.xml
to generate a C source file
* compile and link that C source file
If any of these steps fails, abort configure.
Enable AM_MAINTAINER_MODE by default, so that you can still run `./configure --enable-maintainer-mode` without a bit WARNING message, but so that by default builds actually build properly by default. The main purpose of AM_MAINTAINER_MODE is to allow broken builds to happen in some cases. The argument against AM_MAINTAINER_MODE have convinced even the author of AM_MAINTAINER_MODE to stop using it: https://www.gnu.org/software/automake/manual/automake.html#maintainer_002dmode I am not sure whether AM_MAINTAINER_MODE is actually required for something with xnec2c, or whether it just seemed like a good idea in the early 2000s and just was perpetuated without thinking about it too much. If the latter, we could just remove it altogether at the cost of `configure --enable-maintainer-mode` then producing configure: WARNING: unrecognized options: --enable-maintainer-mode
Add list of build tool versions to document why what version should be required.
Have automake warn about possible problems.
ndim
force-pushed
the
buildsystem-cleanups
branch
from
04153e8
to
aaf73e0
Compare
I sort of think that it would be okay to have a special case for
Somewhere the Because they might (or might not) build as a local user and then |
|
I would change the logic a bit and move it to
BTW, the RPM spec file contains an Also, perhaps part of the RPM spec As not having any of that does not change the current state of xnec2c, I would push these to after the big PR merge, though. That is becoming big enough as it is. |
The .gitignore file is long enough that it can use some structure. This categorizes the ignore patterns into categories like "generated by command xy" and "manually generated files". Also adds a few more generated files like dist tarballs, testsuite logs (not in use yet), or some buildsystem artefact in po/.
Refactor the xnec2c flags in src/Makefile.am:
* Stop using AM_* vars. Use xnec2c_* vars instead.
* One flag per line. Allows for easier diff/patch
and also easier adding/removing flags via sed.
* Group library specific --cflags and --libs together as
xnec2c_CPPFLAGS += $(GTK3_CFLAGS)
xnec2c_LDADD += $(GTK3_LIBS)
This makes it easier to catch when one of them is missing.
* Use proper quotes for the C preprocessor string macros:
-DFOO="\"bar\""
* Add AM_PROG_CC_C_O (required for automake < 1.14)
Move auxiliary files into the auto-aux/ subdir, like
compile
config.guess
config.rpath
config.sub
depcomp
install-sh
missing
test-driver
This makes the source tree a bit cleaner.
Look for gmodule-2.0 via PKG_CHECK_MODULES. This removes the need for the AC_CHECK_LIB() check later. Not sure why we need to explicitly link against gmodule-2.0 on macOS and not on Linux, but this certainly builds on both. Fixes one of four from KJ7LNW#16
Use Automake variables to have Automake rules install files instead of using a homebuilt "make install-data-local" recipe. This also adds some files to the dist tarball which were formerly forgotten (MAINTAINER README.md PACKAGING). This also adds a long list of files (150+) spelled out explicitly instead of a recursive copy operation just copying everything from examples/. This looks like a lot to maintain, but after this initial effort, the changes should be only limited, when an antenna or two may be added in the future. We have also added an automatic consistency check which tells you explicitly when that list is inconsistent what part of it is. Advantage: If you add or remove any files in examples/, you will need to reflect that change in Makefile.am which forces you to think whether that change was intended or not, and therefore this will catch unintended changes. The recursive copy method just copied over everything, including editor backup files, or anything else which happens to be in one of the affected directories.
DISTCLEANFILES deleted too much, and therefore broke the "make distcheck" builds. Overall, the DISTCLEANFILES looked very much like a copy of the .gitignore file, but that is not what DISTCLEANFILES is meant for. With DISTCLEANFILES deleted, "make distcheck" still runs without complaining about "make distclean" leaving files around which should have been deleted, it looks like DISTCLEANFILES is not needed for xnec2c at all. If you want to restore a git source tree to the pristine state of a fresh git clone, use `git clean`.
ndim
force-pushed
the
buildsystem-cleanups
branch
from
aaf73e0
to
6b501d6
Compare
|
Status Update These things MUST be addressed before merging this PR and cutting a release:
These things SHOULD be addressed before merging this PR and cutting a release:
These things COULD be addressed before merging this PR and cutting a release:
These things SHOULD be addressed in new PRs after merging this PR, before or after the next release:
It might be useful to merge all changes which change the way people build and install xnec2c at once, so people only need to adapt to build/install changes once, not every few days and every release for some time. |
Good idea.
I saw that too. It came from the original Fedora maintainer's .spec and I just copy-pasted his into the package with some minor tweaks. Popping that out into an XML is a good idea. Added #26 so that doesn't get lost.
True. The desktop-install bits are hacked in place so adding
Agreed, noted in #26.
I've been watching your commits and nothing particularly noteworthy stands out except that if we use Can you think of anything else?
What would it take to add github's CI integration (
I think we should just replace the content of
I assume you're referring to
Neat! Go for it if its easy.
I think I'm that someone. :)
Awesome!
I agree. I'm planning a do this as part of a 4.5 release. |
|
Hi @ndim , I've merged this branch into master. Your changes made it possible to build on OpenBSD. Thanks for your hard work on this! If there are other changes you still wish to make for the buildsystem refactor/cleanup then feel free to open another PR. -Eric, KJ7LNW |
This tries to fix a few obvious problems with the build system, such as
make disttrying to distribute a non-existing file.xnec2cmake distnot distribute required filesresources/*configureup unexpanded.gitignoreignoring some files where they should not be ignored and ignoring some files only in some places but not in others where they should be ignoredsrc/Makefile.amI am creating this as a draft pull request as I have a few questions about the dependency tracking for built sources which I still need to clarify.
(FYI, I am not an xnec2c user. I am just running into questions about its build system on stackoverflow again and again.)