git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Primrose <jprimros@gmail.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org
Subject: Re: FREAD_READS_DIRECTORIES test fails for the wrong reasons
Date: Fri, 6 Apr 2018 17:57:15 -0400	[thread overview]
Message-ID: <8ed8491f-9410-c509-5ef7-77638aa74038@gmail.com> (raw)
In-Reply-To: <20180406193301.GA11450@sigill.intra.peff.net>

On 04/06/2018 03:33 PM, Jeff King wrote:
> On Fri, Apr 06, 2018 at 02:06:50PM -0400, Jonathan Primrose wrote:
>
>> A while ago, the configure test for FREAD_READS_DIRECTORIES was changed
>> to (attempt to) check for a NULL result from fopen. Unfortunately, the
>> test will always fail - because it won't compile. The code snippet in
>> configure.ac translates to:
>>
>> return f);
>>
>> Either there's an extra ) or a missing (. A cast to int wouldn't hurt
>> either.
>
> Oops. This is due to my 3adf9fdecf (configure.ac: loosen
> FREAD_READS_DIRECTORIES test program, 2017-06-14).
>
> Neither I nor the original tester noticed, because we're on Linux, which
> needs that set.

I'm also running Linux, but somehow I ended up needing to check
config.log and saw the error. I don't remember why I had to check.
(I've been fixing the problem locally for the last few releases,
figuring someone else would notice. I also remove the 'rm configure'
from the distclean target in Makefile, just in case I decide to
rebuild later.)
>
>> I'd supply a patch to fix this, but I'm not sure what the results of
>> suddenly fixing the test would be. It seems to work well on my
>> machine, but I don't stress git much here, and it's just one machine.
>
> I think it should be fixed (and agreed on the "int" thing; the return
> value should be "f != NULL" or similar).

Ironically, most of the time I check config.log is because of the
missing cast to int. A few years ago I set this machine up as a
tri-arch system (x86, x86_64, and x32). Because of subtle errors
that appear when converting between int and pointer on x32, I
tend to use -Werror=int-conversion on all my builds, and I wrap
configure in a script that (among other things) checks config.log
for that particular error. Even though x32 isn't as promising
anymore (thanks to a few projects rejecting support patches),
there are very few packages that still trigger the error.
>
> I don't know autoconf very well, but is there a way to invoke it that
> will let us know if a test-program fails to compile at all? Obviously
> for probing the system compler/includes, "does not compile" is an
> expected possible outcome. But for tests like this it's really a
> tristate: yes, no, or something went horribly wrong.

I don't know if that's supported. From what I've seen in the docs I've
found online, it looks like the closest is a case for "I couldn't run
the program because I'm cross-compiling." You might be able to
determine that the compile and/or link failed by looking at some of
autoconf's internal variables, but it would be fragile.
>
> -Peff
>

Jonathan

  reply	other threads:[~2018-04-06 21:57 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-06 18:06 FREAD_READS_DIRECTORIES test fails for the wrong reasons Jonathan Primrose
2018-04-06 19:33 ` Jeff King
2018-04-06 21:57   ` Jonathan Primrose [this message]
2018-04-06 22:07     ` Jeff King

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=8ed8491f-9410-c509-5ef7-77638aa74038@gmail.com \
    --to=jprimros@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /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).