After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 722887 - Makefile.introspection: pass *FLAGS to scanner
Makefile.introspection: pass *FLAGS to scanner
Status: RESOLVED FIXED
Product: gobject-introspection
Classification: Platform
Component: build
unspecified
Other All
: Normal normal
: ---
Assigned To: gobject-introspection Maintainer(s)
gobject-introspection Maintainer(s)
Depends on:
Blocks: 722480
 
 
Reported: 2014-01-24 04:38 UTC by Allison Karlitskaya (desrt)
Modified: 2015-03-30 21:18 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Makefile.introspection: pass *FLAGS to scanner (2.09 KB, patch)
2014-01-24 04:38 UTC, Allison Karlitskaya (desrt)
reviewed Details | Review
Makefile.introspection: pass *FLAGS to scanner (2.09 KB, patch)
2015-03-30 21:18 UTC, Dieter Verfaillie
none Details | Review

Description Allison Karlitskaya (desrt) 2014-01-24 04:38:03 UTC
automake provides a few mechanisms for setting the various user-defined
variables CFLAGS, CPPFLAGS, LDFLAGS that g-ir-scanner passes through to
the C compiler.

 - can be set in the environment

 - can be given as a commandline option to configure:

     ./configure LDFLAGS=-L/usr/local/lib

 - can be given as a commandline option to make:

     make LDFLAGS=-L/usr/local/lib

Currently, g-ir-scanner only checks for the variables in the
environment, whereas the correct value to use will always be the value
that is available from the Makefile, eg: $(LDFLAGS).

We could pass these variables as new arguments to the scanner, but the
easiest thing to do is promote them to the environment where the scanner
can see them.
Comment 1 Allison Karlitskaya (desrt) 2014-01-24 04:38:05 UTC
Created attachment 267099 [details] [review]
Makefile.introspection: pass *FLAGS to scanner
Comment 2 Colin Walters 2014-02-27 13:40:05 UTC
Review of attachment 267099 [details] [review]:

Sorry for not reviewing this earlier.  

What was the actual issue this patch was fixing?  Can you at least briefly mention the real-world use case?  Is this something like FreeBSD's ports system setting CPPFLAGS=-I/usr/local/include ?

Could this result in duplicated CFLAGS for projects which are already working around this?  Hopefully that's not an issue.

Do you think it's OK to try to land this patch this late (again sorry for not reviewing earlier), or can it wait for the start of next cycle?
Comment 3 Allison Karlitskaya (desrt) 2014-02-27 13:56:15 UTC
This originated from FreeBSD building on account of bug 722480.

The summary there is that gegl was overriding the environment of the scanner like so:

  INTROSPECTION_SCANNER_ENV = CFLAGS= LDFLAGS= CXXFLAGS=

because the maintainer didn't like the inconsistent treatment of the variables (CFLAGS, etc) depending on if they had come from configure, from arguments to make, or from having been in the environment.

He's right that there is a problem there.  CFLAGS was getting used by the scanner only if it was set in the environment -- not by the other routes.

This bug is an attempt to fix that properly upstream, so that he doesn't need to use the hack in his makefile anymore.  The difference is that we do it in the other direction: whereas he achieved consistency by ensuring that CFLAGS was always unset, this patch achieves it by ensuring that it is always correctly set.

(The approach of) the proposed change has been getting some testing in gegl:

https://git.gnome.org/browse/gegl/commit/?id=449c994af3f74ea0d605fda654a53e239eb65ebd

but as you note, if we apply this now, we'll end up with the arguments twice in that case.  I'm sure we'd get a quick response from gegl to correct that, though, and I don't know of other projects that are doing this.

This is not blocking FreeBSD (and never was) so it's not urgent that it goes in, but it would be nice to have it so that the hack can be removed from gegl.
Comment 4 André Klapper 2015-02-07 17:15:34 UTC
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]
Comment 5 Dieter Verfaillie 2015-03-24 21:13:31 UTC
Now that g-i 1.44 is out the door, can we get this in?
Would this approach also work for bug # 700066 (as far
as I can see, the same concern but for the CC env var)
Comment 6 Colin Walters 2015-03-24 22:14:03 UTC
(In reply to Dieter Verfaillie from comment #5)
> Now that g-i 1.44 is out the door, can we get this in?
> Would this approach also work for bug # 700066 (as far
> as I can see, the same concern but for the CC env var)

Yep, sounds fine to commit to me.
Comment 7 Dieter Verfaillie 2015-03-30 21:17:59 UTC
The following fix has been pushed:
92d9c38 Makefile.introspection: pass *FLAGS to scanner
Comment 8 Dieter Verfaillie 2015-03-30 21:18:05 UTC
Created attachment 300626 [details] [review]
Makefile.introspection: pass *FLAGS to scanner

automake provides a few mechanisms for setting the various user-defined
variables CFLAGS, CPPFLAGS, LDFLAGS that g-ir-scanner passes through to
the C compiler.

 - can be set in the environment

 - can be given as a commandline option to configure:

     ./configure LDFLAGS=-L/usr/local/lib

 - can be given as a commandline option to make:

     make LDFLAGS=-L/usr/local/lib

Currently, g-ir-scanner only checks for the variables in the
environment, whereas the correct value to use will always be the value
that is available from the Makefile, eg: $(LDFLAGS).

We could pass these variables as new arguments to the scanner, but the
easiest thing to do is promote them to the environment where the scanner
can see them.