ccan.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Kevin Locke <kevin@kevinlocke.name>
Cc: ccan@lists.ozlabs.org
Subject: Re: [PATCH 8/9] configurator: Pass output cflag to configurator
Date: Tue, 20 Sep 2016 23:32:41 +1000	[thread overview]
Message-ID: <20160920133241.GJ20488@umbus> (raw)
In-Reply-To: <5e147bac-4111-060b-1fbe-f652bbfd7dc0@kevinlocke.name>


[-- Attachment #1.1: Type: text/plain, Size: 3064 bytes --]

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan

  reply	other threads:[~2016-09-20 14:21 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-19  0:51 [PATCH 0/9] configurator: Support for Windows and MSVC Kevin Locke
2016-09-19  0:51 ` [PATCH 1/9] configurator: Replace unlink with remove Kevin Locke
2016-09-20  4:50   ` David Gibson
2016-09-19  0:51 ` [PATCH 2/9] configurator: Reimplement run using popen Kevin Locke
2016-09-20  5:00   ` David Gibson
2016-09-20  6:22     ` Kevin Locke
2016-09-20 13:28       ` David Gibson
2016-09-19  0:52 ` [PATCH 3/9] configurator: Inline err.h functions from musl libc Kevin Locke
2016-09-20  5:03   ` David Gibson
2016-09-27  6:11     ` Rusty Russell
2016-09-19  0:52 ` [PATCH 4/9] configurator: Use native directory separator Kevin Locke
2016-09-20  5:04   ` David Gibson
2016-09-19  0:52 ` [PATCH 5/9] configurator: Mark non-Windows tests MAY_NOT_COMPILE Kevin Locke
2016-09-20  5:06   ` David Gibson
2016-09-19  0:52 ` [PATCH 6/9] configurator: Print test source without cat Kevin Locke
2016-09-20  5:09   ` David Gibson
2016-09-19  0:52 ` [PATCH 7/9] configurator: Fix compiler warning with compare Kevin Locke
2016-09-20  5:09   ` David Gibson
2016-09-19  0:52 ` [PATCH 8/9] configurator: Pass output cflag to configurator Kevin Locke
2016-09-20  5:23   ` David Gibson
2016-09-20  6:22     ` Kevin Locke
2016-09-20 13:32       ` David Gibson [this message]
2016-09-19  0:52 ` [PATCH 9/9] configurator: DEFAULT_{COMPILER, FLAGS} for MSVC Kevin Locke
2016-09-20  4:48 ` [PATCH 0/9] configurator: Support for Windows and MSVC David Gibson
2016-09-20  6:21   ` Kevin Locke
2016-09-20 12:25     ` David Gibson
2016-09-20 14:16       ` Daniel Burke
2016-09-22  2:00         ` David Gibson
2016-09-23  3:33 ` [PATCH v2 00/13] " Kevin Locke
2016-09-23  3:33   ` [PATCH v2 01/13] configurator: Replace unlink with remove Kevin Locke
2016-09-27  4:50     ` David Gibson
2016-09-23  3:33   ` [PATCH v2 02/13] configurator: Reimplement run using popen Kevin Locke
2016-09-27  4:52     ` David Gibson
2016-09-28  5:29       ` [PATCH v3 2/13] " Kevin Locke
2016-09-23  3:33   ` [PATCH v2 03/13] configurator: Inline err.h functions from musl libc Kevin Locke
2016-09-27  5:01     ` David Gibson
2016-09-28  5:35       ` [PATCH v3 3/13] configurator: Inline err.h functions Kevin Locke
2016-09-23  3:33   ` [PATCH v2 04/13] configurator: Use native directory separator Kevin Locke
2016-09-27  5:05     ` David Gibson
2016-09-23  3:33   ` [PATCH v2 05/13] configurator: Mark non-Windows tests MAY_NOT_COMPILE Kevin Locke
2016-09-27  5:05     ` David Gibson
2016-09-23  3:33   ` [PATCH v2 06/13] configurator: Print test source without cat Kevin Locke
2016-09-27  5:06     ` David Gibson
2016-09-28  5:38       ` Kevin Locke
2016-09-23  3:33   ` [PATCH v2 07/13] configurator: Fix compiler warning with compare Kevin Locke
2016-09-27  5:09     ` David Gibson
2016-09-23  3:33   ` [PATCH v2 08/13] configurator: Add output cflag option and macro Kevin Locke
2016-09-27  5:17     ` David Gibson
2016-09-23  3:33   ` [PATCH v2 09/13] configurator: DEFAULT_{COMPILER, FLAGS} for MSVC Kevin Locke
2016-09-27  5:17     ` David Gibson
2016-09-23  3:33   ` [PATCH v2 10/13] configurator: Fix warning in HAVE_FOR_LOOP_DECLARATION Kevin Locke
2016-09-27  5:10     ` David Gibson
2016-09-23  3:33   ` [PATCH v2 11/13] configurator: Return pointer difference as ptrdiff_t Kevin Locke
2016-09-27  5:10     ` David Gibson
2016-09-23  3:33   ` [PATCH v2 12/13] Makefile: Define CFLAGS_FORCE_C_SOURCE macro Kevin Locke
2016-09-27  5:18     ` David Gibson
2016-09-27  6:18       ` Rusty Russell
2016-09-28  2:01         ` David Gibson
2016-10-26  2:49           ` Rusty Russell
2016-09-23  3:33   ` [PATCH v2 13/13] Add appveyor.yml Kevin Locke
2016-09-27  5:20     ` David Gibson
2016-09-28  6:01       ` [PATCH v3 " Kevin Locke
2016-09-27  5:23   ` [PATCH v2 00/13] configurator: Support for Windows and MSVC David Gibson
2016-09-27  6:20     ` Rusty Russell
2016-09-28  6:32       ` Kevin Locke
2016-09-28  6:28     ` Kevin Locke
2016-09-29  0:21       ` David Gibson
2016-09-29  0:44         ` [PATCH v3 0/7] " Kevin Locke
2016-09-29  0:44           ` [PATCH v3 1/7] configurator: Reimplement run using popen Kevin Locke
2016-09-29  0:44           ` [PATCH v3 2/7] configurator: Inline err.h functions Kevin Locke
2016-09-29  0:44           ` [PATCH v3 3/7] configurator: Print test source without cat Kevin Locke
2016-09-29  0:44           ` [PATCH v3 4/7] configurator: Add output cflag option and macro Kevin Locke
2016-09-29  0:44           ` [PATCH v3 5/7] configurator: DEFAULT_{COMPILER, FLAGS} for MSVC Kevin Locke
2016-09-29  0:44           ` [PATCH v3 6/7] Makefile: Define CFLAGS_FORCE_C_SOURCE macro Kevin Locke
2016-09-29  0:44           ` [PATCH v3 7/7] Add appveyor.yml Kevin Locke
2016-09-30 15:08           ` [PATCH v3 0/7] configurator: Support for Windows and MSVC David Gibson
2016-10-03  3:14             ` Kevin Locke
2016-12-02 20:23               ` AppVeyor custom Git URL support (was Re: [PATCH v3 0/7] configurator: Support for Windows and MSVC) Kevin Locke

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160920133241.GJ20488@umbus \
    --to=david@gibson.dropbear.id.au \
    --cc=ccan@lists.ozlabs.org \
    --cc=kevin@kevinlocke.name \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).