All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] debian: Fix xfsprogs FT(C)BFS
@ 2021-11-19 17:12 Bastian Germann
  2021-11-19 17:12 ` [PATCH 1/2] debian: Generate .gitcensus instead of .census (Closes: #999743) Bastian Germann
  2021-11-19 17:12 ` [PATCH 2/2] debian: Fix FTCBFS: Skip crc32 test (Closes: #999879) Bastian Germann
  0 siblings, 2 replies; 13+ messages in thread
From: Bastian Germann @ 2021-11-19 17:12 UTC (permalink / raw)
  To: linux-xfs; +Cc: Bastian Germann

Hi,

I am going to include these two patches for the unstable Debian upload
of v5.14.0-1.

Thanks,
Bastian

Bastian Germann (2):
  debian: Generate .gitcensus instead of .census (Closes: #999743)
  debian: Fix FTCBFS: Skip crc32 test (Closes: #999879)

 debian/rules | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

-- 
2.30.2



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/2] debian: Generate .gitcensus instead of .census (Closes: #999743)
  2021-11-19 17:12 [PATCH 0/2] debian: Fix xfsprogs FT(C)BFS Bastian Germann
@ 2021-11-19 17:12 ` Bastian Germann
  2021-11-23 20:01   ` Bastian Germann
  2021-11-23 20:08   ` Darrick J. Wong
  2021-11-19 17:12 ` [PATCH 2/2] debian: Fix FTCBFS: Skip crc32 test (Closes: #999879) Bastian Germann
  1 sibling, 2 replies; 13+ messages in thread
From: Bastian Germann @ 2021-11-19 17:12 UTC (permalink / raw)
  To: linux-xfs; +Cc: Bastian Germann

Fix the Debian build outside a git tree (e.g., Debian archive builds) by
creating an empty .gitcensus instead of .census file on config.

Signed-off-by: Bastian Germann <bage@debian.org>
---
 debian/rules | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/debian/rules b/debian/rules
index 615289b4..6d5b82a8 100755
--- a/debian/rules
+++ b/debian/rules
@@ -43,15 +43,15 @@ built: dibuild config
 	$(MAKE) $(PMAKEFLAGS) default
 	touch built
 
-config: .census
-.census:
+config: .gitcensus
+.gitcensus:
 	@echo "== dpkg-buildpackage: configure" 1>&2
 	$(checkdir)
 	AUTOHEADER=/bin/true dh_autoreconf
 	dh_update_autotools_config
 	$(options) $(MAKE) $(PMAKEFLAGS) include/platform_defs.h
 	cp -f include/install-sh .
-	touch .census
+	touch .gitcensus
 
 dibuild:
 	$(checkdir)
@@ -72,7 +72,7 @@ dibuild:
 clean:
 	@echo "== dpkg-buildpackage: clean" 1>&2
 	$(checkdir)
-	-rm -f built .census mkfs/mkfs.xfs-$(bootpkg)
+	-rm -f built .gitcensus mkfs/mkfs.xfs-$(bootpkg)
 	$(MAKE) distclean
 	-rm -rf $(dirme) $(dirdev) $(dirdi)
 	-rm -f debian/*substvars debian/files* debian/*.debhelper
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/2] debian: Fix FTCBFS: Skip crc32 test (Closes: #999879)
  2021-11-19 17:12 [PATCH 0/2] debian: Fix xfsprogs FT(C)BFS Bastian Germann
  2021-11-19 17:12 ` [PATCH 1/2] debian: Generate .gitcensus instead of .census (Closes: #999743) Bastian Germann
@ 2021-11-19 17:12 ` Bastian Germann
  2021-11-19 23:11   ` Dave Chinner
  1 sibling, 1 reply; 13+ messages in thread
From: Bastian Germann @ 2021-11-19 17:12 UTC (permalink / raw)
  To: linux-xfs; +Cc: Bastian Germann, Helmut Grohne

xfsprogs fails to cross build from source, because it attempts to build
its crc32 test with the build architecture compiler and thus fails
finding the liburcu, which is only requested for the host architecture.
While this test is useful for native builds, it is not that useful for
cross builds. Skip it by pre-creating the output file.

Reported-by: Helmut Grohne <helmut@subdivi.de>
Signed-off-by: Bastian Germann <bage@debian.org>
---
 debian/rules | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/debian/rules b/debian/rules
index 6d5b82a8..28cc414d 100755
--- a/debian/rules
+++ b/debian/rules
@@ -40,6 +40,9 @@ build-arch: built
 build-indep: built
 built: dibuild config
 	@echo "== dpkg-buildpackage: build" 1>&2
+ifneq ($(DEB_BUILD_ARCH),$(DEB_HOST_ARCH))
+	touch --date=+3day libfrog/crc32selftest
+endif
 	$(MAKE) $(PMAKEFLAGS) default
 	touch built
 
-- 
2.30.2



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] debian: Fix FTCBFS: Skip crc32 test (Closes: #999879)
  2021-11-19 17:12 ` [PATCH 2/2] debian: Fix FTCBFS: Skip crc32 test (Closes: #999879) Bastian Germann
@ 2021-11-19 23:11   ` Dave Chinner
  2021-11-20  7:32     ` Helmut Grohne
  0 siblings, 1 reply; 13+ messages in thread
From: Dave Chinner @ 2021-11-19 23:11 UTC (permalink / raw)
  To: Bastian Germann; +Cc: linux-xfs, Helmut Grohne

On Fri, Nov 19, 2021 at 06:12:41PM +0100, Bastian Germann wrote:
> xfsprogs fails to cross build from source, because it attempts to build
> its crc32 test with the build architecture compiler and thus fails
> finding the liburcu, which is only requested for the host architecture.
> While this test is useful for native builds, it is not that useful for
> cross builds. Skip it by pre-creating the output file.

I don't get it. The crcselftest does not use liburcu in
any way, nor does it try to link against liburcu, so it should not
fail because other parts of xfsprogs use liburcu.

What's the build error that occurs?

> Reported-by: Helmut Grohne <helmut@subdivi.de>
> Signed-off-by: Bastian Germann <bage@debian.org>
> ---
>  debian/rules | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/debian/rules b/debian/rules
> index 6d5b82a8..28cc414d 100755
> --- a/debian/rules
> +++ b/debian/rules
> @@ -40,6 +40,9 @@ build-arch: built
>  build-indep: built
>  built: dibuild config
>  	@echo "== dpkg-buildpackage: build" 1>&2
> +ifneq ($(DEB_BUILD_ARCH),$(DEB_HOST_ARCH))
> +	touch --date=+3day libfrog/crc32selftest
> +endif
>  	$(MAKE) $(PMAKEFLAGS) default
>  	touch built

We need to fix the generic cross-build problem in the xfsprogs code,
not slap a distro-specific build band-aid over it.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] debian: Fix FTCBFS: Skip crc32 test (Closes: #999879)
  2021-11-19 23:11   ` Dave Chinner
@ 2021-11-20  7:32     ` Helmut Grohne
  2021-11-20 17:15       ` Darrick J. Wong
  0 siblings, 1 reply; 13+ messages in thread
From: Helmut Grohne @ 2021-11-20  7:32 UTC (permalink / raw)
  To: Dave Chinner; +Cc: Bastian Germann, linux-xfs

Hi Dave,

On Sat, Nov 20, 2021 at 10:11:05AM +1100, Dave Chinner wrote:
> I don't get it. The crcselftest does not use liburcu in
> any way, nor does it try to link against liburcu, so it should not
> fail because other parts of xfsprogs use liburcu.
> 
> What's the build error that occurs?

As the build log shows, that's not technically accurate. You can find
logs of test builds for various architecture combinations at
http://crossqa.debian.net/src/xfsprogs. This is also available as a link
called "cross" in https://tracker.debian.org/xfsprogs.

The relevant part is:
|     [TEST]    CRC32
| In file included from crc32.c:35:
| ../include/platform_defs.h:27:10: fatal error: urcu.h: No such file or directory
|    27 | #include <urcu.h>
|       |          ^~~~~~~~
| compilation terminated.

I failed to figure a good way of dropping either include directive.

> We need to fix the generic cross-build problem in the xfsprogs code,
> not slap a distro-specific build band-aid over it.

I fully agree with this in principle. However, when I fail to find that
upstreamable solution, I try to at least provide a Debian-specific
solution to iterate from.

Can you propose a way to drop either #include?

Helmut


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] debian: Fix FTCBFS: Skip crc32 test (Closes: #999879)
  2021-11-20  7:32     ` Helmut Grohne
@ 2021-11-20 17:15       ` Darrick J. Wong
  2021-11-20 22:20         ` Dave Chinner
  0 siblings, 1 reply; 13+ messages in thread
From: Darrick J. Wong @ 2021-11-20 17:15 UTC (permalink / raw)
  To: Helmut Grohne; +Cc: Dave Chinner, Bastian Germann, linux-xfs

On Sat, Nov 20, 2021 at 08:32:40AM +0100, Helmut Grohne wrote:
> Hi Dave,
> 
> On Sat, Nov 20, 2021 at 10:11:05AM +1100, Dave Chinner wrote:
> > I don't get it. The crcselftest does not use liburcu in
> > any way, nor does it try to link against liburcu, so it should not
> > fail because other parts of xfsprogs use liburcu.
> > 
> > What's the build error that occurs?
> 
> As the build log shows, that's not technically accurate. You can find
> logs of test builds for various architecture combinations at
> http://crossqa.debian.net/src/xfsprogs. This is also available as a link
> called "cross" in https://tracker.debian.org/xfsprogs.
> 
> The relevant part is:
> |     [TEST]    CRC32
> | In file included from crc32.c:35:
> | ../include/platform_defs.h:27:10: fatal error: urcu.h: No such file or directory
> |    27 | #include <urcu.h>
> |       |          ^~~~~~~~
> | compilation terminated.
> 
> I failed to figure a good way of dropping either include directive.
> 
> > We need to fix the generic cross-build problem in the xfsprogs code,
> > not slap a distro-specific build band-aid over it.
> 
> I fully agree with this in principle. However, when I fail to find that
> upstreamable solution, I try to at least provide a Debian-specific
> solution to iterate from.
> 
> Can you propose a way to drop either #include?

Frankly I don't really see the value in crc32selftest when cross
compiling -- sure, the crc32c code works correctly on the build host,
but that proves nothing about the (cross-)built binaries that end up in
the package.

The selftest code is modular enough that it's included in xfs_io and it
seems to run in under 500us even on my ancient raspberry pi 3b+
downclocked to 600mhz.  Why don't we just add it to mkfs and repair?
Those tools shouldn't be run all that frequently, and now we'll know
immediately if crc32c is broken on a user's system.

Then we don't need the build-time selftest at all.

--D

> Helmut
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] debian: Fix FTCBFS: Skip crc32 test (Closes: #999879)
  2021-11-20 17:15       ` Darrick J. Wong
@ 2021-11-20 22:20         ` Dave Chinner
  2021-11-20 22:27           ` Dave Chinner
  2021-11-22 23:31           ` Darrick J. Wong
  0 siblings, 2 replies; 13+ messages in thread
From: Dave Chinner @ 2021-11-20 22:20 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Helmut Grohne, Bastian Germann, linux-xfs

On Sat, Nov 20, 2021 at 09:15:48AM -0800, Darrick J. Wong wrote:
> On Sat, Nov 20, 2021 at 08:32:40AM +0100, Helmut Grohne wrote:
> > Hi Dave,
> > 
> > On Sat, Nov 20, 2021 at 10:11:05AM +1100, Dave Chinner wrote:
> > > I don't get it. The crcselftest does not use liburcu in
> > > any way, nor does it try to link against liburcu, so it should not
> > > fail because other parts of xfsprogs use liburcu.
> > > 
> > > What's the build error that occurs?
> > 
> > As the build log shows, that's not technically accurate. You can find
> > logs of test builds for various architecture combinations at
> > http://crossqa.debian.net/src/xfsprogs. This is also available as a link
> > called "cross" in https://tracker.debian.org/xfsprogs.
> > 
> > The relevant part is:
> > |     [TEST]    CRC32
> > | In file included from crc32.c:35:
> > | ../include/platform_defs.h:27:10: fatal error: urcu.h: No such file or directory
> > |    27 | #include <urcu.h>
> > |       |          ^~~~~~~~
> > | compilation terminated.
> > 
> > I failed to figure a good way of dropping either include directive.
> > 
> > > We need to fix the generic cross-build problem in the xfsprogs code,
> > > not slap a distro-specific build band-aid over it.
> > 
> > I fully agree with this in principle. However, when I fail to find that
> > upstreamable solution, I try to at least provide a Debian-specific
> > solution to iterate from.
> > 
> > Can you propose a way to drop either #include?
> 
> Frankly I don't really see the value in crc32selftest when cross
> compiling -- sure, the crc32c code works correctly on the build host,
> but that proves nothing about the (cross-)built binaries that end up in
> the package.

Yup, that's pretty much what I was getting at.

> The selftest code is modular enough that it's included in xfs_io and it
> seems to run in under 500us even on my ancient raspberry pi 3b+
> downclocked to 600mhz.  Why don't we just add it to mkfs and repair?
> Those tools shouldn't be run all that frequently, and now we'll know
> immediately if crc32c is broken on a user's system.
> 
> Then we don't need the build-time selftest at all.

Sounds reasonable, my only concern is what do users do when
xfs-repair fails the self test and they need to fix their
filesystem?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] debian: Fix FTCBFS: Skip crc32 test (Closes: #999879)
  2021-11-20 22:20         ` Dave Chinner
@ 2021-11-20 22:27           ` Dave Chinner
  2021-11-22 23:32             ` Darrick J. Wong
  2021-11-22 23:31           ` Darrick J. Wong
  1 sibling, 1 reply; 13+ messages in thread
From: Dave Chinner @ 2021-11-20 22:27 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Helmut Grohne, Bastian Germann, linux-xfs

On Sun, Nov 21, 2021 at 09:20:09AM +1100, Dave Chinner wrote:
> On Sat, Nov 20, 2021 at 09:15:48AM -0800, Darrick J. Wong wrote:
> > On Sat, Nov 20, 2021 at 08:32:40AM +0100, Helmut Grohne wrote:
> > > Hi Dave,
> > > 
> > > On Sat, Nov 20, 2021 at 10:11:05AM +1100, Dave Chinner wrote:
> > > > I don't get it. The crcselftest does not use liburcu in
> > > > any way, nor does it try to link against liburcu, so it should not
> > > > fail because other parts of xfsprogs use liburcu.
> > > > 
> > > > What's the build error that occurs?
> > > 
> > > As the build log shows, that's not technically accurate. You can find
> > > logs of test builds for various architecture combinations at
> > > http://crossqa.debian.net/src/xfsprogs. This is also available as a link
> > > called "cross" in https://tracker.debian.org/xfsprogs.
> > > 
> > > The relevant part is:
> > > |     [TEST]    CRC32
> > > | In file included from crc32.c:35:
> > > | ../include/platform_defs.h:27:10: fatal error: urcu.h: No such file or directory
> > > |    27 | #include <urcu.h>
> > > |       |          ^~~~~~~~
> > > | compilation terminated.
> > > 
> > > I failed to figure a good way of dropping either include directive.
> > > 
> > > > We need to fix the generic cross-build problem in the xfsprogs code,
> > > > not slap a distro-specific build band-aid over it.
> > > 
> > > I fully agree with this in principle. However, when I fail to find that
> > > upstreamable solution, I try to at least provide a Debian-specific
> > > solution to iterate from.
> > > 
> > > Can you propose a way to drop either #include?
> > 
> > Frankly I don't really see the value in crc32selftest when cross
> > compiling -- sure, the crc32c code works correctly on the build host,
> > but that proves nothing about the (cross-)built binaries that end up in
> > the package.
> 
> Yup, that's pretty much what I was getting at.

Though this is pretty simple and should just work, too:

--- a/libfrog/crc32.c
+++ b/libfrog/crc32.c
@@ -29,10 +29,11 @@
  * match the hardware acceleration available on Intel CPUs.
  */
 
+#include <stdio.h>
+#include <sys/types.h>
 #include <inttypes.h>
 #include <asm/types.h>
 #include <sys/time.h>
-#include "platform_defs.h"
 /* For endian conversion routines */
 #include "xfs_arch.h"
 #include "crc32defs.h"

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] debian: Fix FTCBFS: Skip crc32 test (Closes: #999879)
  2021-11-20 22:20         ` Dave Chinner
  2021-11-20 22:27           ` Dave Chinner
@ 2021-11-22 23:31           ` Darrick J. Wong
  2021-11-22 23:37             ` Dave Chinner
  1 sibling, 1 reply; 13+ messages in thread
From: Darrick J. Wong @ 2021-11-22 23:31 UTC (permalink / raw)
  To: Dave Chinner; +Cc: Helmut Grohne, Bastian Germann, linux-xfs

On Sun, Nov 21, 2021 at 09:20:09AM +1100, Dave Chinner wrote:
> On Sat, Nov 20, 2021 at 09:15:48AM -0800, Darrick J. Wong wrote:
> > On Sat, Nov 20, 2021 at 08:32:40AM +0100, Helmut Grohne wrote:
> > > Hi Dave,
> > > 
> > > On Sat, Nov 20, 2021 at 10:11:05AM +1100, Dave Chinner wrote:
> > > > I don't get it. The crcselftest does not use liburcu in
> > > > any way, nor does it try to link against liburcu, so it should not
> > > > fail because other parts of xfsprogs use liburcu.
> > > > 
> > > > What's the build error that occurs?
> > > 
> > > As the build log shows, that's not technically accurate. You can find
> > > logs of test builds for various architecture combinations at
> > > http://crossqa.debian.net/src/xfsprogs. This is also available as a link
> > > called "cross" in https://tracker.debian.org/xfsprogs.
> > > 
> > > The relevant part is:
> > > |     [TEST]    CRC32
> > > | In file included from crc32.c:35:
> > > | ../include/platform_defs.h:27:10: fatal error: urcu.h: No such file or directory
> > > |    27 | #include <urcu.h>
> > > |       |          ^~~~~~~~
> > > | compilation terminated.
> > > 
> > > I failed to figure a good way of dropping either include directive.
> > > 
> > > > We need to fix the generic cross-build problem in the xfsprogs code,
> > > > not slap a distro-specific build band-aid over it.
> > > 
> > > I fully agree with this in principle. However, when I fail to find that
> > > upstreamable solution, I try to at least provide a Debian-specific
> > > solution to iterate from.
> > > 
> > > Can you propose a way to drop either #include?
> > 
> > Frankly I don't really see the value in crc32selftest when cross
> > compiling -- sure, the crc32c code works correctly on the build host,
> > but that proves nothing about the (cross-)built binaries that end up in
> > the package.
> 
> Yup, that's pretty much what I was getting at.
> 
> > The selftest code is modular enough that it's included in xfs_io and it
> > seems to run in under 500us even on my ancient raspberry pi 3b+
> > downclocked to 600mhz.  Why don't we just add it to mkfs and repair?
> > Those tools shouldn't be run all that frequently, and now we'll know
> > immediately if crc32c is broken on a user's system.
> > 
> > Then we don't need the build-time selftest at all.
> 
> Sounds reasonable, my only concern is what do users do when
> xfs-repair fails the self test and they need to fix their
> filesystem?

Find a system with a non-broken CPU, or complain to their support rep or
the upstream mailing list.  I'd rather they do that than let repair
erroneously obliterate all the metadata with incorrect crc validation,
or write garbage out that fails to mount immediately after.

--D

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] debian: Fix FTCBFS: Skip crc32 test (Closes: #999879)
  2021-11-20 22:27           ` Dave Chinner
@ 2021-11-22 23:32             ` Darrick J. Wong
  0 siblings, 0 replies; 13+ messages in thread
From: Darrick J. Wong @ 2021-11-22 23:32 UTC (permalink / raw)
  To: Dave Chinner; +Cc: Helmut Grohne, Bastian Germann, linux-xfs

On Sun, Nov 21, 2021 at 09:27:19AM +1100, Dave Chinner wrote:
> On Sun, Nov 21, 2021 at 09:20:09AM +1100, Dave Chinner wrote:
> > On Sat, Nov 20, 2021 at 09:15:48AM -0800, Darrick J. Wong wrote:
> > > On Sat, Nov 20, 2021 at 08:32:40AM +0100, Helmut Grohne wrote:
> > > > Hi Dave,
> > > > 
> > > > On Sat, Nov 20, 2021 at 10:11:05AM +1100, Dave Chinner wrote:
> > > > > I don't get it. The crcselftest does not use liburcu in
> > > > > any way, nor does it try to link against liburcu, so it should not
> > > > > fail because other parts of xfsprogs use liburcu.
> > > > > 
> > > > > What's the build error that occurs?
> > > > 
> > > > As the build log shows, that's not technically accurate. You can find
> > > > logs of test builds for various architecture combinations at
> > > > http://crossqa.debian.net/src/xfsprogs. This is also available as a link
> > > > called "cross" in https://tracker.debian.org/xfsprogs.
> > > > 
> > > > The relevant part is:
> > > > |     [TEST]    CRC32
> > > > | In file included from crc32.c:35:
> > > > | ../include/platform_defs.h:27:10: fatal error: urcu.h: No such file or directory
> > > > |    27 | #include <urcu.h>
> > > > |       |          ^~~~~~~~
> > > > | compilation terminated.
> > > > 
> > > > I failed to figure a good way of dropping either include directive.
> > > > 
> > > > > We need to fix the generic cross-build problem in the xfsprogs code,
> > > > > not slap a distro-specific build band-aid over it.
> > > > 
> > > > I fully agree with this in principle. However, when I fail to find that
> > > > upstreamable solution, I try to at least provide a Debian-specific
> > > > solution to iterate from.
> > > > 
> > > > Can you propose a way to drop either #include?
> > > 
> > > Frankly I don't really see the value in crc32selftest when cross
> > > compiling -- sure, the crc32c code works correctly on the build host,
> > > but that proves nothing about the (cross-)built binaries that end up in
> > > the package.
> > 
> > Yup, that's pretty much what I was getting at.
> 
> Though this is pretty simple and should just work, too:
> 
> --- a/libfrog/crc32.c
> +++ b/libfrog/crc32.c
> @@ -29,10 +29,11 @@
>   * match the hardware acceleration available on Intel CPUs.
>   */
>  
> +#include <stdio.h>
> +#include <sys/types.h>
>  #include <inttypes.h>
>  #include <asm/types.h>
>  #include <sys/time.h>
> -#include "platform_defs.h"
>  /* For endian conversion routines */
>  #include "xfs_arch.h"
>  #include "crc32defs.h"

That works for my system, though I don't do cross builds frequently so I
don't really know if it fixes those outside of my toy environment.  I'll
send a bigger patch to add selftest to mkfs/repair tomorrow.

--D

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] debian: Fix FTCBFS: Skip crc32 test (Closes: #999879)
  2021-11-22 23:31           ` Darrick J. Wong
@ 2021-11-22 23:37             ` Dave Chinner
  0 siblings, 0 replies; 13+ messages in thread
From: Dave Chinner @ 2021-11-22 23:37 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Helmut Grohne, Bastian Germann, linux-xfs

On Mon, Nov 22, 2021 at 03:31:13PM -0800, Darrick J. Wong wrote:
> On Sun, Nov 21, 2021 at 09:20:09AM +1100, Dave Chinner wrote:
> > On Sat, Nov 20, 2021 at 09:15:48AM -0800, Darrick J. Wong wrote:
> > > On Sat, Nov 20, 2021 at 08:32:40AM +0100, Helmut Grohne wrote:
> > > > Hi Dave,
> > > > 
> > > > On Sat, Nov 20, 2021 at 10:11:05AM +1100, Dave Chinner wrote:
> > > > > I don't get it. The crcselftest does not use liburcu in
> > > > > any way, nor does it try to link against liburcu, so it should not
> > > > > fail because other parts of xfsprogs use liburcu.
> > > > > 
> > > > > What's the build error that occurs?
> > > > 
> > > > As the build log shows, that's not technically accurate. You can find
> > > > logs of test builds for various architecture combinations at
> > > > http://crossqa.debian.net/src/xfsprogs. This is also available as a link
> > > > called "cross" in https://tracker.debian.org/xfsprogs.
> > > > 
> > > > The relevant part is:
> > > > |     [TEST]    CRC32
> > > > | In file included from crc32.c:35:
> > > > | ../include/platform_defs.h:27:10: fatal error: urcu.h: No such file or directory
> > > > |    27 | #include <urcu.h>
> > > > |       |          ^~~~~~~~
> > > > | compilation terminated.
> > > > 
> > > > I failed to figure a good way of dropping either include directive.
> > > > 
> > > > > We need to fix the generic cross-build problem in the xfsprogs code,
> > > > > not slap a distro-specific build band-aid over it.
> > > > 
> > > > I fully agree with this in principle. However, when I fail to find that
> > > > upstreamable solution, I try to at least provide a Debian-specific
> > > > solution to iterate from.
> > > > 
> > > > Can you propose a way to drop either #include?
> > > 
> > > Frankly I don't really see the value in crc32selftest when cross
> > > compiling -- sure, the crc32c code works correctly on the build host,
> > > but that proves nothing about the (cross-)built binaries that end up in
> > > the package.
> > 
> > Yup, that's pretty much what I was getting at.
> > 
> > > The selftest code is modular enough that it's included in xfs_io and it
> > > seems to run in under 500us even on my ancient raspberry pi 3b+
> > > downclocked to 600mhz.  Why don't we just add it to mkfs and repair?
> > > Those tools shouldn't be run all that frequently, and now we'll know
> > > immediately if crc32c is broken on a user's system.
> > > 
> > > Then we don't need the build-time selftest at all.
> > 
> > Sounds reasonable, my only concern is what do users do when
> > xfs-repair fails the self test and they need to fix their
> > filesystem?
> 
> Find a system with a non-broken CPU, or complain to their support rep or
> the upstream mailing list.  I'd rather they do that than let repair
> erroneously obliterate all the metadata with incorrect crc validation,
> or write garbage out that fails to mount immediately after.

Ok, sounds like a man page update is in order, then :)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] debian: Generate .gitcensus instead of .census (Closes: #999743)
  2021-11-19 17:12 ` [PATCH 1/2] debian: Generate .gitcensus instead of .census (Closes: #999743) Bastian Germann
@ 2021-11-23 20:01   ` Bastian Germann
  2021-11-23 20:08   ` Darrick J. Wong
  1 sibling, 0 replies; 13+ messages in thread
From: Bastian Germann @ 2021-11-23 20:01 UTC (permalink / raw)
  To: linux-xfs

@Darrick: Thanks for caring about the 2nd patch. Is anyone up to review this patch?

Am 19.11.21 um 18:12 schrieb Bastian Germann:
> Fix the Debian build outside a git tree (e.g., Debian archive builds) by
> creating an empty .gitcensus instead of .census file on config.
> 
> Signed-off-by: Bastian Germann <bage@debian.org>
> ---
>   debian/rules | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/debian/rules b/debian/rules
> index 615289b4..6d5b82a8 100755
> --- a/debian/rules
> +++ b/debian/rules
> @@ -43,15 +43,15 @@ built: dibuild config
>   	$(MAKE) $(PMAKEFLAGS) default
>   	touch built
>   
> -config: .census
> -.census:
> +config: .gitcensus
> +.gitcensus:
>   	@echo "== dpkg-buildpackage: configure" 1>&2
>   	$(checkdir)
>   	AUTOHEADER=/bin/true dh_autoreconf
>   	dh_update_autotools_config
>   	$(options) $(MAKE) $(PMAKEFLAGS) include/platform_defs.h
>   	cp -f include/install-sh .
> -	touch .census
> +	touch .gitcensus
>   
>   dibuild:
>   	$(checkdir)
> @@ -72,7 +72,7 @@ dibuild:
>   clean:
>   	@echo "== dpkg-buildpackage: clean" 1>&2
>   	$(checkdir)
> -	-rm -f built .census mkfs/mkfs.xfs-$(bootpkg)
> +	-rm -f built .gitcensus mkfs/mkfs.xfs-$(bootpkg)
>   	$(MAKE) distclean
>   	-rm -rf $(dirme) $(dirdev) $(dirdi)
>   	-rm -f debian/*substvars debian/files* debian/*.debhelper
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] debian: Generate .gitcensus instead of .census (Closes: #999743)
  2021-11-19 17:12 ` [PATCH 1/2] debian: Generate .gitcensus instead of .census (Closes: #999743) Bastian Germann
  2021-11-23 20:01   ` Bastian Germann
@ 2021-11-23 20:08   ` Darrick J. Wong
  1 sibling, 0 replies; 13+ messages in thread
From: Darrick J. Wong @ 2021-11-23 20:08 UTC (permalink / raw)
  To: Bastian Germann; +Cc: linux-xfs

On Fri, Nov 19, 2021 at 06:12:40PM +0100, Bastian Germann wrote:
> Fix the Debian build outside a git tree (e.g., Debian archive builds) by
> creating an empty .gitcensus instead of .census file on config.
> 
> Signed-off-by: Bastian Germann <bage@debian.org>

Heh.  I used to have a patch that more or less did this, until I
switched to building development packages only from git.  Sorry I forgot
to come back to this one.

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  debian/rules | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/debian/rules b/debian/rules
> index 615289b4..6d5b82a8 100755
> --- a/debian/rules
> +++ b/debian/rules
> @@ -43,15 +43,15 @@ built: dibuild config
>  	$(MAKE) $(PMAKEFLAGS) default
>  	touch built
>  
> -config: .census
> -.census:
> +config: .gitcensus
> +.gitcensus:
>  	@echo "== dpkg-buildpackage: configure" 1>&2
>  	$(checkdir)
>  	AUTOHEADER=/bin/true dh_autoreconf
>  	dh_update_autotools_config
>  	$(options) $(MAKE) $(PMAKEFLAGS) include/platform_defs.h
>  	cp -f include/install-sh .
> -	touch .census
> +	touch .gitcensus
>  
>  dibuild:
>  	$(checkdir)
> @@ -72,7 +72,7 @@ dibuild:
>  clean:
>  	@echo "== dpkg-buildpackage: clean" 1>&2
>  	$(checkdir)
> -	-rm -f built .census mkfs/mkfs.xfs-$(bootpkg)
> +	-rm -f built .gitcensus mkfs/mkfs.xfs-$(bootpkg)
>  	$(MAKE) distclean
>  	-rm -rf $(dirme) $(dirdev) $(dirdi)
>  	-rm -f debian/*substvars debian/files* debian/*.debhelper
> -- 
> 2.30.2
> 
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-11-23 20:08 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-19 17:12 [PATCH 0/2] debian: Fix xfsprogs FT(C)BFS Bastian Germann
2021-11-19 17:12 ` [PATCH 1/2] debian: Generate .gitcensus instead of .census (Closes: #999743) Bastian Germann
2021-11-23 20:01   ` Bastian Germann
2021-11-23 20:08   ` Darrick J. Wong
2021-11-19 17:12 ` [PATCH 2/2] debian: Fix FTCBFS: Skip crc32 test (Closes: #999879) Bastian Germann
2021-11-19 23:11   ` Dave Chinner
2021-11-20  7:32     ` Helmut Grohne
2021-11-20 17:15       ` Darrick J. Wong
2021-11-20 22:20         ` Dave Chinner
2021-11-20 22:27           ` Dave Chinner
2021-11-22 23:32             ` Darrick J. Wong
2021-11-22 23:31           ` Darrick J. Wong
2021-11-22 23:37             ` Dave Chinner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.