From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-3.8 required=3.0 tests=AWL,BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_HI,T_RP_MATCHES_RCVD shortcircuit=no autolearn=ham autolearn_force=no version=3.4.0 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by dcvr.yhbt.net (Postfix) with ESMTP id 236CD1FA7B for ; Wed, 14 Jun 2017 05:15:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751065AbdFNFPr (ORCPT ); Wed, 14 Jun 2017 01:15:47 -0400 Received: from cloud.peff.net ([104.130.231.41]:39676 "EHLO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750721AbdFNFPr (ORCPT ); Wed, 14 Jun 2017 01:15:47 -0400 Received: (qmail 30376 invoked by uid 109); 14 Jun 2017 05:15:47 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.84) with SMTP; Wed, 14 Jun 2017 05:15:47 +0000 Received: (qmail 8108 invoked by uid 111); 14 Jun 2017 05:15:48 -0000 Received: from sigill.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.7) by peff.net (qpsmtpd/0.84) with SMTP; Wed, 14 Jun 2017 01:15:48 -0400 Received: by sigill.intra.peff.net (sSMTP sendmail emulation); Wed, 14 Jun 2017 01:15:44 -0400 Date: Wed, 14 Jun 2017 01:15:44 -0400 From: Jeff King To: =?utf-8?Q?=C3=98yvind_A=2E?= Holm , Jonathan Nieder , Git mailing list , =?utf-8?B?Tmd1eeG7hW4gVGjDoWkgTmfhu41j?= Duy , Junio C Hamano Subject: Re: t1308-config-set.sh fails on current master Message-ID: <20170614051544.cz2zvnkc4mlysz7h@sigill.intra.peff.net> References: <20170614011514.sh4euddp44hjbu4u@sunbase.org> <20170614012535.GU133952@aiede.mtv.corp.google.com> <20170614021739.erkdifufziwiqjxp@sunbase.org> <20170614050215.c32crnjifah6cxae@sigill.intra.peff.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20170614050215.c32crnjifah6cxae@sigill.intra.peff.net> Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On Wed, Jun 14, 2017 at 01:02:15AM -0400, Jeff King wrote: > On Wed, Jun 14, 2017 at 04:17:40AM +0200, Øyvind A. Holm wrote: > > > > Interesting. I'm not able to reproduce it, but of course that doesn't > > > mean much. > > > > I'll admit that I have a somewhat special build system, but it's been > > working great since I created it 7 months ago, and I run the test suite > > every time I install a new git. I'm using the Makefile located at > > > > https://gitlab.com/sunny256/src-other/blob/master/devel/git/Makefile > > > > It's only doing regular stuff like "make configure", "./configure", etc, > > but I'm mentioning it in case the Makefile reveals something > > interesting. The git installation is in a non-standard location, the > > newest version of git I've installed is for example located under > > /usr/src-other/pool/git.master.v2.13.1-394-g41dd4330a121/ . > > I couldn't reproduce either with my usual build, but I don't usually use > autoconf. Running: > > make configure > ./configure > make > (cd t && ./t1308-*) > > does fail for me. The problem is that the generated config.mak.autogen > sets the wrong value for FREAD_READS_DIRECTORIES (and overrides the > default entry for Linux from config.mak.uname. So the configure script > needs to be fixed. Actually, I'm not sure if configure.ac is wrong, or the new uses of FREAD_READS_DIRECTORIES. Because the test configure.ac actually checks: FILE *f = fopen(".", "r"); return f && fread(&c, 1, 1, f); I.e., it sees that not only do we fopen() a directory, but we actually read garbage from it. Whereas on Linux, we fopen the file and then the read gets EISDIR. So it's not true that FREAD_READS_DIRECTORIES; this is more like FOPEN_OPENS_DIRECTORIES. Just looking at how the macro is used, I think we want to handle both cases the same (by doing an fstat check after fopen). So I think it would be OK to continue to use FREAD_READS_DIRECTORIES for both cases, and just fix the configure script. It may be worth updating the macro name for clarity, though. -Peff