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
