Libguestfs - Jun 2021 - distributing bash completion links [was: [libnbd PATCH 17/20] ci: Add support for test skipping]

On Tue, Jun 08, 2021 at 09:09:32PM +0200, Martin Kletzander
wrote:
> I still have one more thing that bothers me, though.  The symlinks in
> bash/ are set to be generated and packed into the distribution (using
> the `dist_` prefix for `dist_bashcompdir_DATA`), but they are defined
> conditionally based on whether bash-completion (and libxml2) are
> available on the system.  This, of course, breaks `make dist` for
> systems where any of the packages is not available.  I think that there
> are few ways how to get out of it:
Yeah, it's never a good idea to have the contents of a tarball differ
according to what was present on the machine of the person running
'make dist'.  Tarballs should be invariant, such that what gets
installed should be determined solely by the results of ./configure on
the machine unpacking the tarball, not on the machine that created the
tarball.  So you have indeed uncovered a bug that needs fixing.
> 
> 1) Remove the `dist_` prefix -- this would generate the files during
>    build time when the availability of the dependencies is more relevant.
That means the files only get installed on platforms where configure
says it is useful to install them, rather than being part of the
tarball.  Seems reasonable.
> 
> 2) Remove the conditionals -- creating the files only relies only on
>    `ln` to create the symlink so there is no need for the dependency to
>    be available.
That says the files should always be part of the tarball, regardless
of whether they will be installed.  Also seems reasonable.
> 
> 3) Remove the rules and commit the symlinks to the repository -- this
>    would keep the symlinks in git, they would get installed based on the
>    conditionals, but would be always available for packing into the
>    distribution, at least that's how I understand automake.
The autotools frown on shipping a tarball with a symlink (because not
all systems support symlinks, so extracting such a tarball is not
portable).  That recommendation still exists today, where the major
culprits are Windows and FAT filesystems.  Windows actually supports
symlinks on NTFS file systems, but not necessarily out-of-the-box, but
more importantly, anyone who unpacks the tarball on a flash drive
(which are most-often formatted as FAT) suffer from the problem.  So
just as we shouldn't stick symlinks in the tarball (but instead should
generate them during './config.status'), we probably shouldn't need
to
store symlinks in git.

Of all of these options, I'm leaning the most towards 1.  But it's not
at the top of my priority list at the moment, so I suspect it will be
easier for you to propose a patch and me review it than for me to try
and write the patch.

Thanks for working on CI, by the way!

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

On Wed, Jun 09, 2021 at 11:00:26AM -0500, Eric Blake
wrote:
> On Tue, Jun 08, 2021 at 09:09:32PM +0200, Martin Kletzander wrote:
> > I still have one more thing that bothers me, though.  The symlinks in
> > bash/ are set to be generated and packed into the distribution (using
> > the `dist_` prefix for `dist_bashcompdir_DATA`), but they are defined
> > conditionally based on whether bash-completion (and libxml2) are
> > available on the system.  This, of course, breaks `make dist` for
> > systems where any of the packages is not available.  I think that
there
> > are few ways how to get out of it:
> 
> Yeah, it's never a good idea to have the contents of a tarball differ
> according to what was present on the machine of the person running
> 'make dist'.  Tarballs should be invariant, such that what gets
> installed should be determined solely by the results of ./configure on
> the machine unpacking the tarball, not on the machine that created the
> tarball.  So you have indeed uncovered a bug that needs fixing.
I've been gradually fixing similar issues over time, eg most recently:

https://gitlab.com/nbdkit/nbdkit/-/commit/021f44bfed9b222677c90402486bec76c16a1ef3

So yes, I agree with what Eric said.

Rich.
> > 
> > 1) Remove the `dist_` prefix -- this would generate the files during
> >    build time when the availability of the dependencies is more
relevant.
> 
> That means the files only get installed on platforms where configure
> says it is useful to install them, rather than being part of the
> tarball.  Seems reasonable.
> 
> > 
> > 2) Remove the conditionals -- creating the files only relies only on
> >    `ln` to create the symlink so there is no need for the dependency
to
> >    be available.
> 
> That says the files should always be part of the tarball, regardless
> of whether they will be installed.  Also seems reasonable.
> 
> > 
> > 3) Remove the rules and commit the symlinks to the repository -- this
> >    would keep the symlinks in git, they would get installed based on
the
> >    conditionals, but would be always available for packing into the
> >    distribution, at least that's how I understand automake.
> 
> The autotools frown on shipping a tarball with a symlink (because not
> all systems support symlinks, so extracting such a tarball is not
> portable).  That recommendation still exists today, where the major
> culprits are Windows and FAT filesystems.  Windows actually supports
> symlinks on NTFS file systems, but not necessarily out-of-the-box, but
> more importantly, anyone who unpacks the tarball on a flash drive
> (which are most-often formatted as FAT) suffer from the problem.  So
> just as we shouldn't stick symlinks in the tarball (but instead should
> generate them during './config.status'), we probably shouldn't
need to
> store symlinks in git.
> 
> Of all of these options, I'm leaning the most towards 1.  But it's
not
> at the top of my priority list at the moment, so I suspect it will be
> easier for you to propose a patch and me review it than for me to try
> and write the patch.
> 
> Thanks for working on CI, by the way!
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/