Message ID | 8f01ac45-1b6e-be1d-b6e3-48fe6cdc4833@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 2017年08月16日 02:11, Eric Sandeen wrote: > The mktables binary needs to be build with the host > compiler at built time, not the target compiler, because > it runs at build time to generate the raid tables. > > Copy auto-fu from xfsprogs and modify Makefile to > accomodate this. > > Reported-by: Hallo32 <Hallo32@gmx.net> > Signed-off-by: Eric Sandeen <sandeen@redhat.com> Looks better than my previous patch. With @BUILD_CLFAGS support and better BUILD_CC/CLFAGS detection for native build environment. Reviewed-by: Qu Wenruo <quwenruo.btrfs@gmx.com> Thanks, Qu > --- > > diff --git a/Makefile b/Makefile > index b3e2b63..2647b95 100644 > --- a/Makefile > +++ b/Makefile > @@ -323,7 +323,7 @@ version.h: version.sh version.h.in configure.ac > > mktables: kernel-lib/mktables.c > @echo " [CC] $@" > - $(Q)$(CC) $(CFLAGS) $< -o $@ > + $(Q)$(BUILD_CC) $(BUILD_CFLAGS) $< -o $@ > > kernel-lib/tables.c: mktables > @echo " [TABLE] $@" > diff --git a/Makefile.inc.in b/Makefile.inc.in > index 4e1b68c..0570bf8 100644 > --- a/Makefile.inc.in > +++ b/Makefile.inc.in > @@ -4,6 +4,8 @@ > export > > CC = @CC@ > +BUILD_CC = @BUILD_CC@ > +BUILD_CFLAGS = @BUILD_CFLAGS@ > LN_S = @LN_S@ > AR = @AR@ > RM = @RM@ > diff --git a/configure.ac b/configure.ac > index 30055f8..bc590cc 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -26,6 +26,23 @@ AC_CONFIG_SRCDIR([btrfs.c]) > AC_PREFIX_DEFAULT([/usr/local]) > > AC_PROG_CC > +AC_ARG_VAR(BUILD_CC, [C compiler for build tools]) > +if test "${BUILD_CC+set}" != "set"; then > + if test $cross_compiling = no; then > + BUILD_CC="$CC" > + else > + AC_CHECK_PROGS(BUILD_CC, gcc cc) > + fi > +fi > +AC_ARG_VAR(BUILD_CFLAGS, [C compiler flags for build tools]) > +if test "${BUILD_CFLAGS+set}" != "set"; then > + if test $cross_compiling = no; then > + BUILD_CFLAGS="$CFLAGS" > + else > + BUILD_CFLAGS="-g -O2" > + fi > +fi > + > AC_CANONICAL_HOST > AC_C_CONST > AC_C_VOLATILE > > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 8/15/17 7:17 PM, Qu Wenruo wrote: > > > On 2017年08月16日 02:11, Eric Sandeen wrote: >> The mktables binary needs to be build with the host >> compiler at built time, not the target compiler, because >> it runs at build time to generate the raid tables. >> >> Copy auto-fu from xfsprogs and modify Makefile to >> accomodate this. >> >> Reported-by: Hallo32 <Hallo32@gmx.net> >> Signed-off-by: Eric Sandeen <sandeen@redhat.com> > > Looks better than my previous patch. > With @BUILD_CLFAGS support and better BUILD_CC/CLFAGS detection for native build environment. > > Reviewed-by: Qu Wenruo <quwenruo.btrfs@gmx.com> Thanks - and sorry for missing your earlier patch, I didn't mean to ignore it. :) I just missed it. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Aug 16, 2017 at 08:17:00AM +0800, Qu Wenruo wrote: > > > On 2017年08月16日 02:11, Eric Sandeen wrote: > > The mktables binary needs to be build with the host > > compiler at built time, not the target compiler, because > > it runs at build time to generate the raid tables. > > > > Copy auto-fu from xfsprogs and modify Makefile to > > accomodate this. > > > > Reported-by: Hallo32 <Hallo32@gmx.net> > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> > > Looks better than my previous patch. > With @BUILD_CLFAGS support and better BUILD_CC/CLFAGS detection for > native build environment. The contents of tables.c has practially not changed since its introduction in 2000s. I see no sense in regenerating it each time during build and I don't want to introduce host CC build just for this one file, standard practice or not. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2017年08月25日 01:01, David Sterba wrote: > On Wed, Aug 16, 2017 at 08:17:00AM +0800, Qu Wenruo wrote: >> >> >> On 2017年08月16日 02:11, Eric Sandeen wrote: >>> The mktables binary needs to be build with the host >>> compiler at built time, not the target compiler, because >>> it runs at build time to generate the raid tables. >>> >>> Copy auto-fu from xfsprogs and modify Makefile to >>> accomodate this. >>> >>> Reported-by: Hallo32 <Hallo32@gmx.net> >>> Signed-off-by: Eric Sandeen <sandeen@redhat.com> >> >> Looks better than my previous patch. >> With @BUILD_CLFAGS support and better BUILD_CC/CLFAGS detection for >> native build environment. > > The contents of tables.c has practially not changed since its > introduction in 2000s. I see no sense in regenerating it each time > during build and I don't want to introduce host CC build just for > this one file, standard practice or not. > I understand the worry about adding host CC. But just shown by the patch, the impact is minimal. At least for me and already mentioned before, text file managed by git should be reviewable. No matter if the file get modified in 20 years or not. (BTW, I don't think there is much problem regenerating the RAID6 table. It's way faster than auto-conf/auto-header things) And I think that's the same reason other projects doesn't manage their generated CRC tables using git. Thanks, Qu -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 8/24/17 12:01 PM, David Sterba wrote: > On Wed, Aug 16, 2017 at 08:17:00AM +0800, Qu Wenruo wrote: >> >> >> On 2017年08月16日 02:11, Eric Sandeen wrote: >>> The mktables binary needs to be build with the host >>> compiler at built time, not the target compiler, because >>> it runs at build time to generate the raid tables. >>> >>> Copy auto-fu from xfsprogs and modify Makefile to >>> accomodate this. >>> >>> Reported-by: Hallo32 <Hallo32@gmx.net> >>> Signed-off-by: Eric Sandeen <sandeen@redhat.com> >> >> Looks better than my previous patch. >> With @BUILD_CLFAGS support and better BUILD_CC/CLFAGS detection for >> native build environment. > > The contents of tables.c has practially not changed since its > introduction in 2000s. I see no sense in regenerating it each time > during build Make does know enough to generate it only as needed, of course. > and I don't want to introduce host CC build just for > this one file, standard practice or not. *shrug* it seems like the obviously correct fix to me, but I won't fight over it. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/24/2017 07:01 PM, David Sterba wrote: > On Wed, Aug 16, 2017 at 08:17:00AM +0800, Qu Wenruo wrote: >> >> >> On 2017年08月16日 02:11, Eric Sandeen wrote: >>> The mktables binary needs to be build with the host >>> compiler at built time, not the target compiler, because >>> it runs at build time to generate the raid tables. >>> >>> Copy auto-fu from xfsprogs and modify Makefile to >>> accomodate this. >>> >>> Reported-by: Hallo32 <Hallo32@gmx.net> >>> Signed-off-by: Eric Sandeen <sandeen@redhat.com> >> >> Looks better than my previous patch. >> With @BUILD_CLFAGS support and better BUILD_CC/CLFAGS detection for >> native build environment. > > The contents of tables.c has practially not changed since its > introduction in 2000s. I see no sense in regenerating it each time > during build and I don't want to introduce host CC build just for > this one file, standard practice or not. I think that is a problem to have both the table generated and the mktable.c generator int git. Only one it is sufficient. Having both generate only confusion. As my first option, I am to have only mktable in the git with BUILD_CC/CFLAGS. As second choice, we could remove mktable and put a comment that the tables are the kernel ones. BTW, I suggest to put a comment in table.c to change mktable in case of update of the table. Otherwise is not obvious. BR G.Baroncelli
diff --git a/Makefile b/Makefile index b3e2b63..2647b95 100644 --- a/Makefile +++ b/Makefile @@ -323,7 +323,7 @@ version.h: version.sh version.h.in configure.ac mktables: kernel-lib/mktables.c @echo " [CC] $@" - $(Q)$(CC) $(CFLAGS) $< -o $@ + $(Q)$(BUILD_CC) $(BUILD_CFLAGS) $< -o $@ kernel-lib/tables.c: mktables @echo " [TABLE] $@" diff --git a/Makefile.inc.in b/Makefile.inc.in index 4e1b68c..0570bf8 100644 --- a/Makefile.inc.in +++ b/Makefile.inc.in @@ -4,6 +4,8 @@ export CC = @CC@ +BUILD_CC = @BUILD_CC@ +BUILD_CFLAGS = @BUILD_CFLAGS@ LN_S = @LN_S@ AR = @AR@ RM = @RM@ diff --git a/configure.ac b/configure.ac index 30055f8..bc590cc 100644 --- a/configure.ac +++ b/configure.ac @@ -26,6 +26,23 @@ AC_CONFIG_SRCDIR([btrfs.c]) AC_PREFIX_DEFAULT([/usr/local]) AC_PROG_CC +AC_ARG_VAR(BUILD_CC, [C compiler for build tools]) +if test "${BUILD_CC+set}" != "set"; then + if test $cross_compiling = no; then + BUILD_CC="$CC" + else + AC_CHECK_PROGS(BUILD_CC, gcc cc) + fi +fi +AC_ARG_VAR(BUILD_CFLAGS, [C compiler flags for build tools]) +if test "${BUILD_CFLAGS+set}" != "set"; then + if test $cross_compiling = no; then + BUILD_CFLAGS="$CFLAGS" + else + BUILD_CFLAGS="-g -O2" + fi +fi + AC_CANONICAL_HOST AC_C_CONST AC_C_VOLATILE
The mktables binary needs to be build with the host compiler at built time, not the target compiler, because it runs at build time to generate the raid tables. Copy auto-fu from xfsprogs and modify Makefile to accomodate this. Reported-by: Hallo32 <Hallo32@gmx.net> Signed-off-by: Eric Sandeen <sandeen@redhat.com> --- -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html