public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
* Checks that autotools generated files were re-generated correctly
@ 2023-11-06 17:04 Martin Jambor
  2023-11-07 22:13 ` Frank Ch. Eigler
  2023-11-08 23:30 ` Mark Wielaard
  0 siblings, 2 replies; 10+ messages in thread
From: Martin Jambor @ 2023-11-06 17:04 UTC (permalink / raw)
  To: GCC Mailing List; +Cc: Maxim Kuvyrkov

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
false-positives should be low (I thought zero but see
https://gcc.gnu.org/pipermail/gcc-patches/2023-November/635358.html).

Thanks for any ideas which can lead to a mostly automated process.

Martin

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Checks that autotools generated files were re-generated correctly
  2023-11-06 17:04 Checks that autotools generated files were re-generated correctly Martin Jambor
@ 2023-11-07 22:13 ` Frank Ch. Eigler
  2023-11-08 23:30 ` Mark Wielaard
  1 sibling, 0 replies; 10+ messages in thread
From: Frank Ch. Eigler @ 2023-11-07 22:13 UTC (permalink / raw)
  To: Martin Jambor; +Cc: GCC Mailing List, Maxim Kuvyrkov

Martin Jambor <mjambor@suse.cz> writes:

> [...]  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.  [...]

The gccadmin account on sourceware already does some daily routine git
commits ("Daily bump.").  You could decide to delegate regeneration of
the configury to the same service.  It could run on a scheduled basis
rather than the commit hook, if people are angsty about that.

It would use designated master auto* tool versions.  None of the
developers would have to struggle keeping them matched: just commit the
inputs.  Or if they commit regenerated versions but with an oddball
auto* version, the sourceware bot could fix that later.

- FChE


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Checks that autotools generated files were re-generated correctly
  2023-11-06 17:04 Checks that autotools generated files were re-generated correctly Martin Jambor
  2023-11-07 22:13 ` Frank Ch. Eigler
@ 2023-11-08 23:30 ` Mark Wielaard
  2023-11-15 19:48   ` Mark Wielaard
  1 sibling, 1 reply; 10+ messages in thread
From: Mark Wielaard @ 2023-11-08 23:30 UTC (permalink / raw)
  To: Martin Jambor; +Cc: GCC Mailing List, Maxim Kuvyrkov

Hi Martin,

On Mon, Nov 06, 2023 at 06:04:48PM +0100, Martin Jambor wrote:
> 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.

Cool! A small python script cannot replace him of course. But it is
nice to get a small virtual mliska :)

> 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.

So this doesn't just need that script, but also an execution
environment that contains the right versions of the autotools. We
could install those of course, but running them from a git hook on
checkin indeed seems a little expensive.

Creating a container with the script and the right versions of all
tools might be the best thing. Then the script can be run from a cron
job or buildbot. Or even by someone hacking on the build/configure
scripts to make sure they are generating with the right tools.

builder.sourceware.org already contains various of such containers:
https://sourceware.org/cgit/builder/tree/builder/containers
https://sourceware.org/cgit/builder/tree/README_containers

Friday is Sourceware Open Office hour (#overseers on irc.libera.chat
at 18:00 UTC). We could hack something together then and see how to
hook it up.

Cheers,

Mark

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Checks that autotools generated files were re-generated correctly
  2023-11-08 23:30 ` Mark Wielaard
@ 2023-11-15 19:48   ` Mark Wielaard
  2023-11-16 18:37     ` Martin Jambor
  0 siblings, 1 reply; 10+ messages in thread
From: Mark Wielaard @ 2023-11-15 19:48 UTC (permalink / raw)
  To: Martin Jambor; +Cc: GCC Mailing List, Maxim Kuvyrkov, binutils, gdb

Hi! (adding gdb and binutils to the CC)

On Thu, Nov 09, 2023 at 12:30:59AM +0100, Mark Wielaard wrote:
> On Mon, Nov 06, 2023 at 06:04:48PM +0100, Martin Jambor wrote:
> > 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.
> 
> Cool! A small python script cannot replace him of course. But it is
> nice to get a small virtual mliska :)
> 
> > 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.
> 
> So this doesn't just need that script, but also an execution
> environment that contains the right versions of the autotools. We
> could install those of course, but running them from a git hook on
> checkin indeed seems a little expensive.
> 
> Creating a container with the script and the right versions of all
> tools might be the best thing. Then the script can be run from a cron
> job or buildbot. Or even by someone hacking on the build/configure
> scripts to make sure they are generating with the right tools.
> 
> builder.sourceware.org already contains various of such containers:
> https://sourceware.org/cgit/builder/tree/builder/containers
> https://sourceware.org/cgit/builder/tree/README_containers
> 
> Friday is Sourceware Open Office hour (#overseers on irc.libera.chat
> at 18:00 UTC). We could hack something together then and see how to
> hook it up.

So we did indeed discuss and hack something together.

The container file is here:
https://sourceware.org/cgit/builder/tree/builder/containers/Containerfile-autotools

The script is here:
https://sourceware.org/cgit/builder/tree/builder/containers/autoregen.py

A buildbot can then use that container to run that script and then
check that git diff is empty on every commit to binutils-gdb.git and
gcc.git. If it finds an issue it will sent email with the diff.

It already found issues in both gcc and binutils-gdb. All fixed now.

Thanks to Arsen and Sam for testing, fixing and updating the script
(it now also runs aclocal).

If you want to run this locally you can use the container file to
create an image and then run autoregen.py on your local tree by bind
mounting your git tree inside it.

$ git clone https://sourceware.org/git/builder.git
$ cd builder/
$ podman build -t autotools -f builder/containers/Containerfile-autotools \
  builder/containers
$ cd .. # assuming your binutils-gdb directory is here
$ podman run --privileged -u root -v $(pwd)/binutils-gdb:/binutils-gdb \
  -ti --entrypoint "/bin/bash" autotools
# then inside the container
 cd /binutils-gdb
 autoregen.py

Change binutils-gdb to gcc if you are working on gcc.
You should also be able to do the above with docker instead of podman.

Cheers,

Mark

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Checks that autotools generated files were re-generated correctly
  2023-11-15 19:48   ` Mark Wielaard
@ 2023-11-16 18:37     ` Martin Jambor
  0 siblings, 0 replies; 10+ messages in thread
From: Martin Jambor @ 2023-11-16 18:37 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: GCC Mailing List, Maxim Kuvyrkov, binutils, gdb

Hi,

On Wed, Nov 15 2023, Mark Wielaard wrote:
> Hi! (adding gdb and binutils to the CC)
>
> On Thu, Nov 09, 2023 at 12:30:59AM +0100, Mark Wielaard wrote:
>> On Mon, Nov 06, 2023 at 06:04:48PM +0100, Martin Jambor wrote:
>> > 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.
>> 
>> Cool! A small python script cannot replace him of course. But it is
>> nice to get a small virtual mliska :)
>> 
>> > 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.
>> 
>> So this doesn't just need that script, but also an execution
>> environment that contains the right versions of the autotools. We
>> could install those of course, but running them from a git hook on
>> checkin indeed seems a little expensive.
>> 
>> Creating a container with the script and the right versions of all
>> tools might be the best thing. Then the script can be run from a cron
>> job or buildbot. Or even by someone hacking on the build/configure
>> scripts to make sure they are generating with the right tools.
>> 
>> builder.sourceware.org already contains various of such containers:
>> https://sourceware.org/cgit/builder/tree/builder/containers
>> https://sourceware.org/cgit/builder/tree/README_containers
>> 
>> Friday is Sourceware Open Office hour (#overseers on irc.libera.chat
>> at 18:00 UTC). We could hack something together then and see how to
>> hook it up.
>
> So we did indeed discuss and hack something together.

Wonderful, thanks to everyone working to improve the situation with
these errors.

Martin

>
> The container file is here:
> https://sourceware.org/cgit/builder/tree/builder/containers/Containerfile-autotools
>
> The script is here:
> https://sourceware.org/cgit/builder/tree/builder/containers/autoregen.py
>
> A buildbot can then use that container to run that script and then
> check that git diff is empty on every commit to binutils-gdb.git and
> gcc.git. If it finds an issue it will sent email with the diff.
>
> It already found issues in both gcc and binutils-gdb. All fixed now.
>
> Thanks to Arsen and Sam for testing, fixing and updating the script
> (it now also runs aclocal).
>
> If you want to run this locally you can use the container file to
> create an image and then run autoregen.py on your local tree by bind
> mounting your git tree inside it.
>
> $ git clone https://sourceware.org/git/builder.git
> $ cd builder/
> $ podman build -t autotools -f builder/containers/Containerfile-autotools \
>   builder/containers
> $ cd .. # assuming your binutils-gdb directory is here
> $ podman run --privileged -u root -v $(pwd)/binutils-gdb:/binutils-gdb \
>   -ti --entrypoint "/bin/bash" autotools
> # then inside the container
>  cd /binutils-gdb
>  autoregen.py
>
> Change binutils-gdb to gcc if you are working on gcc.
> You should also be able to do the above with docker instead of podman.
>
> Cheers,
>
> Mark

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Checks that autotools generated files were re-generated correctly
       [not found]     ` <654a4b60.2e0a0220.7f3b.6e5aSMTPIN_ADDED_BROKEN@mx.google.com>
@ 2023-11-07 15:42       ` Christophe Lyon
  0 siblings, 0 replies; 10+ messages in thread
From: Christophe Lyon @ 2023-11-07 15:42 UTC (permalink / raw)
  To: Martin Jambor
  Cc: Maxim Kuvyrkov, GCC Mailing List, Siddhesh Poyarekar,
	Carlos O'Donell

On Tue, 7 Nov 2023 at 15:36, Martin Jambor <mjambor@suse.cz> wrote:
>
> 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.
>
Well, making such checks in a precommit-CI means that authors should
include regenerated files in their patch submissions, so it seems this
would imply a policy change (not impossible, but will likely take some
time to get consensus?)

> >>
> >> 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).

Great, I was thinking about writing something like that :-)

> 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< ------------------------------

Nice, thanks for sharing.

>
> > 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.

Indeed, but as Maxim pointed out to me, we would probably want to run
the same kind of checks for binutils and gdb, so it's better if the CI
can clone a very small repo with such scripts rather than having to
clone the whole GCC when checking if a binutils patch is well-formed.

I'm happy with Carlos' proposal of using a new gitlab repo with such
utilities/checkers, which we can then use from various CI
implementations.

Thanks,

Christophe

>
> Thanks,
>
> Martin

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Checks that autotools generated files were re-generated correctly
  2023-11-07  7:38   ` Maxim Kuvyrkov
  2023-11-07 13:50     ` Carlos O'Donell
@ 2023-11-07 14:36     ` Martin Jambor
       [not found]     ` <654a4b60.2e0a0220.7f3b.6e5aSMTPIN_ADDED_BROKEN@mx.google.com>
  2 siblings, 0 replies; 10+ messages in thread
From: Martin Jambor @ 2023-11-07 14:36 UTC (permalink / raw)
  To: Maxim Kuvyrkov
  Cc: GCC Mailing List, Christophe Lyon, Siddhesh Poyarekar,
	Carlos O'Donell

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Checks that autotools generated files were re-generated correctly
  2023-11-07  7:38   ` Maxim Kuvyrkov
@ 2023-11-07 13:50     ` Carlos O'Donell
  2023-11-07 14:36     ` Martin Jambor
       [not found]     ` <654a4b60.2e0a0220.7f3b.6e5aSMTPIN_ADDED_BROKEN@mx.google.com>
  2 siblings, 0 replies; 10+ messages in thread
From: Carlos O'Donell @ 2023-11-07 13:50 UTC (permalink / raw)
  To: Maxim Kuvyrkov, Martin Jambor
  Cc: GCC Mailing List, Christophe Lyon, Siddhesh Poyarekar

On 11/7/23 02:38, Maxim Kuvyrkov wrote:
>> 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.

$0.02.

We should move in a direction where all server side push hooks removed.

Removing the hooks allows for easy repo replication, and sharing load.

Such checks should all be moved IMO into pre-commit CI, or post-commit CI.

>>> 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?
>> 
>> 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.

This is a great way to handle this until we have more consensus around other
kinds of worfklows.

> 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.  I think Siddhesh and Carlos are looking into
> creating such a repo on gitlab?

I can make any repo we want here:

https://gitlab.com/gnutools

-- 
Cheers,
Carlos.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Checks that autotools generated files were re-generated correctly
  2023-11-06 17:19 ` Christophe Lyon
@ 2023-11-07  7:38   ` Maxim Kuvyrkov
  2023-11-07 13:50     ` Carlos O'Donell
                       ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Maxim Kuvyrkov @ 2023-11-07  7:38 UTC (permalink / raw)
  To: Martin Jambor
  Cc: GCC Mailing List, Christophe Lyon, Siddhesh Poyarekar,
	Carlos O'Donell

> 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?
> 
> 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.

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.  I think Siddhesh and Carlos are looking into creating such a repo on gitlab?

Thanks,

--
Maxim Kuvyrkov
https://www.linaro.org


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Checks that autotools generated files were re-generated correctly
       [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
  0 siblings, 1 reply; 10+ messages in thread
From: Christophe Lyon @ 2023-11-06 17:19 UTC (permalink / raw)
  To: Martin Jambor; +Cc: GCC Mailing List, Maxim Kuvyrkov

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?

We can probably implement that, indeed. Is that the general agreement?

Thanks,

Christophe

> false-positives should be low (I thought zero but see
> https://gcc.gnu.org/pipermail/gcc-patches/2023-November/635358.html).
>
> Thanks for any ideas which can lead to a mostly automated process.
>
> Martin

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-11-16 18:37 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-06 17:04 Checks that autotools generated files were re-generated correctly 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
     [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
     [not found]     ` <654a4b60.2e0a0220.7f3b.6e5aSMTPIN_ADDED_BROKEN@mx.google.com>
2023-11-07 15:42       ` Christophe Lyon

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).