While reviewing the diff between 1.11.20 and 1.11.22 tarballs, I noticed that dbus/versioninfo.rc is included in tarballs. It shouldn't be: it's generated by configure (or by CMake if you use that) from dbus/versioninfo.rc.in. Looking into this further, there are a couple of other files in tarballs that shouldn't be, and several files not in tarballs that should be. I'd like to fix this before 1.12.0 so that 1.12.x releases don't have excess delta between versions, for better interaction with distribution stable release managers (mainly Debian's).
Created attachment 135006 [details] [review] build: Don't distribute versioninfo.rc in "make dist" tarballs It's generated by configure, so we should not distribute it, and we should clean it up in "make distclean".
Created attachment 135007 [details] [review] build: Distribute more test data in source tarballs test-bus exercises the parser by trying to parse every file in these directories. A couple of files were accidentally left out, meaning those parsing code paths are tested when we build from git, but not when we build from a tarball release.
Created attachment 135008 [details] [review] build: Distribute individual files and directories from cmake/ If we distribute the entire directory in "make dist" tarballs, then we include the generated files cmake/DBus1Config.cmake and cmake/DBus1ConfigVersion.cmake, which we should not.
Created attachment 135009 [details] [review] build: Include README.cmake in Autotools "make dist" Our official source releases are Autotools "make dist" tarballs, but there's no reason why CMake users can't use those too, and we already include the CMake build files.
Comparison between `git archive` and `make dist` with those patches: No files are in both with different contents. Only in git: .gitignore | 47¯ .mailmap | 10¯ bus/.gitignore | 28¯ dbus/.gitignore | 17¯ doc/.gitignore | 28¯ test/.gitignore | 39¯ test/data/invalid-service-files-system/.gitignore | 1¯ test/data/valid-config-files-system/.gitignore | 1¯ test/data/valid-config-files/.gitignore | 6¯ test/data/valid-service-files-system/.gitignore | 1¯ test/data/valid-service-files/.gitignore | 1¯ test/name-test/.gitignore | 15¯ tools/.gitignore | 22¯ Git administrivia, not very useful when not in git (could be included if desired, I don't particularly care either way) .travis.yml | 45¯ tools/ci-Dockerfile.in | 10¯ tools/ci-build.sh | 271¯ tools/ci-install.sh | 161¯ Used in CI, not very useful other than on GitHub (but could be included if desired; tools/ci-*.sh are intended to be non-Travis-specific) Makefile.cvs | 8¯ test/break-loader.c | 762¯ test/data/valid-introspection-files/lots-of-types.xml | 93¯ test/unused-code-gc.py | 242¯ Old files that are no longer hooked up to the build system Only in `make dist`: Makefile.in | 1111¯ aclocal.m4 | 2808 + build-aux/compile | 347¯ build-aux/config.guess | 1462¯ build-aux/config.sub | 1825 + build-aux/depcomp | 791¯ build-aux/install-sh | 508¯ build-aux/ltmain.sh |11156 ++++++ build-aux/missing | 215¯ build-aux/tap-driver.sh | 651¯ bus/Makefile.in | 1673¯ config.h.in | 541¯ configure |30337 ++++++++++++++++++ dbus/Makefile.in | 1760 + doc/Makefile.in | 940¯ m4/libtool.m4 | 8387 ++++ m4/ltoptions.m4 | 437¯ m4/ltsugar.m4 | 124¯ m4/ltversion.m4 | 23¯ m4/lt~obsolete.m4 | 99¯ test/Makefile.in | 2393 + test/name-test/Makefile.in | 1285¯ tools/Makefile.in | 1104¯ Standard Autotools droppings to make the tarball not require Autoconf, Automake, Libtool, autoconf-archive
(This is not going to be in 1.11.22 - not important enough - but could still be in 1.12.0, IMO.)
Comment on attachment 135006 [details] [review] build: Don't distribute versioninfo.rc in "make dist" tarballs Review of attachment 135006 [details] [review]: ----------------------------------------------------------------- r- pending an open question. ::: dbus/Makefile.am @@ +333,5 @@ > > clean-local: > $(AM_V_at)rm -fr ./.dbus-keyrings > + > +DISTCLEANFILES = versioninfo.rc Should this be necessary? Since versioninfo.rc is generated by AC_CONFIG_FILES, it should be automatically distcleaned.
Comment on attachment 135007 [details] [review] build: Distribute more test data in source tarballs Review of attachment 135007 [details] [review]: ----------------------------------------------------------------- r+
Comment on attachment 135008 [details] [review] build: Distribute individual files and directories from cmake/ Review of attachment 135008 [details] [review]: ----------------------------------------------------------------- r+
Comment on attachment 135009 [details] [review] build: Include README.cmake in Autotools "make dist" Review of attachment 135009 [details] [review]: ----------------------------------------------------------------- r+
(In reply to Philip Withnall from comment #7) > Should this be necessary? Since versioninfo.rc is generated by > AC_CONFIG_FILES, it should be automatically distcleaned. Good question - I didn't know there was automation there. I'll check.
(In reply to Simon McVittie from comment #5) > Only in git: > *snip* > Git administrivia, not very useful when not in git > (could be included if desired, I don't particularly care > either way) I would vote to drop these in favour of git.mk (https://github.com/behdad/git.mk) which has the benefit of helping to keep the automake CLEAN variables up to date (if something’s generated and not cleaned, then it will show up in `git status` as untracked). > Used in CI, not very useful other than on GitHub > (but could be included if desired; tools/ci-*.sh are intended > to be non-Travis-specific) I have no feelings either way about these. > Makefile.cvs | 8¯ > test/break-loader.c | 762¯ > test/data/valid-introspection-files/lots-of-types.xml | 93¯ > test/unused-code-gc.py | 242¯ > > Old files that are no longer hooked up to the build system Drop them from git too? > Only in `make dist`: *snip* > > Standard Autotools droppings to make the tarball not require > Autoconf, Automake, Libtool, autoconf-archive They all look normal.
Created attachment 135011 [details] [review] build: Don't distribute versioninfo.rc in "make dist" tarballs It's generated by configure, so we should not distribute it. Because it's generated by configure, it is automatically cleaned up by "make distclean" via $(CONFIG_CLEAN_FILES).
Created attachment 135012 [details] [review] build: Don't explicitly clean up configure-generated files cmake/DBus1Config.cmake, cmake/DBus1ConfigVersion.cmake and dbus-1.pc are all generated by AC_CONFIG_FILES, so they are automatically listed in $(CONFIG_CLEAN_FILES) and cleaned in "make distclean" without further help from us.
(In reply to Philip Withnall from comment #12) > I would vote to drop these in favour of git.mk > (https://github.com/behdad/git.mk) which has the benefit of helping to keep > the automake CLEAN variables up to date (if something’s generated and not > cleaned, then it will show up in `git status` as untracked). This is problematic as long as we have two parallel build systems (Autotools and CMake), which we probably will for a while, because Autotools requires a Unix shell and does not support compiling with a non-Unix-style compiler like MSVC, and Windows developers are apparently sufficiently keen on compiling with MSVC to make that a requirement. Perhaps one day Meson will be sufficiently ubiquitous, well-understood by Linux distribution vendors, and good at cross-compiling to replace both Autotools and CMake; but I can't say I'm in any hurry to look into this. dbus is important, only has a small contributor base, and is not a huge codebase, so I feel strongly that it should be a follower, not a leader, in its choice of build system and infrastructure. (Before anyone suggests it: CMake is not sufficiently distro-friendly to be the build system of choice on Unix, for something as important as dbus. I hope that one day, Meson will be.) > > Makefile.cvs | 8¯ > > test/break-loader.c | 762¯ > > test/data/valid-introspection-files/lots-of-types.xml | 93¯ > > test/unused-code-gc.py | 242¯ > > > > Old files that are no longer hooked up to the build system > > Drop them from git too? It's certainly tempting.
Created attachment 135013 [details] [review] build: Remove various unused files from build system These were in git but not distributed in source tarballs, and in fact not hooked up to the Autotools build system at all. test/data/valid-introspection-files was mentioned in the CMake build system (copied from the source directory to the build directory), but according to `git grep` is not used for anything.
(In reply to Simon McVittie from comment #15) > > (Before anyone suggests it: CMake is not sufficiently distro-friendly to be > the build system of choice on Unix, for something as important as dbus. I > hope that one day, Meson will be.) Do you have any problem with reporting this statement to the cmake guys ? May be they are interested in doing thinks better.
Comment on attachment 135011 [details] [review] build: Don't distribute versioninfo.rc in "make dist" tarballs Review of attachment 135011 [details] [review]: ----------------------------------------------------------------- r+
Comment on attachment 135012 [details] [review] build: Don't explicitly clean up configure-generated files Review of attachment 135012 [details] [review]: ----------------------------------------------------------------- r+
Comment on attachment 135013 [details] [review] build: Remove various unused files from build system Review of attachment 135013 [details] [review]: ----------------------------------------------------------------- r+
(In reply to Ralf Habacker from comment #17) > (In reply to Simon McVittie from comment #15) > > > > (Before anyone suggests it: CMake is not sufficiently distro-friendly to be > > the build system of choice on Unix, for something as important as dbus. I > > hope that one day, Meson will be.) > > Do you have any problem with reporting this statement to the cmake guys ? > May be they are interested in doing thinks better. Pass this on if you want, but I don't think I can articulate why I think this in terms that would be an actionable bug report/enhancement request, so it might not be particularly useful. I suppose the main thing is a general feeling that we seem to have had to spend time reinventing a lot of pieces of standard infrastructure that Autotools has always had, and I suspect we're still lagging way behind Autotools in terms of how well the non-mainstream cases (particularly non-Linux Unix) work. Autotools has some nasty design flaws, most of which result from its decades-old assumptions not all being true any more, but the fact remains that it's a repository of hard-won knowledge about what is/was necessary to make the GNU stack portable. Also, if we have to reinvent standard infrastructure, so does everyone else, and they won't do it quite the same way we do, so the various CMake projects in a distribution aren't going to behave the same - that scales poorly. The other side is that distributions are very good at Autotools (because they have to be, given the amount of GNU stuff that depends on Autotools) - so they know how to make use of the good parts and fix or work around the bad parts. This is not something that CMake developers can really improve, because it isn't really a fact about CMake at all. Meson doesn't have that Autotools advantage, but it implements key parts of the GNU/Autotools command-line API (--prefix, --sysconfdir etc.) so that it will fit into the infrastructure that distributions already have; it provides things distributions need (again, like --sysconfdir) in a uniform way for every project, rather than having projects responsible for providing them, which results in them being different per-project; and it seems to have it as a goal that it works well with (and makes use of) "nearby" pieces of infrastructure like pkg-config, which lets it benefit from those parts of the ecosystem, whereas in dbus we've had to spend time adding special CMake equivalents of .pc files. I don't think Meson is there yet - in particular, it needs some attention from people with an in-depth understanding of cross-compiling. However, I think it has the potential to be a successor to Autotools, mainly because of how much it is taking advantage of knowledge and conventions derived from Autotools (particularly via its adoption by GNOME, which has a lot of Autotools history).
Patches merged for 1.11.24 or 1.12.0, thanks for reviewing!
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.