Warn secondary arch folks about change involving: ExclusiveArch or ExcludesArch

[Pierre-Yves Chibon]

Good Morning,

Rel-eng has been wanting a got hook that would notify the people involved with
secondary arches about changes made to packages involving ExclusiveArch or
ExcludesArch.

I took a stab at this hook today and I would like to offer the result for
review.

So with this email is an ansible patch adding the hook and installing it in our
repos.

Things that needs to be tweaked:
- Where to send the email?
- Turn off debug after we confirmed it works
- Currently I install the script as a file not a template, so the URL to point
  to commits in CGIT will be the same in prod and in stg.

Comments and thoughts more than welcome :)

Pierre

[Kevin Fenzi]

On Wed, 19 Oct 2016 12:46:04 +0200
Pierre-Yves Chibon  wrote:
> Good Morning,
>
> Rel-eng has been wanting a got hook that would notify the people
> involved with secondary arches about changes made to packages
> involving ExclusiveArch or ExcludesArch.
>
> I took a stab at this hook today and I would like to offer the result
> for review.
>
> So with this email is an ansible patch adding the hook and installing
> it in our repos.
>
> Things that needs to be tweaked:
> Where to send the email?
> Turn off debug after we confirmed it works
> Currently I install the script as a file not a template, so the URL
>
> to point to commits in CGIT will be the same in prod and in stg.
>
> Comments and thoughts more than welcome :)

It looks fine to me, but I thought we were going to move away from
'secondary arch' in favor of 'alternative arch' ? But I guess this is
really just anything excluded, it could be a primary as well?

I guess the email might go to the releng list? Or devel I suppose, but
might be noisy there.

kevin

[Pierre-Yves Chibon]

On Wed, Oct 19, 2016 at 11:13:25AM -0600, Kevin Fenzi wrote:
> On Wed, 19 Oct 2016 12:46:04 +0200
> Pierre-Yves Chibon  wrote:
> > Good Morning,
> >
> > Rel-eng has been wanting a got hook that would notify the people
> > involved with secondary arches about changes made to packages
> > involving ExclusiveArch or ExcludesArch.
> >
> > I took a stab at this hook today and I would like to offer the result
> > for review.
> >
> > So with this email is an ansible patch adding the hook and installing
> > it in our repos.
> >
> > Things that needs to be tweaked:
> > Where to send the email?
> > Turn off debug after we confirmed it works
> > Currently I install the script as a file not a template, so the URL
> >
> > to point to commits in CGIT will be the same in prod and in stg.
> >
> > Comments and thoughts more than welcome :)
>
> It looks fine to me, but I thought we were going to move away from
> 'secondary arch' in favor of 'alternative arch' ?

That can very well be and I missed it, so let's please s/secondary/alternative/g
here :)
> But I guess this is
> really just anything excluded, it could be a primary as well?

The hook will point out any commit that is touching either ExclusiveArch or
ExcludesArch, regardless of the arch impacted.
There will be false-positive for sure (for example epel packages introduced in
certain arch which doesn't exist in RHEL).
> I guess the email might go to the releng list? Or devel I suppose, but
> might be noisy there.

Is there an alternative-arch-sig list of some sort?

Pierre

[Josh Boyer]

> On Wed, Oct 19, 2016 at 11:13:25AM -0600, Kevin Fenzi wrote:
>
> That can very well be and I missed it, so let's please s/secondary/alternative/g
> here :)

Please!
> The hook will point out any commit that is touching either ExclusiveArch or
> ExcludesArch, regardless of the arch impacted.
> There will be false-positive for sure (for example epel packages introduced in
> certain arch which doesn't exist in RHEL).

Can the hook exclude anything in EPEL?
> Is there an alternative-arch-sig list of some sort?

[e-mail] (ironic, I know).  It might be worth looking at migrating that list to alternative-arches or something.

josh

[Pierre-Yves Chibon]

On Thu, Oct 20, 2016 at 12:40:29AM -0000, Josh Boyer wrote:
> > On Wed, Oct 19, 2016 at 11:13:25AM -0600, Kevin Fenzi wrote:
> >
> > That can very well be and I missed it, so let's please s/secondary/alternative/g
> > here :)
>
> Please!

I'll rename and adjust the file if/when we'll all be ok with it (I'm fine with
re-sending it for review then).
> > The hook will point out any commit that is touching either ExclusiveArch or
> > ExcludesArch, regardless of the arch impacted.
> > There will be false-positive for sure (for example epel packages introduced in
> > certain arch which doesn't exist in RHEL).
>
> Can the hook exclude anything in EPEL?

Sure, this should do it:

diff --git a/ roles/git/hooks/files/post-receive-secondaryarch b/ roles/git/hooks/files/post-receive-secondaryarch
index 9ad165f..189c69c 100755
--- a/ roles/git/hooks/files/post-receive-secondaryarch 
+++ b/ roles/git/hooks/files/post-receive-secondaryarch 
@@ -147,6 +147,11 @@ def run_as_post_receive_hook():
             print('Received:', line.strip())
         (oldrev, newrev, refname) = line.strip().split(' ', 2)

+        # Skip all the branches that are not master and do not start with an 'f'
+        # ie: personal branches and epel
+        if not refname.startswith(('refs/heads/master', 'refs/heads/f')):
+            continue
+
         new_commits_list = get_revs_between(oldrev, newrev, abspath, refname)
         if DEBUG:
             print('List of commits:', new_commits_list)
> > Is there an alternative-arch-sig list of some sort?
>
> [e-mail] (ironic, I know).  It might be worth looking at migrating that list to alternative-arches or something.

Sounds like a good list for me, are the people interested in these emails
subscribed there? (I suspect they are, but better safe than sorry :))

Pierre

[Josh Boyer]

> On Thu, Oct 20, 2016 at 12:40:29AM -0000, Josh Boyer wrote:
>
> I'll rename and adjust the file if/when we'll all be ok with it (I'm fine
> with
> re-sending it for review then).
>
> Sure, this should do it:
>
> diff --git a/ roles/git/hooks/files/post-receive-secondaryarch b/
> roles/git/hooks/files/post-receive-secondaryarch
> index 9ad165f..189c69c 100755
> --- a/ roles/git/hooks/files/post-receive-secondaryarch 
> +++ b/ roles/git/hooks/files/post-receive-secondaryarch 
> @@ -147,6 +147,11 @@ def run_as_post_receive_hook():
>              print('Received:', line.strip())
>          (oldrev, newrev, refname) = line.strip().split(' ', 2)
>    # Skip all the branches that are not master and do not start with an 'f'
>    # ie: personal branches and epel
>    if not refname.startswith(('refs/heads/master',
>
> 'refs/heads/f')):
>        continue
>    new_commits_list = get_revs_between(oldrev, newrev, abspath, refname)
>    if DEBUG:
>        print('List of commits:', new_commits_list)
>
>
>
> Sounds like a good list for me, are the people interested in these emails
> subscribed there? (I suspect they are, but better safe than sorry :))

The list is extremely low traffic.  In the absence of any kind of current postings, it's hard to really tell for sure.  I think using the list is fine and if for some reason we get a number of complaints we can adjust.  I don't expect that to happen though.

josh

[Dennis Gilmore]

On miércoles, 19 de octubre de 2016 12:46:04 PM CDT Pierre-Yves Chibon wrote:
> Good Morning,
>
> Rel-eng has been wanting a got hook that would notify the people involved
> with secondary arches about changes made to packages involving
> ExclusiveArch or ExcludesArch.
>
> I took a stab at this hook today and I would like to offer the result for
> review.
>
> So with this email is an ansible patch adding the hook and installing it in
> our repos.
>
> Things that needs to be tweaked:
> Where to send the email?
> Turn off debug after we confirmed it works
> Currently I install the script as a file not a template, so the URL to
>
> point to commits in CGIT will be the same in prod and in stg.
>
> Comments and thoughts more than welcome :)
>
> Pierre

It should be tied into fmn for noticiations for those that care about them. 
there is groups for the secondary arches in fas, at the least members of the 
groups should get them. it is not for just alternative arches. but people 
should be able to opt in for all arch exclusions or even just x86_64

Dennis

[Pierre-Yves Chibon]

On Thu, Oct 27, 2016 at 02:15:51PM -0500, Dennis Gilmore wrote:
> On miércoles, 19 de octubre de 2016 12:46:04 PM CDT Pierre-Yves Chibon wrote:
> > Good Morning,
> >
> > Rel-eng has been wanting a got hook that would notify the people involved
> > with secondary arches about changes made to packages involving
> > ExclusiveArch or ExcludesArch.
> >
> > I took a stab at this hook today and I would like to offer the result for
> > review.
> >
> > So with this email is an ansible patch adding the hook and installing it in
> > our repos.
> >
> > Things that needs to be tweaked:
> > Where to send the email?
> > Turn off debug after we confirmed it works
> > Currently I install the script as a file not a template, so the URL to
> >
> > point to commits in CGIT will be the same in prod and in stg.
> >
> > Comments and thoughts more than welcome :)
>
> It should be tied into fmn for noticiations for those that care about them. 
> there is groups for the secondary arches in fas, at the least members of the 
> groups should get them. it is not for just alternative arches. but people 
> should be able to opt in for all arch exclusions or even just x86_64

So while this is an interesting idea it involves quite a bit more work than what
we have now.
So what about deploying this first instance and pointing the notifications to
that <that_FAS_group>[e-mail] and see from there if we want to
go the fedmsg, fedmsg_meta, FMN way.

Thoughts?

Pierre

PS: do you know by any changes the name of that FAS group you're speaking about?

[Dennis Gilmore]

On viernes, 28 de octubre de 2016 4:04:11 PM CDT Pierre-Yves Chibon wrote:
> On Thu, Oct 27, 2016 at 02:15:51PM -0500, Dennis Gilmore wrote:
> > On miércoles, 19 de octubre de 2016 12:46:04 PM CDT Pierre-Yves Chibon

wrote:
> > > Good Morning,
> > >
> > > Rel-eng has been wanting a got hook that would notify the people
> > > involved
> > > with secondary arches about changes made to packages involving
> > > ExclusiveArch or ExcludesArch.
> > >
> > > I took a stab at this hook today and I would like to offer the result
> > > for
> > > review.
> > >
> > > So with this email is an ansible patch adding the hook and installing it
> > > in
> > > our repos.
> > >
> > > Things that needs to be tweaked:
> > > Where to send the email?
> > > Turn off debug after we confirmed it works
> > > Currently I install the script as a file not a template, so the URL to
> > >
> > > point to commits in CGIT will be the same in prod and in stg.
> > >
> > > Comments and thoughts more than welcome :)
> >
> > It should be tied into fmn for noticiations for those that care about
> > them.
> > there is groups for the secondary arches in fas, at the least members of
> > the groups should get them. it is not for just alternative arches. but
> > people should be able to opt in for all arch exclusions or even just
> > x86_64
>
> So while this is an interesting idea it involves quite a bit more work than
> what we have now.
> So what about deploying this first instance and pointing the notifications
> to that <that_FAS_group>[e-mail] and see from there if we
> want to go the fedmsg, fedmsg_meta, FMN way.

so anything that adds or excludes %{arm} armv7hl aarch64 should go to fedora-
[e-mail]
ppc64 ppc64p7 and ppc64le should go to [e-mail] 
and snything excluding s390x should go to fedora-s390-
[e-mail]

none of the other arches are setup with fas groups. but maybe we should add 
one for i386 and x86_64  there is possibly some value in having things go to 
the secondary arch list.
> Thoughts?

sounds like a good start. I need to file some tickets to expand fmn, fedmsg 
etc coverage for release deliverables also. so when cloud images fail people 
that care get notification etc

Dennis
> Pierre
>
> PS: do you know by any changes the name of that FAS group you're speaking
> about?

[Pierre-Yves Chibon]

On Fri, Oct 28, 2016 at 01:24:39PM -0500, Dennis Gilmore wrote:
> so anything that adds or excludes %{arm} armv7hl aarch64 should go to fedora-
> [e-mail]
> ppc64 ppc64p7 and ppc64le should go to [e-mail] 
> and snything excluding s390x should go to fedora-s390-
> [e-mail]

The hook is now deployed and sending everything to me for now.
I'll adjust it based on these suggestions.

One thing I realized is that now is that there might be some false-positive, for
example:
````
The package rpms/R-DynDoc.git has added or updated either ExclusiveArch or
ExcludesArch
in commit(s):
http://pkgs.fedoraproject.org/cgit/rpms/R-DynDoc.git/commit/?id=1186b2e7013a....

Change:
+ExclusiveArch:    armv7, ppc, go_arch
````

But all I did was adding 'go_arch', so two solutions:
1) warn arm and ppc folks again
2) make some of diff between:
   -ExclusiveArch:    armv7, ppc
   +ExclusiveArch:    armv7, ppc, go_arch
   and extract which arch got changed to warn only the groups of interest

Thoughts?

There are also changes such as:
+ExclusiveArch:  %{?go_arches:%{go_arches}}%{!?go_arches:%{ix86} x86_64 %{arm}}

How should these be treated? Warn the alternative-arch list? arm?

Thanks,
Pierre

[Dennis Gilmore]

On Wednesday, November 2, 2016 12:08:50 PM CDT Pierre-Yves Chibon wrote:
> On Fri, Oct 28, 2016 at 01:24:39PM -0500, Dennis Gilmore wrote:
> > so anything that adds or excludes %{arm} armv7hl aarch64 should go to
> > fedora- [e-mail]
> > ppc64 ppc64p7 and ppc64le should go to
> > [e-mail]
> > and snything excluding s390x should go to fedora-s390-
> > [e-mail]
>
> The hook is now deployed and sending everything to me for now.
> I'll adjust it based on these suggestions.
>
> One thing I realized is that now is that there might be some false-positive,
> for example:
> The package rpms/R-DynDoc.git has added or updated either ExclusiveArch or
> ExcludesArch
> in commit(s):
> http://pkgs.fedoraproject.org/cgit/rpms/R-DynDoc.git/commit/?id=1186b2e7013a
> eaa3e88c5b36541a4b841f3b7dc3.
>
> Change:
> +ExclusiveArch:    armv7, ppc, go_arch
>
> But all I did was adding 'go_arch', so two solutions:
> warn arm and ppc folks again
> make some of diff between:
> -ExclusiveArch:    armv7, ppc
> +ExclusiveArch:    armv7, ppc, go_arch
> and extract which arch got changed to warn only the groups of interest
>
> Thoughts?
>
> There are also changes such as:
> +ExclusiveArch:  %{?go_arches:%{go_arches}}%{!?go_arches:%{ix86} x86_64
> %{arm}}
>
> How should these be treated? Warn the alternative-arch list? arm?
>
> Thanks,
> Pierre

likely in the case of something confusing we should default to generic. I am 
not sure the best way to figure out the macros used in different exclude 
lists. and they can and do change on a per release basis

Dennis