Bug 103420 - Resync tarball contents with what's in git
Summary: Resync tarball contents with what's in git
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: git master
Hardware: Other All
: medium enhancement
Assignee: Simon McVittie
QA Contact: D-Bus Maintainers
URL:
Whiteboard: review+
Keywords: patch
Depends on:
Blocks:
 
Reported: 2017-10-23 12:55 UTC by Simon McVittie
Modified: 2017-10-23 15:12 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
build: Don't distribute versioninfo.rc in "make dist" tarballs (1.21 KB, patch)
2017-10-23 13:05 UTC, Simon McVittie
Details | Splinter Review
build: Distribute more test data in source tarballs (1.33 KB, patch)
2017-10-23 13:06 UTC, Simon McVittie
Details | Splinter Review
build: Distribute individual files and directories from cmake/ (1.16 KB, patch)
2017-10-23 13:06 UTC, Simon McVittie
Details | Splinter Review
build: Include README.cmake in Autotools "make dist" (823 bytes, patch)
2017-10-23 13:06 UTC, Simon McVittie
Details | Splinter Review
build: Don't distribute versioninfo.rc in "make dist" tarballs (1.13 KB, patch)
2017-10-23 13:42 UTC, Simon McVittie
Details | Splinter Review
build: Don't explicitly clean up configure-generated files (995 bytes, patch)
2017-10-23 13:43 UTC, Simon McVittie
Details | Splinter Review
build: Remove various unused files from build system (31.65 KB, patch)
2017-10-23 14:08 UTC, Simon McVittie
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Simon McVittie 2017-10-23 12:55:50 UTC
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).
Comment 1 Simon McVittie 2017-10-23 13:05:42 UTC
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".
Comment 2 Simon McVittie 2017-10-23 13:06:15 UTC
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.
Comment 3 Simon McVittie 2017-10-23 13:06:33 UTC
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.
Comment 4 Simon McVittie 2017-10-23 13:06:53 UTC
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.
Comment 5 Simon McVittie 2017-10-23 13:13:29 UTC
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
Comment 6 Simon McVittie 2017-10-23 13:22:31 UTC
(This is not going to be in 1.11.22 - not important enough - but could still be in 1.12.0, IMO.)
Comment 7 Philip Withnall 2017-10-23 13:28:04 UTC
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 8 Philip Withnall 2017-10-23 13:28:28 UTC
Comment on attachment 135007 [details] [review]
build: Distribute more test data in source tarballs

Review of attachment 135007 [details] [review]:
-----------------------------------------------------------------

r+
Comment 9 Philip Withnall 2017-10-23 13:29:17 UTC
Comment on attachment 135008 [details] [review]
build: Distribute individual files and directories from  cmake/

Review of attachment 135008 [details] [review]:
-----------------------------------------------------------------

r+
Comment 10 Philip Withnall 2017-10-23 13:29:29 UTC
Comment on attachment 135009 [details] [review]
build: Include README.cmake in Autotools "make dist"

Review of attachment 135009 [details] [review]:
-----------------------------------------------------------------

r+
Comment 11 Simon McVittie 2017-10-23 13:30:36 UTC
(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.
Comment 12 Philip Withnall 2017-10-23 13:34:38 UTC
(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.
Comment 13 Simon McVittie 2017-10-23 13:42:53 UTC
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).
Comment 14 Simon McVittie 2017-10-23 13:43:23 UTC
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.
Comment 15 Simon McVittie 2017-10-23 13:50:46 UTC
(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.
Comment 16 Simon McVittie 2017-10-23 14:08:14 UTC
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.
Comment 17 Ralf Habacker 2017-10-23 14:25:12 UTC
(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 18 Philip Withnall 2017-10-23 14:33:40 UTC
Comment on attachment 135011 [details] [review]
build: Don't distribute versioninfo.rc in "make dist"  tarballs

Review of attachment 135011 [details] [review]:
-----------------------------------------------------------------

r+
Comment 19 Philip Withnall 2017-10-23 14:33:57 UTC
Comment on attachment 135012 [details] [review]
build: Don't explicitly clean up configure-generated  files

Review of attachment 135012 [details] [review]:
-----------------------------------------------------------------

r+
Comment 20 Philip Withnall 2017-10-23 14:34:13 UTC
Comment on attachment 135013 [details] [review]
build: Remove various unused files from build system

Review of attachment 135013 [details] [review]:
-----------------------------------------------------------------

r+
Comment 21 Simon McVittie 2017-10-23 15:07:03 UTC
(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).
Comment 22 Simon McVittie 2017-10-23 15:12:04 UTC
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.