public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
From: Martin Jambor <mjambor@suse.cz>
To: Maxim Kuvyrkov <maxim.kuvyrkov@linaro.org>
Cc: GCC Mailing List <gcc@gcc.gnu.org>,
	Christophe Lyon <christophe.lyon@linaro.org>,
	Siddhesh Poyarekar <siddhesh@redhat.com>,
	Carlos O'Donell <carlos@redhat.com>
Subject: Re: Checks that autotools generated files were re-generated correctly
Date: Tue, 07 Nov 2023 15:36:14 +0100	[thread overview]
Message-ID: <ri68r791vhd.fsf@> (raw)
In-Reply-To: <BF1173C7-C639-41A0-8709-32AD4BBE693B@linaro.org>

Hello,

On Tue, Nov 07 2023, Maxim Kuvyrkov wrote:
n>> On Nov 6, 2023, at 21:19, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>> 
>> Hi!
>> 
>> On Mon, 6 Nov 2023 at 18:05, Martin Jambor <mjambor@suse.cz> wrote:
>>> 
>>> Hello,
>>> 
>>> I have inherited Martin Liška's buildbot script that checks that all
>>> sorts of autotools generated files, mainly configure scripts, were
>>> re-generated correctly when appropriate.  While the checks are hopefully
>>> useful, they report issues surprisingly often and reporting them feels
>>> especially unproductive.
>>> 
>>> Could such checks be added to our server side push hooks so that commits
>>> introducing these breakages would get refused automatically.  While the
>>> check might be a bit expensive, it only needs to be run on files
>>> touching the generated files and/or the files these are generated from.
>>> 
>>> Alternatively, Maxim, you seem to have an infrastructure that is capable
>>> of sending email.  Would you consider adding the check to your buildbot
>>> instance and report issues automatically?  The level of totally
>> 
>> After the discussions we had during Cauldron, I actually thought we
>> should add such a bot.
>> 
>> Initially I was thinking about adding this as a "precommit" check, to
>> make sure the autogenerated files were submitted correctly, but I
>> realized that the policy is actually not to send autogenerated files
>> as part of the patch (thus making pre-commit check impracticable in
>> such cases, unless we autogenerate those files after applying the
>> patch)
>> 
>> I understand you mean to run this as a post-commit bot, meaning we
>> would continue to "accept" broken commits, but now automatically send
>> a notification, asking for a prompt fix?

My thinking was that ideally bad commits would get refused early, like
when you get your ChangeLog completely wrong, but if there are drawbacks
to that approach, a completely automated notification system would be
great too.

>> 
>> We can probably implement that, indeed. Is that the general agreement?
>
> [CC: Siddhesh, Carlos]
>
> Hi Martin,
>
> I agree with Christophe, and we can add various source-level checks
> and wrap them up as a post-commit job.  The job will then send out
> email reports to developers whose patches failed it.

Thanks, automating this would be a huge improvement.

>
> Where the current script is located?  These checks would be useful for
> all GNU Toolchain projects -- binutils/GDB, GCC, Glibc and, maybe,
> Newlib -- so it would be useful to put it in a separate "gnutools"
> repo.

The test consists of running a python script that I'm pasting below in a
directory with a current master branch and subsequently checking that
"git diff" does not actually produce any diff (which currently does).
You need to have locally built autotools utilities of exactly the right
version.  The script (written by Martin Liška) is:

------------------------------ 8< ------------------------------
#!/usr/bin/env python3

import os
import subprocess
from pathlib import Path

AUTOCONF_BIN = 'autoconf-2.69'
AUTOMAKE_BIN = 'automake-1.15.1'
ACLOCAL_BIN = 'aclocal-1.15.1'
AUTOHEADER_BIN = 'autoheader-2.69'

ENV = f'AUTOCONF={AUTOCONF_BIN} ACLOCAL={ACLOCAL_BIN} AUTOMAKE={AUTOMAKE_BIN}'

config_folders = []

for root, _, files in os.walk('.'):
    for file in files:
        if file == 'configure':
            config_folders.append(Path(root).resolve())

for folder in sorted(config_folders):
    print(folder, flush=True)
    os.chdir(folder)
    configure_lines = open('configure.ac').read().splitlines()
    if any(True for line in configure_lines if line.startswith('AC_CONFIG_HEADERS')):
        subprocess.check_output(f'{ENV} {AUTOHEADER_BIN} -f', shell=True, encoding='utf8')
    # apparently automake is somehow unstable -> skip it for gotools
    if (any(True for line in configure_lines if line.startswith('AM_INIT_AUTOMAKE'))
            and not str(folder).endswith('gotools')):
        subprocess.check_output(f'{ENV} {AUTOMAKE_BIN} -f',
                                shell=True, encoding='utf8')
    subprocess.check_output(f'{ENV} {AUTOCONF_BIN} -f', shell=True, encoding='utf8')

------------------------------ 8< ------------------------------


> I think Siddhesh and Carlos are looking into creating such a repo on
> gitlab?

I guess this particular script may be even put into gcc's contrib
directory.  But it can be put anywhere where it makes most sense and
suits you best.

Thanks,

Martin

  parent reply	other threads:[~2023-11-07 14:36 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <65491cda.c80a0220.5fa79.7688SMTPIN_ADDED_BROKEN@mx.google.com>
2023-11-06 17:19 ` Christophe Lyon
2023-11-07  7:38   ` Maxim Kuvyrkov
2023-11-07 13:50     ` Carlos O'Donell
2023-11-07 14:36     ` Martin Jambor [this message]
     [not found]     ` <654a4b60.2e0a0220.7f3b.6e5aSMTPIN_ADDED_BROKEN@mx.google.com>
2023-11-07 15:42       ` Christophe Lyon
2023-11-06 17:04 Martin Jambor
2023-11-07 22:13 ` Frank Ch. Eigler
2023-11-08 23:30 ` Mark Wielaard
2023-11-15 19:48   ` Mark Wielaard
2023-11-16 18:37     ` Martin Jambor

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=ri68r791vhd.fsf@ \
    --to=mjambor@suse.cz \
    --cc=carlos@redhat.com \
    --cc=christophe.lyon@linaro.org \
    --cc=gcc@gcc.gnu.org \
    --cc=maxim.kuvyrkov@linaro.org \
    --cc=siddhesh@redhat.com \
    /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).