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 8C7223847725 for ; Wed, 3 Apr 2024 15:03:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8C7223847725 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 8C7223847725 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=158.69.221.121 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1712156632; cv=none; b=pKWKT8fB87cEv4mgwkfx0uAzYVQIpXJJOOE8eIl8VfHcvM+PQLPN5KvPVm65lL0xWvZpOzk9RlleR/9dHVFh8t8Ut+P3WNnEj39YTy2Z1fd+bHpmdqCvP4a7DvBAIEDatn62cr7zfk8raxUmOEYgdjc5HIj76hkjSYpbBgFLvTw= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1712156632; c=relaxed/simple; bh=DLXP4VfBq7zxj70bzjDNvGz/iwOW35upeie1fKj+kZc=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=LFR7l57YqP4/6I9tpSRgPPZyo4WuQUvqiWXsmI4yw3OnErWU8puAP4UUM0sIhZvF2Zi2VMFpwCv68oCREjUPFSxhXfQz3zLb1NcZnGE1NXZgyacto9odCbEJ+cGqhuvPRtDdwJZRD6WhubSONRcxUjbWaL3g/y87bWsxHkY6Fys= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1712156621; bh=DLXP4VfBq7zxj70bzjDNvGz/iwOW35upeie1fKj+kZc=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=cx2KPUrE/XS/w9hNjEMvWGbsGyD2OBpynNUJMCuNs7LxKZNOnrttRhjO/Sa9BvLnP uGfJgtDOVo7KFHkounByr27W6zlezi9xHtBlwTPOImPMOwwRzKV7lyHKBu6yX/yxZV 1xVosITVWJ2RfyzvJvZOJ2SInTzYLAQ/xjHCdL/g= Received: from [172.16.0.192] (192-222-143-198.qc.cable.ebox.net [192.222.143.198]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id C2D691E030; Wed, 3 Apr 2024 11:03:40 -0400 (EDT) Message-ID: <92c3c684-be79-4ece-ab2a-805e85145701@simark.ca> Date: Wed, 3 Apr 2024 11:03:39 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: Patches submission policy change To: Christophe Lyon , binutils@sourceware.org, GCC Mailing List , gdb@sourceware.org, Nick Clifton , Richard Biener , Jakub Jelinek , Joel Brobecker , Carlos O'Donell Cc: Maxim Kuvyrkov , Thiago Bauermann , Adhemerval Zanella References: Content-Language: fr From: Simon Marchi In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.7 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,KAM_MANYTO,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 4/3/24 4:22 AM, Christophe Lyon via Gdb wrote: > Dear release managers and developers, > > TL;DR: For the sake of improving precommit CI coverage and simplifying > workflows, I’d like to request a patch submission policy change, so > that we now include regenerated files. This was discussed during the > last GNU toolchain office hours meeting [1] (2024-03-28). > > Benefits or this change include: > - Increased compatibility with precommit CI > - No need to manually edit patches before submitting, thus the “git > send-email” workflow is simplified > - Patch reviewers can be confident that the committed patch will be > exactly what they approved > - Precommit CI can test exactly what has been submitted > > Any concerns/objections? > > As discussed on the lists and during the meeting, we have been facing > issues with testing a class of patches: those which imply regenerating > some files. Indeed, for binutils and gcc, the current patch submission > policy is to *not* include the regenerated files (IIUC the policy is > different for gdb [2]). > > This means that precommit CI receives an incomplete patch, leading to > wrong and misleading regression reports, and complaints/frustration. > (our notifications now include a warning, making it clear that we do > not regenerate files ;-) ) > > I thought the solution was as easy as adding –enable-maintainer-mode > to the configure arguments but this has proven to be broken (random > failures with highly parallel builds). I tried to workaround that by > adding new “regenerate” rules in the makefiles, that we could build at > -j1 before running the real build with a higher parallelism level, but > this is not ideal, not to mention that in the case of gcc, configuring > target libraries requires having built all-gcc before, which is quite > slow at -j1. > > Another approach used in binutils and gdb builtbots is a dedicated > script (aka autoregen.py) which calls autotools as appropriate. It > could be extended to update other types of files, but this can be a > bit tricky (eg. some opcodes files require to build a generator first, > some doc fragments also use non-trivial build sequences), and it > creates a maintenance issue: the build recipe is duplicated in the > script and in the makefiles. Such a script has proven to be useful > though in postcommit CI, to catch regeneration errors. > > Having said that, it seems that for the sake of improving usefulness > of precommit CI, the simplest way forward is to change the policy to > include regenerated files. This does not seem to be a burden for > developers, since they have to regenerate the files before pushing > their patches anyway, and it also enables reviewers to make sure the > generated files are correct. > > Said differently, if you want to increase the chances of having your > patches tested by precommit CI, make sure to include all the > regenerated files, otherwise you might receive failure notifications. > > Any concerns/objections? We already do that for GDB (include generated files), and it works fine. I sometimes have a patch that exceeds the mailing list limit (400k?), but it seems like the only consequence is that someone needs to approve it (thanks to whoever does that). Simon