diff mbox

btrfs-progs: fix cross-compile build

Message ID 8f01ac45-1b6e-be1d-b6e3-48fe6cdc4833@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Eric Sandeen Aug. 15, 2017, 6:11 p.m. UTC
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

Comments

Qu Wenruo Aug. 16, 2017, 12:17 a.m. UTC | #1
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
Eric Sandeen Aug. 16, 2017, 9:38 p.m. UTC | #2
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
David Sterba Aug. 24, 2017, 5:01 p.m. UTC | #3
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
Qu Wenruo Aug. 25, 2017, 12:27 a.m. UTC | #4
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
Eric Sandeen Aug. 25, 2017, 1:06 a.m. UTC | #5
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
Goffredo Baroncelli Aug. 25, 2017, 2:03 p.m. UTC | #6
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 mbox

Patch

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