diff mbox

[XFSPROGS,2/3] Build without enabling DEBUG by default

Message ID 20110722223458.GD25092@thunk.org
State Not Applicable, archived
Headers show

Commit Message

Theodore Ts'o July 22, 2011, 10:34 p.m. UTC
On Fri, Jul 22, 2011 at 02:38:30PM -0400, Christoph Hellwig wrote:
> 
> You can get most of this by simply using make V=1, not just in xfsprogs,
> but in most projects using silent make output (including those using
> automake)

Yeah, I know about V=1 (and I have been using it), but I wanted to
know which of the many Makefile variables which finally make up
$(CFLAGS) was getting set to which value, and V=1 doesn't do this.

> for me.  The biggest thing is that you have another directory in your
> include path.  How did you add that one? Did you add anything else
> special to the ./configure or make command lines?

Yes I needed it because I wanted to do a hermetic build; that is, I
didn't want to depend on the version of the acl, attr, dmapi, etc.,
shipped by the distribution.  (This is actually more important when
building xfstests, since I wanted to make sure we used the version of
the xfsprogs headers shipped by the very tip of xfsprogs-dev, since in
the past I've had problems because the distro-shipped header files
were too old.)

The way I did that was by setting the environment CFLAGS to
-I/kbuild/xfstests/bld/include.  And that seems to be the problem.  If
CFLAGS is set as an environment variable, then it's set as a :=, which
means that CFLAGS is set right away, the first time buildmacros is
included.

How about this instead?  A patch like this so that CFLAGS can be
remembered at configure time:


Then I don't have to set an environment variable when running make; I
just have to pass it to configure.

							- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Christoph Hellwig July 23, 2011, 1:32 a.m. UTC | #1
On Fri, Jul 22, 2011 at 06:34:58PM -0400, Ted Ts'o wrote:
> Yes I needed it because I wanted to do a hermetic build; that is, I
> didn't want to depend on the version of the acl, attr, dmapi, etc.,
> shipped by the distribution.  (This is actually more important when
> building xfstests, since I wanted to make sure we used the version of
> the xfsprogs headers shipped by the very tip of xfsprogs-dev, since in
> the past I've had problems because the distro-shipped header files
> were too old.)

Given that you are on a Debian-ish environment, did you consider simply
using pbuilder?  I use that regularly to test clean package builds,
without all these weird overrides.

> The way I did that was by setting the environment CFLAGS to
> -I/kbuild/xfstests/bld/include.  And that seems to be the problem.  If
> CFLAGS is set as an environment variable, then it's set as a :=, which
> means that CFLAGS is set right away, the first time buildmacros is
> included.
> 
> How about this instead?  A patch like this so that CFLAGS can be
> remembered at configure time:
> 
> diff --git a/include/builddefs.in b/include/builddefs.in
> index 4c8ec64..81ebfcd 100644
> --- a/include/builddefs.in
> +++ b/include/builddefs.in
> @@ -25,6 +25,7 @@ OPTIMIZER = @opt_build@
>  MALLOCLIB = @malloc_lib@
>  LOADERFLAGS = @LDFLAGS@
>  LTLDFLAGS = @LDFLAGS@
> +CFLAGS = @CFLAGS@
>  
>  LIBRT = @librt@
>  LIBUUID = @libuuid@
> 
> Then I don't have to set an environment variable when running make; I
> just have to pass it to configure.

This looks much better, and is indeed similar to some other fixes we
recently had to put in, probably due to changing make behaviour.

If you add a signoff and a proper description I'll put it into all the
repositories.

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoph Hellwig July 26, 2011, 3:10 p.m. UTC | #2
On Fri, Jul 22, 2011 at 09:32:14PM -0400, Christoph Hellwig wrote:
> > diff --git a/include/builddefs.in b/include/builddefs.in
> > index 4c8ec64..81ebfcd 100644
> > --- a/include/builddefs.in
> > +++ b/include/builddefs.in
> > @@ -25,6 +25,7 @@ OPTIMIZER = @opt_build@
> >  MALLOCLIB = @malloc_lib@
> >  LOADERFLAGS = @LDFLAGS@
> >  LTLDFLAGS = @LDFLAGS@
> > +CFLAGS = @CFLAGS@
> >  
> >  LIBRT = @librt@
> >  LIBUUID = @libuuid@
> > 
> > Then I don't have to set an environment variable when running make; I
> > just have to pass it to configure.
> 
> This looks much better, and is indeed similar to some other fixes we
> recently had to put in, probably due to changing make behaviour.
> 
> If you add a signoff and a proper description I'll put it into all the
> repositories.

Can you at least provide the signoff?  I can take care of the rest if
this means I can get it in ASAP.

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoph Hellwig July 30, 2011, 4:55 p.m. UTC | #3
On Tue, Jul 26, 2011 at 11:10:33AM -0400, Christoph Hellwig wrote:
> > This looks much better, and is indeed similar to some other fixes we
> > recently had to put in, probably due to changing make behaviour.
> > 
> > If you add a signoff and a proper description I'll put it into all the
> > repositories.
> 
> Can you at least provide the signoff?  I can take care of the rest if
> this means I can get it in ASAP.

Ted,

can you please give me a signoff for this patch?  I could probably just
put it in anyway given how trivial it is.  But let's just stick to the
proper protocol if we can.

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/builddefs.in b/include/builddefs.in
index 4c8ec64..81ebfcd 100644
--- a/include/builddefs.in
+++ b/include/builddefs.in
@@ -25,6 +25,7 @@  OPTIMIZER = @opt_build@
 MALLOCLIB = @malloc_lib@
 LOADERFLAGS = @LDFLAGS@
 LTLDFLAGS = @LDFLAGS@
+CFLAGS = @CFLAGS@
 
 LIBRT = @librt@
 LIBUUID = @libuuid@