Fedora 35 Change: rpmautospec - removing release and changelog fields from spec files (System-Wide Change proposal)

[Ben Cotton]

https://fedoraproject.org/wiki/Changes/rpmautospec

== Summary ==
The goal of this change is to deploy in production the
[https://docs.pagure.org/fedora-infra.rpmautospec/ rpmautospec]
project.

With it, the content of the `Release` and `%changelog` fields in spec
files can be auto-generated, either locally or in the builder using
the information present in the git repo (in the form of git tags).


Note: This proposal is about changing the way the `Release` and
`%changelog` sections of the '''spec files''' are filled, not
about
removing them from the SRPM or binary RPM.

== Owner ==
* Name: [[User:pingou| Pierre-Yves Chibon]]
* Email: pingou - at - pingoured.fr
* Name: [[User:nphilipp| Nils Philippsen]]
* Email: nphilipp - at - redhat.com


== Detailed Description ==

rpmautospec offers packagers who want to use it the possibility of
replacing the content of the `Release` of their spec file by `Release:
   %autorel` and/or replace the content of the `%changelog` section of
their spec file by:
  %changelog
  %autochangelog

Both `%autorel` and `%autochangelog` are RPM macros that will be
expanded or replaced when the SRPM is built on the build system by
their corresponding values according to rpmautospec.

An overview of how rpmautospec works can be found at:
https://docs.pagure.org/fedora-infra.rpmautospec/principle.html.
We will describe below how each macro works.

=== The %autorel macro ===

To determine the next release information, rpmautospec relies on the
build history of the package.
This information is extracted from the buildsystem when running as a
koji plugin and from git tags when running outside of the buildsystem.

Using the build history of the package (ie a list of NEVRs) as well as
the information provided by the packager in the spec file, rpmautospec
then computes the next best release number for the package.

Once defined, it prepends a suitably defined %autorel macro to the top
of the spec file, freezing the computed value of the release number
and thus allowing reproducible builds of the SRPM.

The [https://docs.pagure.org/fedora-infra.rpmautospec/autorel.html
documentation of the autorel macro] describes how packagers can use it
to provide extra information.

=== The %autochangelog macro ===

The %autochangelog macro is in fact more a placeholder that
rpmautospec fills in during the creation of the SRPM (or when ran
manually on a project).

rpmautospec uses two sources of information to automatically generate
the changelog:
* an optional `changelog` file that packagers can add to the repository
* the git history of the spec file

rpmautospec will include the `changelog` as is in the `%changelog`
section and will use all the commits made to the repository after the
last change of the `changelog` file to generate the changelog.

In other words, if the packager has a repository created on January
10th and works on it for 6 months, adds a `changelog` file on June
1st. All the commits made before that commit of June 1st will be
ignored in favor of the content of the `changelog` file.
Similarly, if the packager then edits the file on July 1st, all the
commits prior to that commit of July 1st will be ignored.
All the commits involving files ending with either ".spec" or
".patch"
(this list can be expended if desired) and made after July 1st will be
used to generate the changelog.


== Benefit to Fedora ==

In short: easier packaging in Fedora for the packagers who opt-in.

The `Release` and `%changelog` fields are the two most conflicting
fields in RPM spec files. They impact most pull requests if they
involve updating the package or if the package is updated/rebuilt
while pull-request are being reviewed.

We currently have three different change logs per package and while
they target different audiences (package maintainer: git, sysadmin:
%changelog and end-user: bodhi notes) they overlap a lot and in most
cases are redundant. With this proposal, package maintainers will only
have to cope with two changelogs: git and bodhi notes.

== Scope ==
* Proposal owners:
** Finish the work on the %autorel macro to support some of the use
cases not implemented currently
** Adjust the packaging so rpmautospec does not live in a specific,
versionized python environment (and thus could be use to bootstrap
python)
** Adjust rpmdev-bumpspec (used by releng for mass-rebuilds) so it
ignores spec files using %autorel/%autochangelog
** Adjust the mass-rebuild script so it only adds a suitable git
commit log message for spec files using %autorel/%autochangelog
** Adjust fedpkg to skip the NEVR check when using rpmautospec
** Add dependency on rpmautospec on redhat-rpm-config

* Other developers:
** Opt-in for those who want to use it
** Test changes in staging
** Provide feedback

* Release engineering: [https://pagure.io/releng/issues #Releng issue number]

* Policies and guidelines: Packaging guidelines should be adjust to
explain how rpmautospec works and can be used. Documentation at:
https://docs.pagure.org/fedora-infra.rpmautospec/ should provide a
good basis for this

* Trademark approval: N/A (not needed for this Change)
* Alignment with Objectives:

== Upgrade/compatibility impact ==

Not applicable


== How To Test ==

In staging:

* Use `stg-koji` instead of `koji`
* Use `fedpkg-stage` instead of `fedpkg` (``dnf install fedpkg-stage``)
* Run `kinit <accountname&gt;(a)STG.FEDORAPROJECT.ORG`
* Test with rawhide as rpmautospec is only available there in staging
at this point
* Install `rpmautospec-rpm-macros` to build packages locally.
* Edit the spec file to use either macro (or both)
* (Optional) Run `rpmautospec` on the git repository of your choice to
see what it does (you may have to run `rpmautospec tag-package` the
first time you run it).
* Build the package
* Check its release or changelog according to the macro set

In production:
* Edit the spec file to use either macro (or both)
* (Optional) Run `rpmautospec` on the git repository of your choice to
see what it does (you may have to run `rpmautospec tag-package` the
first time you run it).
* Build the package
* Check its release or changelog according to the macro set

Issues can be reported at: https://pagure.io/fedora-infra/rpmautospec/issues

Note that using this approach has some peculiarities, we've documented
the ones we've encountered at:
https://docs.pagure.org/fedora-infra.rpmautospec/peculiarities.html

== User Experience ==

Packagers not opting-in should not be affected in any way by this change.

Packagers opting-in should be able to stop worrying (or worry much
less) about the content of the Release/%changelog fields in their spec
file.

== Dependencies ==
None

== Contingency Plan ==

* Contingency mechanism: rpmautospec is not deployed in koji
* Contingency deadline: N/A (this isn't tied to a Fedora release)
* Blocks release? No


== Documentation ==
All the upstream documentations can be
https://docs.pagure.org/fedora-infra.rpmautospec/

== Release Notes ==
N/A, this change will not affect end-users (except maybe for some
changes in the content of the rpm changelog).

-- 
Ben Cotton
He / Him / His
Senior Program Manager, Fedora & CentOS Stream
Red Hat
TZ=America/Indiana/Indianapolis

[Miro Hrončok]

On 12. 02. 21 17:20, Ben Cotton wrote:
>  ** Adjust the packaging so rpmautospec does not live in a specific,
>  versionized python environment (and thus could be use to bootstrap
>  python) 
Thank you!

Don't hesitate to let me know once there is testable proof of concept for this, 
so I can try to break it again :D

-- 
Miro Hrončok
--
Phone: [phone]
IRC: mhroncok

[Pierre-Yves Chibon]

On Fri, Feb 12, 2021 at 06:08:31PM +0100, Miro Hrončok wrote:
>  On 12. 02. 21 17:20, Ben Cotton wrote:
>  > ** Adjust the packaging so rpmautospec does not live in a specific,
>  > versionized python environment (and thus could be use to bootstrap
>  > python)
>  
>  Thank you!
>  
>  Don't hesitate to let me know once there is testable proof of concept for
>  this, so I can try to break it again :D 
Sure thing, with pleasure! :D


Pierre

[Neal Gompa]

On Fri, Feb 12, 2021 at 2:42 PM Pierre-Yves Chibon <pingou(a)pingoured.fr&gt; wrote:
>  On Fri, Feb 12, 2021 at 06:08:31PM +0100, Miro Hrončok wrote:
>  > On 12. 02. 21 17:20, Ben Cotton wrote:
>  > > ** Adjust the packaging so rpmautospec does not live in a specific,
>  > > versionized python environment (and thus could be use to bootstrap
>  > > python)
>  >
>  > Thank you!
>  >
>  > Don't hesitate to let me know once there is testable proof of concept for
>  > this, so I can try to break it again :D
>
>  Sure thing, with pleasure! :D
>  
Don't forget to let me in on the fun! :)



-- 
真実はいつも一つ！/ Always, there's only one truth!

[Dan Čermák]

Ben Cotton <bcotton(a)redhat.com&gt; writes:

>  https://fedoraproject.org/wiki/Changes/rpmautospec
>
>  == Summary ==
>  The goal of this change is to deploy in production the
>  [https://docs.pagure.org/fedora-infra.rpmautospec/ rpmautospec]
>  project.
>
>  With it, the content of the `Release` and `%changelog` fields in spec
>  files can be auto-generated, either locally or in the builder using
>  the information present in the git repo (in the form of git tags).
>
>
>  Note: This proposal is about changing the way the `Release` and
>  `%changelog` sections of the '''spec files''' are filled, not
> about
>  removing them from the SRPM or binary RPM. 
Thanks for submitting this! I'm looking forward to automating package
updates even more :)


Cheers,

Dan

[Kevin Fenzi]

So, I am a bit leary of this change for the same reason that I was for
the last few changes like this. Namely, we are building out own layer on
top of rpm to make our lives easier, when we probibly should try and
just improve rpm to do this for everyone.

I realize that rpm is slow to move and this might not be practical, but
I sure wish we would try. 

As a strawman, what about this that upstream might take: 

* keep the same macros you propose (don't care what colour the bikeshed
is)
* src.rpms grow a (optional) releases (or whatever) dir. 
* under this (optional) dir we have a subdir for each version. 
* under that (optional) dir we have a subdir for each release.
* under that (optional) dir we have files for changelogs.

So, the last few ansible builds would have:

ansible-2.9.17-1.fc34:

ansible/releases/2.9.17/1/update-to-2.9.17
ansible/releases/2.9.16/2/conflict-with-ansible-base
ansible/releases/2.9.16/2/Adjust-collections-generator
ansible/releases/2.9.16/1/update-to-2.9.16
...

When building, rpm would sort the releases dir, then the version subdir,
then concat the changelog fragments into a changelog. This way
everything is in the src.rpm.

When doing a PR, you would make a new version subdir and 
put a changelog fragment in it and commit it with any of
your other changes. 

Probibly some problems with that, but I thought I would throw it out
there from my sleep deprived mind. :) 

Anyhow, I just am a bit leary of us building up more and more around
rpm, and not just fixing rpm.

kevin

[Miro Hrončok]

On 14. 02. 21 19:45, Kevin Fenzi wrote:
>  So, I am a bit leary of this change for the same reason that I was
> for
>  the last few changes like this. Namely, we are building out own layer on
>  top of rpm to make our lives easier, when we probibly should try and
>  just improve rpm to do this for everyone.
>  
>  I realize that rpm is slow to move and this might not be practical, but
>  I sure wish we would try.
>  
>  As a strawman, what about this that upstream might take:
>  
>  * keep the same macros you propose (don't care what colour the bikeshed
>  is)
>  * src.rpms grow a (optional) releases (or whatever) dir.
>  * under this (optional) dir we have a subdir for each version.
>  * under that (optional) dir we have a subdir for each release.
>  * under that (optional) dir we have files for changelogs.
>  
>  So, the last few ansible builds would have:
>  
>  ansible-2.9.17-1.fc34:
>  
>  ansible/releases/2.9.17/1/update-to-2.9.17
>  ansible/releases/2.9.16/2/conflict-with-ansible-base
>  ansible/releases/2.9.16/2/Adjust-collections-generator
>  ansible/releases/2.9.16/1/update-to-2.9.16
>  ...
>  
>  When building, rpm would sort the releases dir, then the version subdir,
>  then concat the changelog fragments into a changelog. This way
>  everything is in the src.rpm.
>  
>  When doing a PR, you would make a new version subdir and
>  put a changelog fragment in it and commit it with any of
>  your other changes.
>  
>  Probibly some problems with that, but I thought I would throw it out
>  there from my sleep deprived mind. :) 
That still means a particular change needs to "bump" the release number to a 
specific value and hence it generates conflicts/mismatch when:

  - two or more PRs are opened at the same time
  - individual commits are cherry-picked between branches

Hence, not sure if it actually solves some problem.

-- 
Miro Hrončok
--
Phone: [phone]
IRC: mhroncok

[Kevin Fenzi]

On Sun, Feb 14, 2021 at 09:39:01PM +0100, Miro Hrončok wrote:
>  
>  That still means a particular change needs to "bump" the release number to a
>  specific value and hence it generates conflicts/mismatch when:
>  
>   - two or more PRs are opened at the same time
>   - individual commits are cherry-picked between branches
>  
>  Hence, not sure if it actually solves some problem. 
See, I knew there would be problems. ;) 

Anyhow, I think it's a noble goal if we can have src.rpms still provide
all the information needed to contibute (and I know, the shipped rpms
from this will match the build, but not whats in git). 

kevin

[clime]

On Sun, 14 Feb 2021 at 22:16, Kevin Fenzi <kevin(a)scrye.com&gt; wrote:

>  On Sun, Feb 14, 2021 at 09:39:01PM +0100, Miro Hrončok wrote:
>  >
>  > That still means a particular change needs to "bump" the release number
>  to a
>  > specific value and hence it generates conflicts/mismatch when:
>  >
>  >  - two or more PRs are opened at the same time
>  >  - individual commits are cherry-picked between branches
>  >
>  > Hence, not sure if it actually solves some problem.
>
>  See, I knew there would be problems. ;)
>
>  Anyhow, I think it's a noble goal if we can have src.rpms still provide
>  all the information needed to contibute (and I know, the shipped rpms
>  from this will match the build, but not whats in git). 

Conflicts are not the problem as the release would be derived from the dir
structure and in this structure the content is separate.

There is a more subtle problem that you should include branchname in the
structure because release 2 in one branch can mean something else than
release 2 in another (this could lead to git conflicts but that's rather
coincidental consequence).

Just wanted to say that your proposal would solve that particular problem
as well.


>  kevin
>  _______________________________________________
>  devel mailing list -- devel(a)lists.fedoraproject.org
>  To unsubscribe send an email to devel-leave(a)lists.fedoraproject.org
>  Fedora Code of Conduct:
>  https://docs.fedoraproject.org/en-US/project/code-of-conduct/
>  List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
>  List Archives:
>  https://lists.fedoraproject.org/archives/list/[e-mail]
>  Do not reply to spam on the list, report it:
>  https://pagure.io/fedora-infrastructure
>

[Vít Ondruch]

Dne 14. 02. 21 v 19:45 Kevin Fenzi napsal(a):
>  So, I am a bit leary of this change for the same reason that I was
> for
>  the last few changes like this. Namely, we are building out own layer on
>  top of rpm to make our lives easier, when we probibly should try and
>  just improve rpm to do this for everyone.
>
>  I realize that rpm is slow to move and this might not be practical, but
>  I sure wish we would try.
>
>  As a strawman, what about this that upstream might take:
>
>  * keep the same macros you propose (don't care what colour the bikeshed
>  is)
>  * src.rpms grow a (optional) releases (or whatever) dir.
>  * under this (optional) dir we have a subdir for each version.
>  * under that (optional) dir we have a subdir for each release.
>  * under that (optional) dir we have files for changelogs. 

Just FTR, when I was proposing some macros to include Changelog into 
RPM, I was told that one of benefits of RPM and spec file is that it is 
all together in one file:

https://github.com/rpm-software-management/rpm/issues/393#issuecomment-36...

If you look at the problem from this angle, then I can't see how this 
would be acceptable to RPM upstream :/


Vít


>  So, the last few ansible builds would have:
>
>  ansible-2.9.17-1.fc34:
>
>  ansible/releases/2.9.17/1/update-to-2.9.17
>  ansible/releases/2.9.16/2/conflict-with-ansible-base
>  ansible/releases/2.9.16/2/Adjust-collections-generator
>  ansible/releases/2.9.16/1/update-to-2.9.16
>  ...
>
>  When building, rpm would sort the releases dir, then the version subdir,
>  then concat the changelog fragments into a changelog. This way
>  everything is in the src.rpm.
>
>  When doing a PR, you would make a new version subdir and
>  put a changelog fragment in it and commit it with any of
>  your other changes.
>
>  Probibly some problems with that, but I thought I would throw it out
>  there from my sleep deprived mind. :)
>
>  Anyhow, I just am a bit leary of us building up more and more around
>  rpm, and not just fixing rpm.
>
>  kevin
>
>  _______________________________________________
>  devel mailing list -- devel(a)lists.fedoraproject.org
>  To unsubscribe send an email to devel-leave(a)lists.fedoraproject.org
>  Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
>  List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
>  List Archives:
> https://lists.fedoraproject.org/archives/list/[e-mail]
>  Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure

[Martin Kolman]

On Mon, 2021-02-15 at 11:50 +0100, Vít Ondruch wrote:
>  Dne 14. 02. 21 v 19:45 Kevin Fenzi napsal(a):
>  > So, I am a bit leary of this change for the same reason that I was for
>  > the last few changes like this. Namely, we are building out own layer on
>  > top of rpm to make our lives easier, when we probibly should try and
>  > just improve rpm to do this for everyone.
>  > 
>  > I realize that rpm is slow to move and this might not be practical, but
>  > I sure wish we would try. 
>  > 
>  > As a strawman, what about this that upstream might take: 
>  > 
>  > * keep the same macros you propose (don't care what colour the bikeshed
>  > is)
>  > * src.rpms grow a (optional) releases (or whatever) dir. 
>  > * under this (optional) dir we have a subdir for each version. 
>  > * under that (optional) dir we have a subdir for each release.
>  > * under that (optional) dir we have files for changelogs.
>  
>  
>  Just FTR, when I was proposing some macros to include Changelog into RPM, I was told that
> one of benefits of RPM and
>  spec file is that it is all together in one file:
>  
>  https://github.com/rpm-software-management/rpm/issues/393#issuecomment-36... 
I
rather agree with that - I did some Debian packaging in the past and the bunch of random
folder trees containing
almost random files that stands in for the spec file in Deb packages was pretty scary for
me as a packaging beginner.

Having everything in a single file really helps to see what a package is about without
having to dig through a million
files, not to mention makes comparing package revision easy.

I guess having changelog in one separate file is still bearable in this context, but would
I hope it does not start a
splitting trend where other parts of the spec file get randomly split out as well.


>  
>  If you look at the problem from this angle, then I can't see how this would be
> acceptable to RPM upstream :/
>  
>  
>  
>  Vít
>  
>  
>  
>  > So, the last few ansible builds would have:
>  > 
>  > ansible-2.9.17-1.fc34:
>  > 
>  > ansible/releases/2.9.17/1/update-to-2.9.17
>  > ansible/releases/2.9.16/2/conflict-with-ansible-base
>  > ansible/releases/2.9.16/2/Adjust-collections-generator
>  > ansible/releases/2.9.16/1/update-to-2.9.16
>  > ...
>  > 
>  > When building, rpm would sort the releases dir, then the version subdir,
>  > then concat the changelog fragments into a changelog. This way
>  > everything is in the src.rpm.
>  > 
>  > When doing a PR, you would make a new version subdir and 
>  > put a changelog fragment in it and commit it with any of
>  > your other changes. 
>  > 
>  > Probibly some problems with that, but I thought I would throw it out
>  > there from my sleep deprived mind. :) 
>  > 
>  > Anyhow, I just am a bit leary of us building up more and more around
>  > rpm, and not just fixing rpm.
>  > 
>  > kevin
>  > 
>  > 
>  > _______________________________________________
>  > devel mailing list -- devel(a)lists.fedoraproject.org
>  > To unsubscribe send an email to devel-leave(a)lists.fedoraproject.org
>  > Fedora Code of Conduct:
> https://docs.fedoraproject.org/en-US/project/code-of-conduct/
>  > List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
>  > List Archives:
> https://lists.fedoraproject.org/archives/list/[e-mail]
>  > Do not reply to spam on the list, report it:
> https://pagure.io/fedora-infrastructure
>  
>  _______________________________________________
>  devel mailing list -- devel(a)lists.fedoraproject.org
>  To unsubscribe send an email to devel-leave(a)lists.fedoraproject.org
>  Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
>  List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
>  List Archives:
> https://lists.fedoraproject.org/archives/list/[e-mail]
>  Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure

[Vitaly Zaitsev]

On 12.02.2021 17:20, Ben Cotton wrote:
>  With it, the content of the `Release` and `%changelog` fields in
> spec
>  files can be auto-generated, either locally or in the builder using
>  the information present in the git repo (in the form of git tags). 
Great. Finally. +1 for this change. Thanks.

-- 
Sincerely,
   Vitaly Zaitsev (vitaly(a)easycoding.org)

[Fabio Valentini]

On Fri, Feb 12, 2021 at 5:20 PM Ben Cotton <bcotton(a)redhat.com&gt; wrote:
>  https://fedoraproject.org/wiki/Changes/rpmautospec
>
>  == Summary ==
>  The goal of this change is to deploy in production the
>  [https://docs.pagure.org/fedora-infra.rpmautospec/ rpmautospec]
>  project.
>
>  With it, the content of the `Release` and `%changelog` fields in spec
>  files can be auto-generated, either locally or in the builder using
>  the information present in the git repo (in the form of git tags).
>
>
>  Note: This proposal is about changing the way the `Release` and
>  `%changelog` sections of the '''spec files''' are filled, not
> about
>  removing them from the SRPM or binary RPM.
>
>  == Owner ==
>  * Name: [[User:pingou| Pierre-Yves Chibon]]
>  * Email: pingou - at - pingoured.fr
>  * Name: [[User:nphilipp| Nils Philippsen]]
>  * Email: nphilipp - at - redhat.com
>
>
>  == Detailed Description ==
>
>  rpmautospec offers packagers who want to use it the possibility of
>  replacing the content of the `Release` of their spec file by `Release:
>     %autorel` and/or replace the content of the `%changelog` section of
>  their spec file by:
>    %changelog
>    %autochangelog
>
>  Both `%autorel` and `%autochangelog` are RPM macros that will be
>  expanded or replaced when the SRPM is built on the build system by
>  their corresponding values according to rpmautospec.
>
>  An overview of how rpmautospec works can be found at:
>  https://docs.pagure.org/fedora-infra.rpmautospec/principle.html.
>  We will describe below how each macro works.
>
>  === The %autorel macro ===
>
>  To determine the next release information, rpmautospec relies on the
>  build history of the package.
>  This information is extracted from the buildsystem when running as a
>  koji plugin and from git tags when running outside of the buildsystem.
>
>  Using the build history of the package (ie a list of NEVRs) as well as
>  the information provided by the packager in the spec file, rpmautospec
>  then computes the next best release number for the package.
>
>  Once defined, it prepends a suitably defined %autorel macro to the top
>  of the spec file, freezing the computed value of the release number
>  and thus allowing reproducible builds of the SRPM.
>
>  The [https://docs.pagure.org/fedora-infra.rpmautospec/autorel.html
>  documentation of the autorel macro] describes how packagers can use it
>  to provide extra information. 
I wonder why you're not relying solely on git history for both local
builds and in koji?

When this stuff was last discussed last year, I tried to implement a
simple algorithm for automatic Release values as well (because why
not), and I even had a working proof-of-concept implementation (in
python + pygit2) for the Release number that even worked for
non-linear git histories, something like (python-esque-pseudocode):

def release_num(commit, last_version) -> int:
    if version_at(commit) != last_version:
        return 0
    else:
        return max(release_num(parent, last_version) for parent of commit)) +1

(With "version_at" being a function that returns the value of Version
tag present in the specified commit in the git history.)

With an algorithm like this, you would not need to rely on either
build history or git tags, but use *only* commit history - which
should make this more reliable and produce always the same results
locally *and* in koji (because it uses the same underlying data).

Fabio

[Miro Hrončok]

On 16. 02. 21 14:48, Fabio Valentini wrote:
>       if version_at(commit) != last_version:
>           return 0 
Should this be "return 1"?

-- 
Miro Hrončok
--
Phone: [phone]
IRC: mhroncok

[Fabio Valentini]

On Tue, Feb 16, 2021 at 3:01 PM Miro Hrončok <mhroncok(a)redhat.com&gt; wrote:
>  On 16. 02. 21 14:48, Fabio Valentini wrote:
>  >      if version_at(commit) != last_version:
>  >          return 0
>
>  Should this be "return 1"? 
No, 0 is correct. If the version does not match, this is the last
commit *before* a version update.
The "max(parents) + 1" then sets the Release to 1 for the commit that
actually changed the version :)

>  To prevent accidental divergence between the git history and the
> build system.
>  That's why this information is only used in the koji plugin, locally (ie: via
>  the rpmautospec CLI) it only relies on the git tags. 
So ... you want to *prevent* divergence by *introducing* divergence? I
do not follow ...

>  Using the number of commits can give weird results with merge commits
> and even
>  though the upgrade path is not really an issue anymore, we preferred to try
>  preserving it. So rpmautospec should minimize the risk of broken upgrade path. 
That is possible. However, I assume it's possible to determine whether
a given commit is a merge commit? Then it would be easy to skip over
those in the computation.

And while I agree that upgrade path should be clean, fixing those
corner cases by making the whole process brittle and possibly
inconsistent does not sound like a good idea to me. And with system
upgrade tools defaulting to `distro-sync` mode now, this corner cases
are even less of a problem.

Fabio

[Neal Gompa]

On Tue, Feb 16, 2021 at 9:39 AM Fabio Valentini <decathorpe(a)gmail.com&gt; wrote:
>  On Tue, Feb 16, 2021 at 3:01 PM Miro Hrončok <mhroncok(a)redhat.com&gt; wrote:
>  >
>  > On 16. 02. 21 14:48, Fabio Valentini wrote:
>  > >      if version_at(commit) != last_version:
>  > >          return 0
>  >
>  > Should this be "return 1"?
>
>  No, 0 is correct. If the version does not match, this is the last
>  commit *before* a version update.
>  The "max(parents) + 1" then sets the Release to 1 for the commit that
>  actually changed the version :)
>
>  > To prevent accidental divergence between the git history and the build system.
>  > That's why this information is only used in the koji plugin, locally (ie: via
>  > the rpmautospec CLI) it only relies on the git tags.
>
>  So ... you want to *prevent* divergence by *introducing* divergence? I
>  do not follow ...
>
>  > Using the number of commits can give weird results with merge commits and even
>  > though the upgrade path is not really an issue anymore, we preferred to try
>  > preserving it. So rpmautospec should minimize the risk of broken upgrade path.
>
>  That is possible. However, I assume it's possible to determine whether
>  a given commit is a merge commit? Then it would be easy to skip over
>  those in the computation.
>
>  And while I agree that upgrade path should be clean, fixing those
>  corner cases by making the whole process brittle and possibly
>  inconsistent does not sound like a good idea to me. And with system
>  upgrade tools defaulting to `distro-sync` mode now, this corner cases
>  are even less of a problem.
>  
It's a problem in the normal upgrade case, since people release
post-release updates this way too.



-- 
真実はいつも一つ！/ Always, there's only one truth!

[Pierre-Yves Chibon]

On Tue, Feb 16, 2021 at 03:38:35PM +0100, Fabio Valentini wrote:
>  On Tue, Feb 16, 2021 at 3:01 PM Miro Hrončok
> <mhroncok(a)redhat.com&gt; wrote:
>  >
>  > On 16. 02. 21 14:48, Fabio Valentini wrote:
>  > >      if version_at(commit) != last_version:
>  > >          return 0
>  >
>  > Should this be "return 1"?
>  
>  No, 0 is correct. If the version does not match, this is the last
>  commit *before* a version update.
>  The "max(parents) + 1" then sets the Release to 1 for the commit that
>  actually changed the version :)
>  
>  > To prevent accidental divergence between the git history and the build system.
>  > That's why this information is only used in the koji plugin, locally (ie: via
>  > the rpmautospec CLI) it only relies on the git tags.
>  
>  So ... you want to *prevent* divergence by *introducing* divergence? I
>  do not follow ... 
The build information is used to check if all the builds made in koji exists as
tags. If they don't, then they are added, thus resolving the divergence.
If they do, git tags are used, just like they are used locally.



Pierre

[Fabio Valentini]

On Tue, Feb 16, 2021 at 8:20 PM Pierre-Yves Chibon <pingou(a)pingoured.fr&gt; wrote:
>  On Tue, Feb 16, 2021 at 03:38:35PM +0100, Fabio Valentini wrote:
>  > On Tue, Feb 16, 2021 at 3:01 PM Miro Hrončok <mhroncok(a)redhat.com&gt; wrote:
>  > >
>  > > On 16. 02. 21 14:48, Fabio Valentini wrote:
>  > > >      if version_at(commit) != last_version:
>  > > >          return 0
>  > >
>  > > Should this be "return 1"?
>  >
>  > No, 0 is correct. If the version does not match, this is the last
>  > commit *before* a version update.
>  > The "max(parents) + 1" then sets the Release to 1 for the commit that
>  > actually changed the version :)
>  >
>  > > To prevent accidental divergence between the git history and the build system.
>  > > That's why this information is only used in the koji plugin, locally (ie:
> via
>  > > the rpmautospec CLI) it only relies on the git tags.
>  >
>  > So ... you want to *prevent* divergence by *introducing* divergence? I
>  > do not follow ...
>
>  The build information is used to check if all the builds made in koji exists as
>  tags. If they don't, then they are added, thus resolving the divergence.
>  If they do, git tags are used, just like they are used locally. 
There's another issue that I see with using both git tags and koji
build history:
How do users get those tags into their local repository clones, if
they are created by koji after successful builds?
Will we need to "git pull" after every successful koji build so we get
consistent results between local checkout and infra build?

Side note: This amended algorithm should always produce incrementing
release numbers, even across branches:

def release_num(commit, last_version) -> int:
    if version_at(commit) != last_version:
        return 0
    else:
        distance = max(release_num(parent, last_version) for parent of commit))
        if is_merge_commit(commit):
            return distance
        else:
            return distance + 1

That should solve both the upgrade path issue and the data source
problem. No need to look at either git tags or koji build history :)

Fabio

[Pierre-Yves Chibon]

On Tue, Mar 02, 2021 at 09:30:41PM +0100, Fabio Valentini wrote:
>  On Tue, Feb 16, 2021 at 8:20 PM Pierre-Yves Chibon
> <pingou(a)pingoured.fr&gt; wrote:
>  >
>  > On Tue, Feb 16, 2021 at 03:38:35PM +0100, Fabio Valentini wrote:
>  > > On Tue, Feb 16, 2021 at 3:01 PM Miro Hrončok <mhroncok(a)redhat.com&gt;
> wrote:
>  > > >
>  > > > On 16. 02. 21 14:48, Fabio Valentini wrote:
>  > > > >      if version_at(commit) != last_version:
>  > > > >          return 0
>  > > >
>  > > > Should this be "return 1"?
>  > >
>  > > No, 0 is correct. If the version does not match, this is the last
>  > > commit *before* a version update.
>  > > The "max(parents) + 1" then sets the Release to 1 for the commit
> that
>  > > actually changed the version :)
>  > >
>  > > > To prevent accidental divergence between the git history and the build
> system.
>  > > > That's why this information is only used in the koji plugin, locally
> (ie: via
>  > > > the rpmautospec CLI) it only relies on the git tags.
>  > >
>  > > So ... you want to *prevent* divergence by *introducing* divergence? I
>  > > do not follow ...
>  >
>  > The build information is used to check if all the builds made in koji exists as
>  > tags. If they don't, then they are added, thus resolving the divergence.
>  > If they do, git tags are used, just like they are used locally.
>  
>  There's another issue that I see with using both git tags and koji
>  build history:
>  How do users get those tags into their local repository clones, if
>  they are created by koji after successful builds? 
The first time it can be done via the rpmautospect CLI command.

>  Will we need to "git pull" after every successful koji
> build so we get
>  consistent results between local checkout and infra build? 
After that, git pull/fetch is indeed the easiest method.

>  Side note: This amended algorithm should always produce incrementing
>  release numbers, even across branches:
>  
>  def release_num(commit, last_version) -> int:
>      if version_at(commit) != last_version:
>          return 0
>      else:
>          distance = max(release_num(parent, last_version) for parent of commit))
>          if is_merge_commit(commit):
>              return distance
>          else:
>              return distance + 1
>  
>  That should solve both the upgrade path issue and the data source
>  problem. No need to look at either git tags or koji build history :) 
The devil is in the details: pre-release, snapinfo, minorbump aren't really
covered by distance being just an integer bumped.

I know we considered the "number of commits since the last version bump" when
we
looked into this. I honestly do not remember precisely why we didn't go with it.

Maybe Nils or Adam remember it?


Pierre

[Miro Hrončok]

On 02. 03. 21 22:05, Pierre-Yves Chibon wrote:
>  The devil is in the details: pre-release, snapinfo, minorbump
> aren't really
>  covered by distance being just an integer bumped. 
I don't see this covered in the current method either. Or was it in the 
meantime? Anyway:

- pre-release should go into version (~)
- snapinfo should go into version (~ or ^)

The only real problem I see here is minorbump. We could have something like: 
%{autorel -m}. That means, micobump since this was added here. But I guess it is 
ugly.

>  I know we considered the "number of commits since the last
> version bump" when we
>  looked into this. I honestly do not remember precisely why we didn't go with it.
IIRC it was because you considered building several rebuilds with different 
releases from the same commit a goal (while I'd rather require an empty commit 
that explains the reason for the rebuild).

-- 
Miro Hrončok
--
Phone: [phone]
IRC: mhroncok

[Pierre-Yves Chibon]

On Wed, Mar 03, 2021 at 01:07:44AM +0100, Miro Hrončok wrote:
>  On 02. 03. 21 22:05, Pierre-Yves Chibon wrote:
>  > The devil is in the details: pre-release, snapinfo, minorbump aren't really
>  > covered by distance being just an integer bumped.
>  
>  I don't see this covered in the current method either. Or was it in the
>  meantime? Anyway: 
It is planned and part of the work to be done with this proposal:
https://docs.pagure.org/fedora-infra.rpmautospec/autorel.html#using-autorel
 
>  - pre-release should go into version (~)
>  - snapinfo should go into version (~ or ^)
>  
>  The only real problem I see here is minorbump. We could have something like:
>  %{autorel -m}. That means, micobump since this was added here. But I guess
>  it is ugly.
>  
>  > I know we considered the "number of commits since the last version bump"
> when we
>  > looked into this. I honestly do not remember precisely why we didn't go with
> it.
>  
>  IIRC it was because you considered building several rebuilds with different
>  releases from the same commit a goal (while I'd rather require an empty
>  commit that explains the reason for the rebuild). 
Indeed, that approach would not allow rebuilding a commit without adding a new
(potentially empty) commit.
One of the idea being that not having to add commits would make it easier to do
auto-rebuild of dependency chains.


Pierre

[Fabio Valentini]

On Wed, Mar 3, 2021 at 9:20 AM Pierre-Yves Chibon <pingou(a)pingoured.fr&gt; wrote:
>  On Wed, Mar 03, 2021 at 01:07:44AM +0100, Miro Hrončok wrote:
>  > On 02. 03. 21 22:05, Pierre-Yves Chibon wrote:
>  > > The devil is in the details: pre-release, snapinfo, minorbump aren't
> really
>  > > covered by distance being just an integer bumped.
>  >
>  > I don't see this covered in the current method either. Or was it in the
>  > meantime? Anyway:
>
>  It is planned and part of the work to be done with this proposal:
>  https://docs.pagure.org/fedora-infra.rpmautospec/autorel.html#using-autorel 
I'm not sure that this is a good idea. Why not just make the %autorel
macro return the "incrementing number" part of Release?
That would work for all the use cases we have in Fedora today, without
the need for implementing it internally in the %autorel macro.

>  > - pre-release should go into version (~)
>  > - snapinfo should go into version (~ or ^)
>  >
>  > The only real problem I see here is minorbump. We could have something like:
>  > %{autorel -m}. That means, micobump since this was added here. But I guess
>  > it is ugly.
>  >
>  > > I know we considered the "number of commits since the last version
> bump" when we
>  > > looked into this. I honestly do not remember precisely why we didn't go
> with it.
>  >
>  > IIRC it was because you considered building several rebuilds with different
>  > releases from the same commit a goal (while I'd rather require an empty
>  > commit that explains the reason for the rebuild).
>
>  Indeed, that approach would not allow rebuilding a commit without adding a new
>  (potentially empty) commit.
>  One of the idea being that not having to add commits would make it easier to do
>  auto-rebuild of dependency chains. 
That sounds like an anti-feature to me. Having that information (even
if it's an empty commit with the "rebuilt for libfoo soname change")
is valuable IMO.
Also, where would the changelog entry for subsequent rebuilds for the
same commit come from?

Fabio

[Miro Hrončok]

On 16. 02. 21 14:48, Fabio Valentini wrote:
>  I wonder why you're not relying solely on git history for both
> local
>  builds and in koji? 
I'd also appreciate if the only source of truth would be git history.

-- 
Miro Hrončok
--
Phone: [phone]
IRC: mhroncok

[Pierre-Yves Chibon]

On Tue, Feb 16, 2021 at 02:48:23PM +0100, Fabio Valentini wrote:
>  On Fri, Feb 12, 2021 at 5:20 PM Ben Cotton <bcotton(a)redhat.com&gt;
> wrote:
>  >
>  > https://fedoraproject.org/wiki/Changes/rpmautospec
>  >
>  > == Summary ==
>  > The goal of this change is to deploy in production the
>  > [https://docs.pagure.org/fedora-infra.rpmautospec/ rpmautospec]
>  > project.
>  >
>  > With it, the content of the `Release` and `%changelog` fields in spec
>  > files can be auto-generated, either locally or in the builder using
>  > the information present in the git repo (in the form of git tags).
>  >
>  >
>  > Note: This proposal is about changing the way the `Release` and
>  > `%changelog` sections of the '''spec files''' are filled,
> not about
>  > removing them from the SRPM or binary RPM.
>  >
>  > == Owner ==
>  > * Name: [[User:pingou| Pierre-Yves Chibon]]
>  > * Email: pingou - at - pingoured.fr
>  > * Name: [[User:nphilipp| Nils Philippsen]]
>  > * Email: nphilipp - at - redhat.com
>  >
>  >
>  > == Detailed Description ==
>  >
>  > rpmautospec offers packagers who want to use it the possibility of
>  > replacing the content of the `Release` of their spec file by `Release:
>  >    %autorel` and/or replace the content of the `%changelog` section of
>  > their spec file by:
>  >   %changelog
>  >   %autochangelog
>  >
>  > Both `%autorel` and `%autochangelog` are RPM macros that will be
>  > expanded or replaced when the SRPM is built on the build system by
>  > their corresponding values according to rpmautospec.
>  >
>  > An overview of how rpmautospec works can be found at:
>  > https://docs.pagure.org/fedora-infra.rpmautospec/principle.html.
>  > We will describe below how each macro works.
>  >
>  > === The %autorel macro ===
>  >
>  > To determine the next release information, rpmautospec relies on the
>  > build history of the package.
>  > This information is extracted from the buildsystem when running as a
>  > koji plugin and from git tags when running outside of the buildsystem.
>  >
>  > Using the build history of the package (ie a list of NEVRs) as well as
>  > the information provided by the packager in the spec file, rpmautospec
>  > then computes the next best release number for the package.
>  >
>  > Once defined, it prepends a suitably defined %autorel macro to the top
>  > of the spec file, freezing the computed value of the release number
>  > and thus allowing reproducible builds of the SRPM.
>  >
>  > The [https://docs.pagure.org/fedora-infra.rpmautospec/autorel.html
>  > documentation of the autorel macro] describes how packagers can use it
>  > to provide extra information.
>  
>  I wonder why you're not relying solely on git history for both local
>  builds and in koji? 
To prevent accidental divergence between the git history and the build system.
That's why this information is only used in the koji plugin, locally (ie: via
the rpmautospec CLI) it only relies on the git tags.

Using the number of commits can give weird results with merge commits and even
though the upgrade path is not really an issue anymore, we preferred to try
preserving it. So rpmautospec should minimize the risk of broken upgrade path.


Pierre