[PATCH xfsprogs] build: don't assume $BUILD_CC has fsmap.h just because $CC does

[PATCH xfsprogs] build: don't assume $BUILD_CC has fsmap.h just because $CC does

[Brian Norris]

The $BUILD_CC toolchain might have an older set of Linux headers than
the $CC toolchain. It's generally unsafe to try to build both with the
same definitions, but in particular, this one can cause compilation
failures in the local crc32selftest build:

 In file included from crc32.c:37:
 In file included from ../include/xfs.h:37:
 ../include/xfs/linux.h:226:11: fatal error: 'linux/fsmap.h' file not found
 # include <linux/fsmap.h>
           ^~~~~~~~~~~~~~~

It's safe to always assume that the headers don't have getfsmap, since
the alternative just includes our local definitions anyway.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
This isn't exactly a pretty solution, but it fixes cross-compilation
problems I'm seeing.

 include/builddefs.in | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/builddefs.in b/include/builddefs.in
index f7d39a4e4094..209abe1d84dd 100644
--- a/include/builddefs.in
+++ b/include/builddefs.in
@@ -156,8 +156,9 @@ endif
 ifeq ($(NEED_INTERNAL_FSXATTR),yes)
 PCFLAGS+= -DOVERRIDE_SYSTEM_FSXATTR
 endif
+# Don't assume $BUILD_CC has getfsmap just because $CC does.
 ifeq ($(HAVE_GETFSMAP),yes)
-PCFLAGS+= -DHAVE_GETFSMAP
+CFLAGS+= -DHAVE_GETFSMAP
 endif
 
 LIBICU_LIBS = @libicu_LIBS@
-- 
2.19.0.rc2.392.g5ba43deb5a-goog

Re: [PATCH xfsprogs] build: don't assume $BUILD_CC has fsmap.h just because $CC does

[Eric Sandeen]

On 9/6/18 1:35 PM, Brian Norris wrote:
> The $BUILD_CC toolchain might have an older set of Linux headers than
> the $CC toolchain. It's generally unsafe to try to build both with the
> same definitions, but in particular, this one can cause compilation
> failures in the local crc32selftest build:
> 
>  In file included from crc32.c:37:
>  In file included from ../include/xfs.h:37:
>  ../include/xfs/linux.h:226:11: fatal error: 'linux/fsmap.h' file not found
>  # include <linux/fsmap.h>
>            ^~~~~~~~~~~~~~~
> 
> It's safe to always assume that the headers don't have getfsmap, since
> the alternative just includes our local definitions anyway.
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>

Seems reasonable, thanks for bringing up the problem.

At first I was thinking that separating out crc32test.c from crc32.c
like the kernel did would help, but of course crc32.c still has to be
built.  ;)

I was trying to just get the selftest to build with a custom set of
userspace headers, like the patch below, so we don't have to pull in all
the various and sundry xfs headers & dependencies.  Not sure what the better
approach is here ...

---



diff --git a/libfrog/crc32.c b/libfrog/crc32.c
index 1d52f68..b5addad 100644
--- a/libfrog/crc32.c
+++ b/libfrog/crc32.c
@@ -29,16 +29,34 @@
  * match the hardware acceleration available on Intel CPUs.
  */
 
+#ifdef CRC32_SELFTEST
+#define __STDC_FORMAT_MACROS
+#include <inttypes.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <sys/time.h>
+#include <sys/types.h>
+#include <asm/types.h>
+#include "xfs_arch.h"
+#include "crc32c.h"
+
+/* types specifc to this file */
+typedef unsigned char  u8;
+typedef unsigned int   u32;
+#else
 #include "platform_defs.h"
 #include "xfs.h"
 #include "xfs_arch.h"
-#include "crc32defs.h"
 
 /* types specifc to this file */
 typedef __u8	u8;
-typedef __u16	u16;
 typedef __u32	u32;
-typedef __u32	u64;
+
+#endif /* CRC32_SELFTEST */
+
+#include "crc32defs.h"
+
 #define __pure
 
 #if CRC_LE_BITS > 8

Re: [PATCH xfsprogs] build: don't assume $BUILD_CC has fsmap.h just because $CC does

[Brian Norris]

Hi Eric,

On Thu, Sep 06, 2018 at 05:33:56PM -0500, Eric Sandeen wrote:
> I was trying to just get the selftest to build with a custom set of
> userspace headers, like the patch below, so we don't have to pull in all
> the various and sundry xfs headers & dependencies.  Not sure what the better
> approach is here ...

I think an ideal goal is still to remove all usage of HAVE_* from
anything that gets built with $BUILD_CC. I guess that's just
crc32selftest? So even if you do that, it would still be good to do
something like my patch. (Maybe even more than that -- remove the BLKID,
FSETXATTR, etc., flags from $BUILD_CFLAGS.)

But your patch does work for me. (Well, I had to port it back a little,
since my cross-compilation environment isn't building off the
development tip.)

Thanks,
Brian

Re: [PATCH xfsprogs] build: don't assume $BUILD_CC has fsmap.h just because $CC does

[Brian Norris]

On Thu, Sep 06, 2018 at 03:55:34PM -0700, Brian Norris wrote:
> On Thu, Sep 06, 2018 at 05:33:56PM -0500, Eric Sandeen wrote:
> > I was trying to just get the selftest to build with a custom set of
> > userspace headers, like the patch below, so we don't have to pull in all
> > the various and sundry xfs headers & dependencies.  Not sure what the better
> > approach is here ...
> 
> I think an ideal goal is still to remove all usage of HAVE_* from
> anything that gets built with $BUILD_CC. I guess that's just
> crc32selftest? So even if you do that, it would still be good to do
> something like my patch. (Maybe even more than that -- remove the BLKID,
> FSETXATTR, etc., flags from $BUILD_CFLAGS.)
> 
> But your patch does work for me. (Well, I had to port it back a little,
> since my cross-compilation environment isn't building off the
> development tip.)

Ping? Should I send a new version that combines our patches?

Regards,
Brian

Re: [PATCH xfsprogs] build: don't assume $BUILD_CC has fsmap.h just because $CC does

[Eric Sandeen]

On 9/21/18 1:37 PM, Brian Norris wrote:
> On Thu, Sep 06, 2018 at 03:55:34PM -0700, Brian Norris wrote:
>> On Thu, Sep 06, 2018 at 05:33:56PM -0500, Eric Sandeen wrote:
>>> I was trying to just get the selftest to build with a custom set of
>>> userspace headers, like the patch below, so we don't have to pull in all
>>> the various and sundry xfs headers & dependencies.  Not sure what the better
>>> approach is here ...
>>
>> I think an ideal goal is still to remove all usage of HAVE_* from
>> anything that gets built with $BUILD_CC. I guess that's just
>> crc32selftest? So even if you do that, it would still be good to do
>> something like my patch. (Maybe even more than that -- remove the BLKID,
>> FSETXATTR, etc., flags from $BUILD_CFLAGS.)
>>
>> But your patch does work for me. (Well, I had to port it back a little,
>> since my cross-compilation environment isn't building off the
>> development tip.)
> 
> Ping? Should I send a new version that combines our patches?
> 
> Regards,
> Brian
> 

Hi Brian - I'm not ignoring your patch, I've just been working on getting
libxfs synced for 4.19 before merging anything else - this is usually how
I work when patches come in near the beginning of a release cycle.
I'll revisit your patch & reach out to you if I have other questions or
suggestions when I get there, ok?  Sorry for the delay.

Thanks,
-Eric

Re: [PATCH xfsprogs] build: don't assume $BUILD_CC has fsmap.h just because $CC does

[Brian Norris]

On Fri, Sep 21, 2018 at 02:12:31PM -0500, Eric Sandeen wrote:
> On 9/21/18 1:37 PM, Brian Norris wrote:
> > Ping? Should I send a new version that combines our patches?
> 
> Hi Brian - I'm not ignoring your patch, I've just been working on getting
> libxfs synced for 4.19 before merging anything else - this is usually how
> I work when patches come in near the beginning of a release cycle.
> I'll revisit your patch & reach out to you if I have other questions or
> suggestions when I get there, ok?  Sorry for the delay.

No problem at all. I don't think I understood the development patterns
here anyway. It just wasn't clear if there was a next action for me.
This makes it clear.

Thanks!

Brian

Re: [PATCH xfsprogs] build: don't assume $BUILD_CC has fsmap.h just because $CC does

[Eric Sandeen]

On 9/21/18 2:34 PM, Brian Norris wrote:
> On Fri, Sep 21, 2018 at 02:12:31PM -0500, Eric Sandeen wrote:
>> On 9/21/18 1:37 PM, Brian Norris wrote:
>>> Ping? Should I send a new version that combines our patches?
>>
>> Hi Brian - I'm not ignoring your patch, I've just been working on getting
>> libxfs synced for 4.19 before merging anything else - this is usually how
>> I work when patches come in near the beginning of a release cycle.
>> I'll revisit your patch & reach out to you if I have other questions or
>> suggestions when I get there, ok?  Sorry for the delay.
> 
> No problem at all. I don't think I understood the development patterns
> here anyway. It just wasn't clear if there was a next action for me.
> This makes it clear.

Eh, it changes from time to time and isn't exactly advertised, no worries.

And this time I'm a bit behind so xfsprogs is as well.

Normal xfsprogs cycle lately is something like:

Release the last version to match the last kernel, shortly after last kernel release
Start merging in libxfs changes from next devel kernel to prepare for next release, cut -rc0
Pick up most of the accumulated pending patches, cut an -rc1
do the normal soak/patch collection until release, rc's as needed
rinse, repeat.

-Eric