public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
From: Christophe Lyon <christophe.lyon@linaro.org>
To: binutils@sourceware.org, GCC Mailing List <gcc@gcc.gnu.org>,
	gdb@sourceware.org,  Nick Clifton <nickc@redhat.com>,
	Richard Biener <rguenther@suse.de>,
	Jakub Jelinek <jakub@redhat.com>,
	 Joel Brobecker <brobecker@adacore.com>,
	"Carlos O'Donell" <carlos@redhat.com>
Cc: Maxim Kuvyrkov <maxim.kuvyrkov@linaro.org>,
	 Thiago Bauermann <thiago.bauermann@linaro.org>,
	 Adhemerval Zanella <adhemerval.zanella@linaro.org>
Subject: Patches submission policy change
Date: Wed, 3 Apr 2024 10:22:24 +0200	[thread overview]
Message-ID: <CAPS5khZeWkAD=V8ka9g5eECDNK3BDHFNZJuMpVC+HeDmkVJMCg@mail.gmail.com> (raw)

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?

Thanks,

Christophe

[1] https://gcc.gnu.org/wiki/OfficeHours#Meeting:_2024-03-28_.40_1100h_EST5EDT
[2] https://inbox.sourceware.org/gdb/cc0a5c86-a041-429a-9890-efd3937606e5@simark.ca/

             reply	other threads:[~2024-04-03  8:22 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-03  8:22 Christophe Lyon [this message]
2024-04-03  8:30 ` Jan Beulich
2024-04-03 13:11   ` Christophe Lyon
2024-04-04  8:12     ` Jan Beulich
2024-04-05  7:22       ` Christophe Lyon
2024-04-03  8:45 ` Jakub Jelinek
2024-04-03  8:49   ` Jan Beulich
2024-04-03  8:57     ` Richard Biener
2024-04-03 10:21       ` Jan Beulich
2024-04-03 12:58         ` Joel Sherrill
2024-04-03 13:23           ` Christophe Lyon
2024-04-08 15:37             ` Richard Earnshaw (lists)
2024-04-03 12:59         ` Jason Merrill
2024-04-03 13:19         ` Christophe Lyon
2024-04-03  9:50   ` Jonathan Wakely
2024-04-03 15:03 ` Simon Marchi
2024-04-04 21:35 ` Mark Wielaard
2024-04-04 21:51   ` Simon Marchi
2024-04-05  6:44   ` Marc
2024-04-05  7:17   ` Christophe Lyon
2024-04-06 16:29     ` Mark Wielaard
2024-04-07 12:32   ` Jonathan Wakely
2024-04-07 14:02     ` Mark Wielaard
2024-04-07 14:20       ` Jonathan Wakely
2024-04-07 22:00         ` Mark Wielaard

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='CAPS5khZeWkAD=V8ka9g5eECDNK3BDHFNZJuMpVC+HeDmkVJMCg@mail.gmail.com' \
    --to=christophe.lyon@linaro.org \
    --cc=adhemerval.zanella@linaro.org \
    --cc=binutils@sourceware.org \
    --cc=brobecker@adacore.com \
    --cc=carlos@redhat.com \
    --cc=gcc@gcc.gnu.org \
    --cc=gdb@sourceware.org \
    --cc=jakub@redhat.com \
    --cc=maxim.kuvyrkov@linaro.org \
    --cc=nickc@redhat.com \
    --cc=rguenther@suse.de \
    --cc=thiago.bauermann@linaro.org \
    /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 read-only IMAP folder(s) and NNTP newsgroup(s).