| Message ID | 20180906183529.117251-1-briannorris@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [xfsprogs] build: don't assume $BUILD_CC has fsmap.h just because $CC does | expand |
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
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
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
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
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
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
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@
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(-)