GNOME Bugzilla – Bug 695182
gi-r-scanner: add support for raw CFLAGS flags option
Last modified: 2015-02-07 16:56:44 UTC
91c5274 gi-r-scanner: add support for raw CFLAGS flags option
Created attachment 238086 [details] [review] gi-r-scanner: add support for raw CFLAGS flags option gi-r-scanner chokes when gir_CFLAGS have an '-include <header>' since this is not a recognised option. This commit adds a new --cflags option that passes cflags directly to the spawned gcc.
*** Bug 693928 has been marked as a duplicate of this bug. ***
Review of attachment 238086 [details] [review]: The other option is something like: --begin-cflags $(pkg_FLAGS) --end-cflags That's probably more difficult to implement though. ::: giscanner/scannermain.py @@ +47,3 @@ + group.add_option("--cflags", help="Pre-processor cflags", + action="append", dest="cpp_cflags", + default=[]) So either this should be action="store" default="", or: ::: giscanner/sourcescanner.py @@ +226,3 @@ + def set_cpp_options(self, cflags, includes, defines, undefines): + if cflags: + self._cpp_options = cflags[0].split() Or this should be: self._cpp_options = [] for flags in cflags: self._cpp_options.extend(flags.split())
Created attachment 238162 [details] [review] It seems unlikely that there would be a need more than one --cflags argument, so went with the "store" option. gi-r-scanner: add support for raw CFLAGS flags option gi-r-scanner chokes when gir_CFLAGS have an '-include <header>' since this is not a recognised option. This commit adds a new --cflags option that passes cflags directly to the spawned gcc.
Review of attachment 238162 [details] [review]: Looks right. (We should probably do the same for the linker flags, but that can be another patch)
Breaks when the CFLAGS have shell-quoted arguments, like gtk2 has: -DGTK_PRINT_PREVIEW_COMMAND=\""evince --unlink-tempfile --preview --print-settings %s %f"\"
Created attachment 238179 [details] [review] gi-r-scanner: add support for raw CFLAGS flags option gi-r-scanner chokes when gir_CFLAGS have an '-include <header>' since this is not a recognised option. This commit adds a new --cflags option that passes cflags directly to the spawned gcc. This appears to work and passes the newly added enclosed quote test. I don't know if its a bit too much of a hack though?
Created attachment 238214 [details] [review] 0001-tests-Remove-duplicate-specification-of-CPPFLAGS-for.patch
Created attachment 238215 [details] [review] 0002-tests-Pass-a-include-option-for-Regress.patch
Created attachment 238216 [details] [review] 0003-scanner-Allow-CFLAGS-to-contain-arbitrary-preprocess.patch Ok, this patch series does what I was suggesting with optparse's option callbacks, and I think it's a lot cleaner than attempting to quote and parse shell script syntax.
Review of attachment 238214 [details] [review]: OK.
Review of attachment 238215 [details] [review]: Should we also use single quotes as well to test all combinations?
Created attachment 238243 [details] [review] gi-r-scanner: add support for raw CFLAGS flags option gi-r-scanner chokes when gir_CFLAGS have an '-include <header>' since this is not a recognised option. This commit adds a new --cflags option that passes cflags directly to the spawned gcc. This one works for me, and I like it better than the --begin-blah stuff.
Review of attachment 238243 [details] [review]: ::: Makefile.introspection @@ +143,3 @@ $(_gir_libraries) \ $($(_gir_name)_SCANNERFLAGS) \ + --cflags='$($(_gir_name)_CFLAGS)' \ This will fail if the CFLAGS contains single quotes.
Pushed after IRC discussion.
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]