From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 8D4F5386F83C; Thu, 30 Jul 2020 12:43:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 8D4F5386F83C Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark@simark.ca Received: from [10.0.0.11] (173-246-6-90.qc.cable.ebox.net [173.246.6.90]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 82B191E792; Thu, 30 Jul 2020 08:43:44 -0400 (EDT) Subject: Re: [PATCH] Unify Solaris procfs and largefile handling To: Rainer Orth Cc: binutils@sourceware.org, gdb-patches@sourceware.org References: <51f44a63-3062-39e5-14c5-ed08e32f2129@simark.ca> <5aae580e-ab2e-f008-91c6-f3b1c1f757b7@simark.ca> <169bf482-296b-737d-2a48-581d5b1d92bb@simark.ca> From: Simon Marchi Message-ID: <4d3bf991-c0e5-2136-5935-4c4e47aadbf2@simark.ca> Date: Thu, 30 Jul 2020 08:43:37 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: fr Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, NICE_REPLY_A, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 30 Jul 2020 12:43:49 -0000 On 2020-07-30 5:17 a.m., Rainer Orth wrote: > Hi Simon, > >> On 2020-07-29 7:19 a.m., Rainer Orth wrote: >>> It's even simpler: every configure script has code to parse >>> --enable-foo/--disable-foo and turn the result into enable_foo=[yes|no]. >> >> Ok, nice! >> >>> No: the code has been (and should remain) like this. It allows the user >>> to override the automatic largefile detection with explicit >>> --enable-largefile/--disable-largefile options without having to change >>> the code. >> >> Ack. >> >>> I've now removed AC_ARG_ENABLE from largefile.m4. Retested on >>> i386-pc-solaris2.11 without and with --disable-gdb, checking that >>> _FILE_OFFSET_BITS are set as expected, and amd64-pc-solaris2.11. >>> >>> Ok for master now? >> >> When I run `autoreconf -vf`, I get a lot of changes. Make sure to run >> it in any directory you touch that has a configure.ac and add the resulting >> changes. > > I didn't use autoreconf, but ran the appropriate > autoconf/autoheader/aclocal/automake dance manually. With one exception > (LARGEFILE_CPPFLAGS in gnulib Makefile.in) I'd gotten things right :-) > > I don't usually include generated files in patch submissions, though: > they are heavily frowned upon at least over in GCC because they make > review quite difficult, espcially in a case like this where the > largefile.m4 change spreads to lots of configure scripts, obscuring the > change proper. > > Does GDB handle things differently here? I don't think there's a hard rule. If it makes the patch too big for sending on the list, then it's fine for sure to not include them. If you don't want to include them, that's fine with my too, but in either case it's important to say that you've omitted them on purpose so we know it's not an oversight (otherwise I'll complain about them missing :)). Maybe it can be inconvenient for people who read the diff directly to review... I personally use meld to review patches, so it's simple to just skip over generated files. The patch LGTM, thanks. Simon