Re: [ccan] [PATCH 8/9] configurator: Pass output cflag to configurator
On Tue, Sep 20, 2016 at 12:22:47AM -0600, Kevin Locke wrote: > On 09/19/2016 11:23 PM, David Gibson wrote: > > On Sun, Sep 18, 2016 at 06:52:05PM -0600, Kevin Locke wrote: > > > Unfortunately, not all compilers support -o as a command-line option for > > > specifying the output file. Visual Studio cl.exe issues warning D9035 > > > when -o is given, which is detected as a compile warning by the > > > configurator. > > > > > > To support such compilers, pass the output flag as the last flag to the > > > configurator. > > > > > > This is a breaking change. Existing scripts which pass flags to the > > > compiler must be modified to add "-o" as the last flag. As noted in the > > > cover letter for this patch series, I'm open to considering alternatives > > > if this is unacceptable. > > > > So, as it stands, this change completely breaks ccanlint on POSIX, > > which is not ok. Specifically it looks like the problem is that the > > DEFAULT_FLAGS from the configurator make it into CCAN_CFLAGS in > > config.h. ccanlint then tries to add further options and its own -o > > to the end of that, which isn't going to work. > > > > In short, I think the problem is that ccanlint needs fixes to work > > with MSVC (and possibly anything much else apart from gcc). I think > > we need to look at how to fix that and the configurator together with > > each other. > > > > Not having working ccanlint on Windows seems like a pretty big, since > > its the main tool for testing ccan, amongst other things. > > Good catch. I hadn't noticed the ccanlint breakage. That was a big > oversight on my part. > > The breakage likely extends to any other programs which use CCAN_CFLAGS, > since none are expecting the output flag at the end. Perhaps a better > solution would be to emit the output flag as a separate macro. Something > like CCAN_OUTPUT_CFLAG. What do you think? I think that's a better option, and maybe the way to go in the short term. Longer term, I'm wondering if we want some sort of mini-library for shared use by the ccan tools - configurator, ccanlint, etc. Amongst other things that could have an "invoke compiler" function which abstracts this sort of thing in a more general way. Of course, this borders on the questions about use of err() etc. in the configurator - within the tools themselves, what can we and can't we assume about the system. > The command line API for configurator could either expect the output flag to > be the last argument as currently in the patch (which is a breaking change > for anything which call it). Or a command line option such as > --output-cflag could be added to specify the flag and preserve command-line > syntax compatibility. > > Thoughts? --output-cflag is kind of clunky, but again might be our best bet in the short term. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature ___ ccan mailing list ccan@lists.ozlabs.org https://lists.ozlabs.org/listinfo/ccan
Re: [ccan] [PATCH 8/9] configurator: Pass output cflag to configurator
On 09/19/2016 11:23 PM, David Gibson wrote: On Sun, Sep 18, 2016 at 06:52:05PM -0600, Kevin Locke wrote: Unfortunately, not all compilers support -o as a command-line option for specifying the output file. Visual Studio cl.exe issues warning D9035 when -o is given, which is detected as a compile warning by the configurator. To support such compilers, pass the output flag as the last flag to the configurator. This is a breaking change. Existing scripts which pass flags to the compiler must be modified to add "-o" as the last flag. As noted in the cover letter for this patch series, I'm open to considering alternatives if this is unacceptable. So, as it stands, this change completely breaks ccanlint on POSIX, which is not ok. Specifically it looks like the problem is that the DEFAULT_FLAGS from the configurator make it into CCAN_CFLAGS in config.h. ccanlint then tries to add further options and its own -o to the end of that, which isn't going to work. In short, I think the problem is that ccanlint needs fixes to work with MSVC (and possibly anything much else apart from gcc). I think we need to look at how to fix that and the configurator together with each other. Not having working ccanlint on Windows seems like a pretty big, since its the main tool for testing ccan, amongst other things. Good catch. I hadn't noticed the ccanlint breakage. That was a big oversight on my part. The breakage likely extends to any other programs which use CCAN_CFLAGS, since none are expecting the output flag at the end. Perhaps a better solution would be to emit the output flag as a separate macro. Something like CCAN_OUTPUT_CFLAG. What do you think? The command line API for configurator could either expect the output flag to be the last argument as currently in the patch (which is a breaking change for anything which call it). Or a command line option such as --output-cflag could be added to specify the flag and preserve command-line syntax compatibility. Thoughts? Kevin ___ ccan mailing list ccan@lists.ozlabs.org https://lists.ozlabs.org/listinfo/ccan
Re: [ccan] [PATCH 8/9] configurator: Pass output cflag to configurator
On Sun, Sep 18, 2016 at 06:52:05PM -0600, Kevin Locke wrote:
> Unfortunately, not all compilers support -o as a command-line option for
> specifying the output file. Visual Studio cl.exe issues warning D9035
> when -o is given, which is detected as a compile warning by the
> configurator.
>
> To support such compilers, pass the output flag as the last flag to the
> configurator.
>
> This is a breaking change. Existing scripts which pass flags to the
> compiler must be modified to add "-o" as the last flag. As noted in the
> cover letter for this patch series, I'm open to considering alternatives
> if this is unacceptable.
>
> Signed-off-by: Kevin Locke
So, as it stands, this change completely breaks ccanlint on POSIX,
which is not ok. Specifically it looks like the problem is that the
DEFAULT_FLAGS from the configurator make it into CCAN_CFLAGS in
config.h. ccanlint then tries to add further options and its own -o
to the end of that, which isn't going to work.
In short, I think the problem is that ccanlint needs fixes to work
with MSVC (and possibly anything much else apart from gcc). I think
we need to look at how to fix that and the configurator together with
each other.
Not having working ccanlint on Windows seems like a pretty big, since
its the main tool for testing ccan, amongst other things.
> ---
> Makefile | 2 +-
> tools/configurator/configurator.c | 6 +++---
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 127b875..f36ccb5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -89,7 +89,7 @@ FORCE:
>
> # Ensure we don't end up with empty file if configurator fails!
> config.h: tools/configurator/configurator Makefile Makefile-ccan
> - tools/configurator/configurator $(CC) $(CCAN_CFLAGS) > $@.tmp && mv
> $@.tmp $@
> + tools/configurator/configurator $(CC) $(CCAN_CFLAGS) -o > $@.tmp && mv
> $@.tmp $@
>
> include tools/Makefile
> -include inter-depends
> diff --git a/tools/configurator/configurator.c
> b/tools/configurator/configurator.c
> index 0c30aff..820ccf7 100644
> --- a/tools/configurator/configurator.c
> +++ b/tools/configurator/configurator.c
> @@ -36,7 +36,7 @@
> #endif
>
> #define DEFAULT_COMPILER "cc"
> -#define DEFAULT_FLAGS "-g3 -ggdb -Wall -Wundef -Wmissing-prototypes
> -Wmissing-declarations -Wstrict-prototypes -Wold-style-definition"
> +#define DEFAULT_FLAGS "-g3 -ggdb -Wall -Wundef -Wmissing-prototypes
> -Wmissing-declarations -Wstrict-prototypes -Wold-style-definition -o"
>
> #define OUTPUT_FILE "configurator.out"
> #define INPUT_FILE "configuratortest.c"
> @@ -672,7 +672,7 @@ int main(int argc, const char *argv[])
> if (argc > 1) {
> if (strcmp(argv[1], "--help") == 0) {
> printf("Usage: configurator [-v] [
> ...]\n"
> -"will have \"-o
> \" appended\n"
> +"will have \"
> \" appended\n"
> "Default: %s %s\n",
> DEFAULT_COMPILER, DEFAULT_FLAGS);
> exit(0);
> @@ -691,7 +691,7 @@ int main(int argc, const char *argv[])
> if (argc == 1)
> argv = default_args;
>
> - cmd = connect_args(argv, " -o " OUTPUT_FILE " " INPUT_FILE);
> + cmd = connect_args(argv, OUTPUT_FILE " " INPUT_FILE);
> for (i = 0; i < sizeof(tests)/sizeof(tests[0]); i++)
> run_test(cmd, &tests[i]);
> free(cmd);
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
___
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan
