[PATCH 0/4] debian: Integrate Debian/Ubuntu changes

[PATCH 0/4] debian: Integrate Debian/Ubuntu changes

[Bastian Germann]

Ubuntu has changes on xfsprogs. Integrate the useful ones so that Debian
and Ubuntu differ less.

There was one bug introduced with the 5.6 -> 5.10 change in Debian: The
new libinih package did not have a udeb package which is needed by
xfsprogs-udeb. Explicitly depend on newer libinih versions.

Bastian Germann (4):
  debian: Drop trying to create upstream distribution
  debian: Enable CET on amd64
  debian: Regenerate config.guess using debhelper
  debian: Build-depend on libinih-dev with udeb package

 debian/changelog | 14 ++++++++++++++
 debian/control   |  2 +-
 debian/rules     | 10 ++++++++--
 3 files changed, 23 insertions(+), 3 deletions(-)

-- 
2.30.1

[PATCH 1/4] debian: Drop trying to create upstream distribution

[Bastian Germann]

This is a change introduced in 4.3.0+nmu1ubuntu1.

Reported-by: Dimitri John Ledkov <xnox@ubuntu.com>
Signed-off-by: Bastian Germann <bastiangermann@fishpost.de>
---
 debian/changelog | 7 +++++++
 debian/rules     | 1 -
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/debian/changelog b/debian/changelog
index 5421aed6..8320a2e8 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+xfsprogs (5.11.0-rc0-1) experimental; urgency=medium
+
+  [ Dimitri John Ledkov ]
+  * Drop trying to create upstream distribution
+
+ -- Bastian Germann <bastiangermann@fishpost.de>  Sat, 20 Feb 2021 11:57:31 +0100
+
 xfsprogs (5.10.0-3) unstable; urgency=medium
 
   * Drop unused dh-python from Build-Depends (Closes: #981361)
diff --git a/debian/rules b/debian/rules
index c6ca5491..8a3345b6 100755
--- a/debian/rules
+++ b/debian/rules
@@ -81,7 +81,6 @@ binary-arch: checkroot built
 	$(pkgme)  $(MAKE) -C . install
 	$(pkgdev) $(MAKE) -C . install-dev
 	$(pkgdi)  $(MAKE) -C debian install-d-i
-	$(pkgme)  $(MAKE) dist
 	install -D -m 0755 debian/local/initramfs.hook debian/xfsprogs/usr/share/initramfs-tools/hooks/xfs
 	rmdir debian/xfslibs-dev/usr/share/doc/xfsprogs
 	rm -f debian/xfslibs-dev/lib/libhandle.la
-- 
2.30.1

Re: [PATCH 1/4] debian: Drop trying to create upstream distribution

[Dave Chinner]

On Sat, Feb 20, 2021 at 01:16:06PM +0100, Bastian Germann wrote:
> This is a change introduced in 4.3.0+nmu1ubuntu1.

Why? When the change was made is not important, explaining why the
change needs to be made is what should be in the commit message.
i.e. Tell us why this isn't needed any more...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 1/4] debian: Drop trying to create upstream distribution

[Darrick J. Wong]

On Sun, Feb 21, 2021 at 03:01:29PM +1100, Dave Chinner wrote:
> On Sat, Feb 20, 2021 at 01:16:06PM +0100, Bastian Germann wrote:
> > This is a change introduced in 4.3.0+nmu1ubuntu1.
> 
> Why? When the change was made is not important, explaining why the
> change needs to be made is what should be in the commit message.
> i.e. Tell us why this isn't needed any more...

Yes, this is vital information to have in the commit message.

I'm guessing it has something to do with the fact that 'make dist' tries
to create source tarballs, which is a strange thing to do during the
binary build, and in any case it ultimately fails because tarball
generation now requires .gitcensus, which is cleaned out and then
requires git to regeneration, which you can't do when you're building
from a source tarball?

<shrug> Hm?

--D

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com

[PATCH 2/4] debian: Enable CET on amd64

[Bastian Germann]

This is a change introduced in 5.6.0-1ubuntu3.

Reported-by: Dimitri John Ledkov <xnox@ubuntu.com>
Signed-off-by: Bastian Germann <bastiangermann@fishpost.de>
---
 debian/changelog | 1 +
 debian/rules     | 8 +++++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/debian/changelog b/debian/changelog
index 8320a2e8..c77f04ab 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -2,6 +2,7 @@ xfsprogs (5.11.0-rc0-1) experimental; urgency=medium
 
   [ Dimitri John Ledkov ]
   * Drop trying to create upstream distribution
+  * Enable CET on amd64
 
  -- Bastian Germann <bastiangermann@fishpost.de>  Sat, 20 Feb 2021 11:57:31 +0100
 
diff --git a/debian/rules b/debian/rules
index 8a3345b6..dd093f2c 100755
--- a/debian/rules
+++ b/debian/rules
@@ -23,8 +23,14 @@ pkgdev = DIST_ROOT=`pwd`/$(dirdev); export DIST_ROOT;
 pkgdi  = DIST_ROOT=`pwd`/$(dirdi); export DIST_ROOT;
 stdenv = @GZIP=-q; export GZIP;
 
+ifeq ($(target),amd64)
+export DEB_CFLAGS_MAINT_APPEND=-fcf-protection
+export DEB_LDFLAGS_MAINT_APPEND=-fcf-protection
+endif
+include /usr/share/dpkg/default.mk
+
 options = export DEBUG=-DNDEBUG DISTRIBUTION=debian \
-	  INSTALL_USER=root INSTALL_GROUP=root \
+	  INSTALL_USER=root INSTALL_GROUP=root LDFLAGS='$(LDFLAGS)' \
 	  LOCAL_CONFIGURE_OPTIONS="--enable-editline=yes --enable-blkid=yes --disable-ubsan --disable-addrsan --disable-threadsan --enable-lto" ;
 diopts  = $(options) \
 	  export OPTIMIZER=-Os LOCAL_CONFIGURE_OPTIONS="--enable-gettext=no --disable-ubsan --disable-addrsan --disable-threadsan --enable-lto" ;
-- 
2.30.1

Re: [PATCH 2/4] debian: Enable CET on amd64

[Dave Chinner]

On Sat, Feb 20, 2021 at 01:16:07PM +0100, Bastian Germann wrote:
> This is a change introduced in 5.6.0-1ubuntu3.
> 
> Reported-by: Dimitri John Ledkov <xnox@ubuntu.com>
> Signed-off-by: Bastian Germann <bastiangermann@fishpost.de>
> ---
>  debian/changelog | 1 +
>  debian/rules     | 8 +++++++-
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/debian/changelog b/debian/changelog
> index 8320a2e8..c77f04ab 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -2,6 +2,7 @@ xfsprogs (5.11.0-rc0-1) experimental; urgency=medium
>  
>    [ Dimitri John Ledkov ]
>    * Drop trying to create upstream distribution
> +  * Enable CET on amd64
>  
>   -- Bastian Germann <bastiangermann@fishpost.de>  Sat, 20 Feb 2021 11:57:31 +0100
>  
> diff --git a/debian/rules b/debian/rules
> index 8a3345b6..dd093f2c 100755
> --- a/debian/rules
> +++ b/debian/rules
> @@ -23,8 +23,14 @@ pkgdev = DIST_ROOT=`pwd`/$(dirdev); export DIST_ROOT;
>  pkgdi  = DIST_ROOT=`pwd`/$(dirdi); export DIST_ROOT;
>  stdenv = @GZIP=-q; export GZIP;
>  
> +ifeq ($(target),amd64)
> +export DEB_CFLAGS_MAINT_APPEND=-fcf-protection
> +export DEB_LDFLAGS_MAINT_APPEND=-fcf-protection
> +endif
> +include /usr/share/dpkg/default.mk
> +
>  options = export DEBUG=-DNDEBUG DISTRIBUTION=debian \
> -	  INSTALL_USER=root INSTALL_GROUP=root \
> +	  INSTALL_USER=root INSTALL_GROUP=root LDFLAGS='$(LDFLAGS)' \
>  	  LOCAL_CONFIGURE_OPTIONS="--enable-editline=yes --enable-blkid=yes --disable-ubsan --disable-addrsan --disable-threadsan --enable-lto" ;
>  diopts  = $(options) \
>  	  export OPTIMIZER=-Os LOCAL_CONFIGURE_OPTIONS="--enable-gettext=no --disable-ubsan --disable-addrsan --disable-threadsan --enable-lto" ;

No. This is not the way to turn on build wide compiler/linker
options/protections.

IOWs, if you want to turn on control flow protections to make ROP
exploits harder (why that actually matters for xfsprogs is beyond
me), then it you need to add a configure option similar to
--enable-lto. Then it can actually be enabled and used by other
distros, not just Ubuntu, and it will also ensure that builds will
fail at configure time if the compiler/linker does not support this
functionality.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

NACK Re: [PATCH 2/4] debian: Enable CET on amd64

[Dimitri John Ledkov]

The patch in question is specific to Ubuntu and was not submitted by
me to neither Debian or Upstream.

Indeed, this is very distro specific, because of all the other things
that we turn on by default in our toolchain, dpkg build flags, and all
other packages.

This patch if taken at face value, will not enable CET. And will make
the package start failing to build from source, when using older
toolchains that don't support said flag.

It should not go upstream nor into debian.

NACK

On Sun, Feb 21, 2021 at 3:59 AM Dave Chinner <david@fromorbit.com> wrote:
>
> On Sat, Feb 20, 2021 at 01:16:07PM +0100, Bastian Germann wrote:
> > This is a change introduced in 5.6.0-1ubuntu3.
> >
> > Reported-by: Dimitri John Ledkov <xnox@ubuntu.com>
> > Signed-off-by: Bastian Germann <bastiangermann@fishpost.de>
> > ---
> >  debian/changelog | 1 +
> >  debian/rules     | 8 +++++++-
> >  2 files changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/debian/changelog b/debian/changelog
> > index 8320a2e8..c77f04ab 100644
> > --- a/debian/changelog
> > +++ b/debian/changelog
> > @@ -2,6 +2,7 @@ xfsprogs (5.11.0-rc0-1) experimental; urgency=medium
> >
> >    [ Dimitri John Ledkov ]
> >    * Drop trying to create upstream distribution
> > +  * Enable CET on amd64
> >
> >   -- Bastian Germann <bastiangermann@fishpost.de>  Sat, 20 Feb 2021 11:57:31 +0100
> >
> > diff --git a/debian/rules b/debian/rules
> > index 8a3345b6..dd093f2c 100755
> > --- a/debian/rules
> > +++ b/debian/rules
> > @@ -23,8 +23,14 @@ pkgdev = DIST_ROOT=`pwd`/$(dirdev); export DIST_ROOT;
> >  pkgdi  = DIST_ROOT=`pwd`/$(dirdi); export DIST_ROOT;
> >  stdenv = @GZIP=-q; export GZIP;
> >
> > +ifeq ($(target),amd64)
> > +export DEB_CFLAGS_MAINT_APPEND=-fcf-protection
> > +export DEB_LDFLAGS_MAINT_APPEND=-fcf-protection
> > +endif
> > +include /usr/share/dpkg/default.mk
> > +
> >  options = export DEBUG=-DNDEBUG DISTRIBUTION=debian \
> > -       INSTALL_USER=root INSTALL_GROUP=root \
> > +       INSTALL_USER=root INSTALL_GROUP=root LDFLAGS='$(LDFLAGS)' \
> >         LOCAL_CONFIGURE_OPTIONS="--enable-editline=yes --enable-blkid=yes --disable-ubsan --disable-addrsan --disable-threadsan --enable-lto" ;
> >  diopts  = $(options) \
> >         export OPTIMIZER=-Os LOCAL_CONFIGURE_OPTIONS="--enable-gettext=no --disable-ubsan --disable-addrsan --disable-threadsan --enable-lto" ;
>
> No. This is not the way to turn on build wide compiler/linker
> options/protections.
>
> IOWs, if you want to turn on control flow protections to make ROP
> exploits harder (why that actually matters for xfsprogs is beyond
> me), then it you need to add a configure option similar to
> --enable-lto. Then it can actually be enabled and used by other
> distros, not just Ubuntu, and it will also ensure that builds will
> fail at configure time if the compiler/linker does not support this
> functionality.
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com



-- 
Regards,

Dimitri.

Re: NACK Re: [PATCH 2/4] debian: Enable CET on amd64

[Dave Chinner]

On Sun, Feb 21, 2021 at 04:02:55AM +0000, Dimitri John Ledkov wrote:
> The patch in question is specific to Ubuntu and was not submitted by
> me to neither Debian or Upstream.
> 
> Indeed, this is very distro specific, because of all the other things
> that we turn on by default in our toolchain, dpkg build flags, and all
> other packages.
> 
> This patch if taken at face value, will not enable CET. And will make
> the package start failing to build from source, when using older
> toolchains that don't support said flag.

Yes, that is exactly what I said when pointing out how to *support
this properly* so it doesn't break builds in environments that do
not support such functionality.

Having it as a configure option allows the configure script to -test
whether the toolchain supports it- and then either fail (enable=yes)
or not use it (enable=probe) and continue the build without it.

> It should not go upstream nor into debian.

There is no reason it cannot be implemented as a build option in the
upstream package. Then you can get rid of all your nasty hacks and
simply add --enable-cf-protections to your distro's configure
options.

And other distros that also support all this functionality can use
it to. Please play nice with others and do things the right way
instead of making silly claims about how "nobody else can use this"
when it's clear that they can if they also tick all the necessary
boxes.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: NACK Re: [PATCH 2/4] debian: Enable CET on amd64

[Dimitri John Ledkov]

On Sun, Feb 21, 2021 at 4:28 AM Dave Chinner <david@fromorbit.com> wrote:
>
> On Sun, Feb 21, 2021 at 04:02:55AM +0000, Dimitri John Ledkov wrote:
> > The patch in question is specific to Ubuntu and was not submitted by
> > me to neither Debian or Upstream.
> >
> > Indeed, this is very distro specific, because of all the other things
> > that we turn on by default in our toolchain, dpkg build flags, and all
> > other packages.
> >
> > This patch if taken at face value, will not enable CET. And will make
> > the package start failing to build from source, when using older
> > toolchains that don't support said flag.
>
> Yes, that is exactly what I said when pointing out how to *support
> this properly* so it doesn't break builds in environments that do
> not support such functionality.
>
> Having it as a configure option allows the configure script to -test
> whether the toolchain supports it- and then either fail (enable=yes)
> or not use it (enable=probe) and continue the build without it.
>
> > It should not go upstream nor into debian.
>
> There is no reason it cannot be implemented as a build option in the
> upstream package. Then you can get rid of all your nasty hacks and
> simply add --enable-cf-protections to your distro's configure
> options.
>
> And other distros that also support all this functionality can use
> it to. Please play nice with others and do things the right way
> instead of making silly claims about how "nobody else can use this"
> when it's clear that they can if they also tick all the necessary
> boxes.

debian will probably will not want --enable-cf-protections as a
configure option, and will enable CET via dpkg-buildflags as a
hardening option, which will then be turned on by default for relevant
architectures and series as of when debian kernel starts to support
it.

as that way, debian will be able to affect that change across the whole distro.

once CET is actually merged into kernel, I do not expect configure
options or debian/rules changes to enable CET. At most `export
DEB_BUILD_MAINT_OPTIONS=hardening` should be enough in debian/rules.

-- 
Regards,

Dimitri.

Re: NACK Re: [PATCH 2/4] debian: Enable CET on amd64

[Dave Chinner]

On Sun, Feb 21, 2021 at 04:32:46AM +0000, Dimitri John Ledkov wrote:
> On Sun, Feb 21, 2021 at 4:28 AM Dave Chinner <david@fromorbit.com> wrote:
> >
> > On Sun, Feb 21, 2021 at 04:02:55AM +0000, Dimitri John Ledkov wrote:
> > > The patch in question is specific to Ubuntu and was not submitted by
> > > me to neither Debian or Upstream.
> > >
> > > Indeed, this is very distro specific, because of all the other things
> > > that we turn on by default in our toolchain, dpkg build flags, and all
> > > other packages.
> > >
> > > This patch if taken at face value, will not enable CET. And will make
> > > the package start failing to build from source, when using older
> > > toolchains that don't support said flag.
> >
> > Yes, that is exactly what I said when pointing out how to *support
> > this properly* so it doesn't break builds in environments that do
> > not support such functionality.
> >
> > Having it as a configure option allows the configure script to -test
> > whether the toolchain supports it- and then either fail (enable=yes)
> > or not use it (enable=probe) and continue the build without it.
> >
> > > It should not go upstream nor into debian.
> >
> > There is no reason it cannot be implemented as a build option in the
> > upstream package. Then you can get rid of all your nasty hacks and
> > simply add --enable-cf-protections to your distro's configure
> > options.
> >
> > And other distros that also support all this functionality can use
> > it to. Please play nice with others and do things the right way
> > instead of making silly claims about how "nobody else can use this"
> > when it's clear that they can if they also tick all the necessary
> > boxes.
> 
> debian will probably will not want --enable-cf-protections as a
> configure option, and will enable CET via dpkg-buildflags as a
> hardening option, which will then be turned on by default for relevant
> architectures and series as of when debian kernel starts to support
> it.
> 
> as that way, debian will be able to affect that change across the whole distro.

Except now you've got the problem that dpkg-buildflags is not used
by various packages such as xfsprogs, so the flags it sets up,
even with overrides like DEB_BUILD_MAINT_OPTIONS are completely
ignored by the debian package build because debian/rules is not set
up to source the buildflags at all.

e.g. if you run 'make Q= deb' you won't see any of the compiler or
linker options emitted by dpkg-buildflags being used during the
debian package build. You will, OTOH, see it emit stuff like LTO
options because those get turned on via configure.

> once CET is actually merged into kernel, I do not expect configure
> options or debian/rules changes to enable CET. At most `export
> DEB_BUILD_MAINT_OPTIONS=hardening` should be enough in debian/rules.

The default dpkg-buildflags output already includes "hardening"
options, and as I said above, they aren't actually included in the
build rules. Hence what you suggest just won't work like you think
it should.

Again, if you want build option stuff to work correctly, don't rely
on distro specific magic to turn stuff on because it just doesn't
work for everyone. Use configure options to enable, probe and detect
support and enable the feature in the build, then upstream and other
distros can actually build with it and test it outside the magical
unicorn distro build environment you are running in...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[PATCH 3/4] debian: Regenerate config.guess using debhelper

[Bastian Germann]

This is a change introduced in 5.10.0-2ubuntu2 with the changelog:

> xfsprogs upstream has regressed config.guess, so use
> dh_update_autotools_config.

Reported-by: Steve Langasek <steve.langasek@ubuntu.com>
Signed-off-by: Bastian Germann <bastiangermann@fishpost.de>
---
 debian/changelog | 3 +++
 debian/rules     | 1 +
 2 files changed, 4 insertions(+)

diff --git a/debian/changelog b/debian/changelog
index c77f04ab..6cc9926b 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -4,6 +4,9 @@ xfsprogs (5.11.0-rc0-1) experimental; urgency=medium
   * Drop trying to create upstream distribution
   * Enable CET on amd64
 
+  [ Steve Langasek ]
+  * Regenerate config.guess using debhelper
+
  -- Bastian Germann <bastiangermann@fishpost.de>  Sat, 20 Feb 2021 11:57:31 +0100
 
 xfsprogs (5.10.0-3) unstable; urgency=medium
diff --git a/debian/rules b/debian/rules
index dd093f2c..1913ccb6 100755
--- a/debian/rules
+++ b/debian/rules
@@ -49,6 +49,7 @@ config: .census
 	@echo "== dpkg-buildpackage: configure" 1>&2
 	$(checkdir)
 	AUTOHEADER=/bin/true dh_autoreconf
+	dh_update_autotools_config
 	$(options) $(MAKE) $(PMAKEFLAGS) include/platform_defs.h
 	touch .census
 
-- 
2.30.1

Re: [PATCH 3/4] debian: Regenerate config.guess using debhelper

[Dave Chinner]

On Sat, Feb 20, 2021 at 01:16:08PM +0100, Bastian Germann wrote:
> This is a change introduced in 5.10.0-2ubuntu2 with the changelog:
> 
> > xfsprogs upstream has regressed config.guess, so use
> > dh_update_autotools_config.

What regression?

The xfsprogs build generates config.guess with the libtool
infrastructure installed on the build machine. So I'm not sure
how/what we've regressed here, because AFAIK we haven't changed
anything here recently...

> Reported-by: Steve Langasek <steve.langasek@ubuntu.com>
> Signed-off-by: Bastian Germann <bastiangermann@fishpost.de>
> ---
>  debian/changelog | 3 +++
>  debian/rules     | 1 +
>  2 files changed, 4 insertions(+)
> 
> diff --git a/debian/changelog b/debian/changelog
> index c77f04ab..6cc9926b 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -4,6 +4,9 @@ xfsprogs (5.11.0-rc0-1) experimental; urgency=medium
>    * Drop trying to create upstream distribution
>    * Enable CET on amd64
>  
> +  [ Steve Langasek ]
> +  * Regenerate config.guess using debhelper
> +
>   -- Bastian Germann <bastiangermann@fishpost.de>  Sat, 20 Feb 2021 11:57:31 +0100
>  
>  xfsprogs (5.10.0-3) unstable; urgency=medium
> diff --git a/debian/rules b/debian/rules
> index dd093f2c..1913ccb6 100755
> --- a/debian/rules
> +++ b/debian/rules
> @@ -49,6 +49,7 @@ config: .census
>  	@echo "== dpkg-buildpackage: configure" 1>&2
>  	$(checkdir)
>  	AUTOHEADER=/bin/true dh_autoreconf
> +	dh_update_autotools_config
>  	$(options) $(MAKE) $(PMAKEFLAGS) include/platform_defs.h

Why would running at tool that does a search-n-replace of built
config.guess files do anything when run before the build runs
libtoolize to copy in the config.guess file it uses? I'm a bit
confused by this...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 3/4] debian: Regenerate config.guess using debhelper

[Steve Langasek]

On February 20, 2021 8:11:39 PM PST, Dave Chinner <david@fromorbit.com> wrote:
>On Sat, Feb 20, 2021 at 01:16:08PM +0100, Bastian Germann wrote:
>> This is a change introduced in 5.10.0-2ubuntu2 with the changelog:
>> 
>> > xfsprogs upstream has regressed config.guess, so use
>> > dh_update_autotools_config.
>
>What regression?
>
>The xfsprogs build generates config.guess with the libtool
>infrastructure installed on the build machine. So I'm not sure
>how/what we've regressed here, because AFAIK we haven't changed
>anything here recently...

This upstream release ended up with an older version of config.guess in the tarball.  Specifically, it was too old to recognize RISC-V as an architecture.

>> Reported-by: Steve Langasek <steve.langasek@ubuntu.com>
>> Signed-off-by: Bastian Germann <bastiangermann@fishpost.de>
>> ---
>>  debian/changelog | 3 +++
>>  debian/rules     | 1 +
>>  2 files changed, 4 insertions(+)
>> 
>> diff --git a/debian/changelog b/debian/changelog
>> index c77f04ab..6cc9926b 100644
>> --- a/debian/changelog
>> +++ b/debian/changelog
>> @@ -4,6 +4,9 @@ xfsprogs (5.11.0-rc0-1) experimental; urgency=medium
>>    * Drop trying to create upstream distribution
>>    * Enable CET on amd64
>>  
>> +  [ Steve Langasek ]
>> +  * Regenerate config.guess using debhelper
>> +
>>   -- Bastian Germann <bastiangermann@fishpost.de>  Sat, 20 Feb 2021
>11:57:31 +0100
>>  
>>  xfsprogs (5.10.0-3) unstable; urgency=medium
>> diff --git a/debian/rules b/debian/rules
>> index dd093f2c..1913ccb6 100755
>> --- a/debian/rules
>> +++ b/debian/rules
>> @@ -49,6 +49,7 @@ config: .census
>>  	@echo "== dpkg-buildpackage: configure" 1>&2
>>  	$(checkdir)
>>  	AUTOHEADER=/bin/true dh_autoreconf
>> +	dh_update_autotools_config
>>  	$(options) $(MAKE) $(PMAKEFLAGS) include/platform_defs.h
>
>Why would running at tool that does a search-n-replace of built
>config.guess files do anything when run before the build runs
>libtoolize to copy in the config.guess file it uses? I'm a bit
>confused by this...

Autoreconf was not copying in a newer version of config.guess from the system, because of the specific subset of autotools used by this project.

-- 
Steve Langasek

Re: [PATCH 3/4] debian: Regenerate config.guess using debhelper

[Dave Chinner]

On Sat, Feb 20, 2021 at 11:16:57PM -0800, Steve Langasek wrote:
> 
> 
> On February 20, 2021 8:11:39 PM PST, Dave Chinner <david@fromorbit.com> wrote:
> >On Sat, Feb 20, 2021 at 01:16:08PM +0100, Bastian Germann wrote:
> >> This is a change introduced in 5.10.0-2ubuntu2 with the changelog:
> >> 
> >> > xfsprogs upstream has regressed config.guess, so use
> >> > dh_update_autotools_config.
> >
> >What regression?
> >
> >The xfsprogs build generates config.guess with the libtool
> >infrastructure installed on the build machine. So I'm not sure
> >how/what we've regressed here, because AFAIK we haven't changed
> >anything here recently...
> 
> This upstream release ended up with an older version of config.guess in the tarball.  Specifically, it was too old to recognize RISC-V as an architecture.

So was the RISC-V architecture added to the ubuntu build between the
uploads of the previous version of xfsprogs and xfsprogs-5.10.0? Or
is this an actual regression where the maintainer signed tarball had
RISC-V support in it and now it doesn't?

FWIW, where are you getting the tarballs from? Are you downloading
the maintainer signed tarballs from here:

https://mirrors.edge.kernel.org/pub/linux/utils/fs/xfs/xfsprogs/

Or pulling the git.kernel.org built tarball that is autogenerated
from the release tag?

> >>  	@echo "== dpkg-buildpackage: configure" 1>&2
> >>  	$(checkdir)
> >>  	AUTOHEADER=/bin/true dh_autoreconf
> >> +	dh_update_autotools_config
> >>  	$(options) $(MAKE) $(PMAKEFLAGS) include/platform_defs.h
> >
> >Why would running at tool that does a search-n-replace of built
> >config.guess files do anything when run before the build runs
> >libtoolize to copy in the config.guess file it uses? I'm a bit
> >confused by this...
> 
> Autoreconf was not copying in a newer version of config.guess from the system, because of the specific subset of autotools used by this project.

That sounds wrong. AIUI, autoreconf doesn't copy in anything.

$ man 7 dh-autoreconf
....
CAVEATS
       dh_autoreconf is mostly a superset of the
       dh_update_autotools_config debhelper command included in
       debhelper since version 9.20160115. When using the dh
       sequencer, dh_update_autotools_config is run before
       dh_autoreconf and updates the config.guess and config.sub
       files. This is required in cases where autoreconf does not
       update config.guess and config.sub itself.

So isn't the dh_update_autotools_config call in the wrong place
here?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 3/4] debian: Regenerate config.guess using debhelper

[Steve Langasek]

[-- Attachment #1: Type: text/plain, Size: 2941 bytes --]

On Mon, Feb 22, 2021 at 09:04:43AM +1100, Dave Chinner wrote:

> > This upstream release ended up with an older version of config.guess in
> > the tarball.  Specifically, it was too old to recognize RISC-V as an
> > architecture.

> So was the RISC-V architecture added to the ubuntu build between the
> uploads of the previous version of xfsprogs and xfsprogs-5.10.0? Or
> is this an actual regression where the maintainer signed tarball had
> RISC-V support in it and now it doesn't?

This is a regression.  The previous tarball (5.6.0) had a newer config.guess
that recognized RISC-V, the newer one (5.10.0) had an older config.guess.

> FWIW, where are you getting the tarballs from? Are you downloading
> the maintainer signed tarballs from here:

> https://mirrors.edge.kernel.org/pub/linux/utils/fs/xfs/xfsprogs/

> Or pulling the git.kernel.org built tarball that is autogenerated
> from the release tag?

I don't know the answer to this; we in Ubuntu take the tarballs from Debian.

> > >>  	@echo "== dpkg-buildpackage: configure" 1>&2
> > >>  	$(checkdir)
> > >>  	AUTOHEADER=/bin/true dh_autoreconf
> > >> +	dh_update_autotools_config
> > >>  	$(options) $(MAKE) $(PMAKEFLAGS) include/platform_defs.h

> > >Why would running at tool that does a search-n-replace of built
> > >config.guess files do anything when run before the build runs
> > >libtoolize to copy in the config.guess file it uses? I'm a bit
> > >confused by this...

> > Autoreconf was not copying in a newer version of config.guess from the
> > system, because of the specific subset of autotools used by this
> > project.

> That sounds wrong. AIUI, autoreconf doesn't copy in anything.

> $ man 7 dh-autoreconf
> ....
> CAVEATS
>        dh_autoreconf is mostly a superset of the
>        dh_update_autotools_config debhelper command included in
>        debhelper since version 9.20160115. When using the dh
>        sequencer, dh_update_autotools_config is run before
>        dh_autoreconf and updates the config.guess and config.sub
>        files. This is required in cases where autoreconf does not
>        update config.guess and config.sub itself.

> So isn't the dh_update_autotools_config call in the wrong place
> here?

Documentation notwithstanding, dh_autoreconf was definitively NOT copying in
the newer config.guess, and we have the build logs to prove it.

https://launchpad.net/ubuntu/+source/xfsprogs/5.10.0-2ubuntu1/+build/20952006

I don't know in what sense this would be the "wrong" place to call it,
because it fixes the build failure, which is what I care about.

-- 
Steve Langasek                   Give me a lever long enough and a Free OS
Debian Developer                   to set it on, and I can move the world.
Ubuntu Developer                                   https://www.debian.org/
slangasek@ubuntu.com                                     vorlon@debian.org

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/4] debian: Regenerate config.guess using debhelper

[Dave Chinner]

On Sun, Feb 21, 2021 at 04:16:39PM -0800, Steve Langasek wrote:
> On Mon, Feb 22, 2021 at 09:04:43AM +1100, Dave Chinner wrote:
> 
> > > This upstream release ended up with an older version of config.guess in
> > > the tarball.  Specifically, it was too old to recognize RISC-V as an
> > > architecture.
> 
> > So was the RISC-V architecture added to the ubuntu build between the
> > uploads of the previous version of xfsprogs and xfsprogs-5.10.0? Or
> > is this an actual regression where the maintainer signed tarball had
> > RISC-V support in it and now it doesn't?
> 
> This is a regression.  The previous tarball (5.6.0) had a newer config.guess
> that recognized RISC-V, the newer one (5.10.0) had an older config.guess.

Ok.

Eric, did you change the machine you did the release build from?

> > $ man 7 dh-autoreconf
> > ....
> > CAVEATS
> >        dh_autoreconf is mostly a superset of the
> >        dh_update_autotools_config debhelper command included in
> >        debhelper since version 9.20160115. When using the dh
> >        sequencer, dh_update_autotools_config is run before
> >        dh_autoreconf and updates the config.guess and config.sub
> >        files. This is required in cases where autoreconf does not
> >        update config.guess and config.sub itself.
> 
> > So isn't the dh_update_autotools_config call in the wrong place
> > here?
> 
> Documentation notwithstanding, dh_autoreconf was definitively NOT copying in
> the newer config.guess, and we have the build logs to prove it.
> 
> https://launchpad.net/ubuntu/+source/xfsprogs/5.10.0-2ubuntu1/+build/20952006

I'm not denying that it fixed your problem. As a reviewer, I'm
supposed to point out where the proposed fix doesn't match with
expected usage. And as a reviewer, I expect that patched get changed
to address problems that are found before they are committed, not to
be told:

> I don't know in what sense this would be the "wrong" place to call it,
> because it fixes the build failure, which is what I care about.

"It made my problem go away, so my job is done."

You didn't report it upstream so we could avoid the regression in
future, to either the debian package maintainers or the XFS lists.
Nor do you seem to care if the fix is correct or whether anyone else
might need it.

In future, if there is a regression in the upstream xfsprogs
package, please report it upstream and not just keep it to yourself
and hack around it.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 3/4] debian: Regenerate config.guess using debhelper

[Eric Sandeen]

On 2/21/21 8:44 PM, Dave Chinner wrote:
> On Sun, Feb 21, 2021 at 04:16:39PM -0800, Steve Langasek wrote:
>> On Mon, Feb 22, 2021 at 09:04:43AM +1100, Dave Chinner wrote:
>>
>>>> This upstream release ended up with an older version of config.guess in
>>>> the tarball.  Specifically, it was too old to recognize RISC-V as an
>>>> architecture.
>>
>>> So was the RISC-V architecture added to the ubuntu build between the
>>> uploads of the previous version of xfsprogs and xfsprogs-5.10.0? Or
>>> is this an actual regression where the maintainer signed tarball had
>>> RISC-V support in it and now it doesn't?
>>
>> This is a regression.  The previous tarball (5.6.0) had a newer config.guess
>> that recognized RISC-V, the newer one (5.10.0) had an older config.guess.
> 
> Ok.
> 
> Eric, did you change the machine you did the release build from?

I don't recall doing so, but I must have. I guess I remember this coming up
a while ago and maybe I failed to change process in a sticky way.  :/

But - if my local toolchain can cause a regression in a major distro, it seems
like this patch to regenerate is the obvious path forward, to control
the distro-specific build, and not be subject to my personal toolchain whims.

Is that not best practice? (I honestly don't know.)

-Eric

Re: [PATCH 3/4] debian: Regenerate config.guess using debhelper

[Dave Chinner]

On Mon, Feb 22, 2021 at 01:23:12PM -0600, Eric Sandeen wrote:
> On 2/21/21 8:44 PM, Dave Chinner wrote:
> > On Sun, Feb 21, 2021 at 04:16:39PM -0800, Steve Langasek wrote:
> >> On Mon, Feb 22, 2021 at 09:04:43AM +1100, Dave Chinner wrote:
> >>
> >>>> This upstream release ended up with an older version of config.guess in
> >>>> the tarball.  Specifically, it was too old to recognize RISC-V as an
> >>>> architecture.
> >>
> >>> So was the RISC-V architecture added to the ubuntu build between the
> >>> uploads of the previous version of xfsprogs and xfsprogs-5.10.0? Or
> >>> is this an actual regression where the maintainer signed tarball had
> >>> RISC-V support in it and now it doesn't?
> >>
> >> This is a regression.  The previous tarball (5.6.0) had a newer config.guess
> >> that recognized RISC-V, the newer one (5.10.0) had an older config.guess.
> > 
> > Ok.
> > 
> > Eric, did you change the machine you did the release build from?
> 
> I don't recall doing so, but I must have. I guess I remember this coming up
> a while ago and maybe I failed to change process in a sticky way.  :/
> 
> But - if my local toolchain can cause a regression in a major distro, it seems
> like this patch to regenerate is the obvious path forward, to control
> the distro-specific build, and not be subject to my personal toolchain whims.

I think it's on the package maintainer to ensure what is uploaded as
the official distro package contains the correct tarball contents
for their distro. If the distro build overrides shipped config
files, then there shouldn't be any problems at all with just using
the upstream tarball.

> Is that not best practice? (I honestly don't know.)

Honestly, I think building packages from tarballs is an anachronism
of a time gone by. Debian xfsprogs packages can be build directly
from a clean git tree and none of these problems exist in that case.
i.e. a clean git tree requires the build to libtoolize/autoconf and
set up the source tree from it's environment before anything else,
hence there's nothing that needs to be overridden...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[PATCH 4/4] debian: Build-depend on libinih-dev with udeb package

[Bastian Germann]

The first libinih Debian package version with udeb binary package is 53-1.
Debian bug #981662 documents the need for it:
xfsprogs-udeb depends on libinih1, not libinih1-udeb

Link: https://bugs.debian.org/981662
Signed-off-by: Bastian Germann <bastiangermann@fishpost.de>
---
 debian/changelog | 3 +++
 debian/control   | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/debian/changelog b/debian/changelog
index 6cc9926b..1879485d 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -7,6 +7,9 @@ xfsprogs (5.11.0-rc0-1) experimental; urgency=medium
   [ Steve Langasek ]
   * Regenerate config.guess using debhelper
 
+  [ Bastian Germann ]
+  * Build-depend on libinih-dev with udeb package
+
  -- Bastian Germann <bastiangermann@fishpost.de>  Sat, 20 Feb 2021 11:57:31 +0100
 
 xfsprogs (5.10.0-3) unstable; urgency=medium
diff --git a/debian/control b/debian/control
index 1da8093d..e4ec897c 100644
--- a/debian/control
+++ b/debian/control
@@ -3,7 +3,7 @@ Section: admin
 Priority: optional
 Maintainer: XFS Development Team <linux-xfs@vger.kernel.org>
 Uploaders: Nathan Scott <nathans@debian.org>, Anibal Monsalve Salazar <anibal@debian.org>, Bastian Germann <bastiangermann@fishpost.de>
-Build-Depends: libinih-dev, uuid-dev, dh-autoreconf, debhelper (>= 5), gettext, libtool, libedit-dev, libblkid-dev (>= 2.17), linux-libc-dev, libdevmapper-dev, libattr1-dev, libicu-dev, pkg-config
+Build-Depends: libinih-dev (>= 53), uuid-dev, dh-autoreconf, debhelper (>= 5), gettext, libtool, libedit-dev, libblkid-dev (>= 2.17), linux-libc-dev, libdevmapper-dev, libattr1-dev, libicu-dev, pkg-config
 Standards-Version: 4.0.0
 Homepage: https://xfs.wiki.kernel.org/
 
-- 
2.30.1