#942 Recommend storing changelog entries in separate file.
Opened 6 months ago by vondruch. Modified 5 days ago
vondruch/packaging-committee changelog  into  master

@@ -593,6 +593,19 @@ 

  

  _Every time_ you make changes, that is, whenever you increment the E-V-R of a package, add a changelog entry. This is important not only to have an idea about the history of a package, but also to enable users, fellow packagers, and QA people to easily spot the changes that you make.

  

+ The changelog should be stored in separate `changelog` file referenced among sources:

+ 

+ ....

+ Source1: changelog

+ ....

+ 

+ Please note, that the source number is arbitrary. This file is later included into the spec file via `+%external_changelog+` macro:

+ 

+ ....

+ %changelog

+ %external_changelog %{S:1}

+ ....

+ 

  If a particular change is related to a Bugzilla bug, include the bug ID in the changelog entry for easy reference, e.g.

  

  ....

We are discussion over and over how to get rid of changelogs [1], I see moving changelogs out of .spec file into separate changelog file as very simple first step. This is 1) backward compatible, because everything keeps working 2) the changelog entries might be mass moved from .spef files by simple script. The future automation could create the changelog somehow, if it is not presented in dist-git.

If we're going to do this, I'd just rather adopt some variant of what SUSE or Mer do. The scripts exist already and it'd be relatively easy to plug into our source RPM prep scripts.

Do we really make this a SHOULD at this point?

If we're going to do this, I'd just rather adopt some variant of what SUSE

I don't think SUSE is using %include and that is hackish IMO, because it is not obvious how the changelog gets into the SRPM (neither we have tools to do that if I am not mistaken). What I want, for the start (until we have the automation), is to have the changelog file always present in dist git. This is what we can do right away just by changing policy.

Of course if upstream RPM changed the %changelog keyword/macro to include the changelog file by default, or even included the changelog without being specified, that would be different.

Do we really make this a SHOULD at this point?

Well, I started writing this guideline with recommended, but I don't think we are using such wording in current guidelines. Anyway, SHOULD is not MUST and since I proposed this change to make the transition to generated changelogs easier, then why not use SHOULD to really move somewhere.

My idea was that we would opt-in for this and later make it a should, but should for this first step makes sense as well.

Note that you do not want something based on %include. Using %include turns spec from a standalone entity into something that requires external files in certain paths, causing lots of unnecessary pain and grief and endless stream of bugs that we cannot fix.

Simply cat'ing an external changelog file to the end of spec might seem ugly and primitive, but its far nicer from a user perspective.

FWIW: while clearly it's possible to split changelogs out of specs without further support from rpm, we (talking about rpm team) wouldn't mind adding something to rpm to help this goal. That is, if that something makes sense for rpm in general. There have been some suggestions to "do something" to this end, but none of them have had the overall case thought out.

Note that you do not want something based on %include. Using %include turns spec from a standalone entity into something that requires external files in certain paths, causing lots of unnecessary pain and grief and endless stream of bugs that we cannot fix.

I don't really see difference between %include and Source or Patch or what not. Maybe the guideline could be extended to provide the changelog as a Source, would it change anything?

Simply cat'ing an external changelog file to the end of spec might seem ugly and primitive, but its far nicer from a user perspective.

But that needs to be done on infrastructure side, you have to modify something to make this working. This guideline does not need anything, but aims to enable the infrastructure changes.

Also, it is quite ugly to modify some file which is maintained by SCM (where the external changelog maintained by some infra tools would go into .gitignore file).

FWIW: while clearly it's possible to split changelogs out of specs without further support from rpm, we (talking about rpm team) wouldn't mind adding something to rpm to help this goal. That is, if that something makes sense for rpm in general. There have been some suggestions to "do something" to this end, but none of them have had the overall case thought out.

So is there a way to improve the %changelog situation by e.g.

  • %changelog accepting file option similar to %files section
  • adding new Changelog keyword instead of %changelog section?

Maybe the guideline could be extended to provide the changelog as a Source, would it change anything?

Actually, thinking about it, it has to be included as the Source. I'll modify the proposal.

Right, it kinda has to be a Source to be included in src.rpm, and spec parse knows to skip missing sources and patches to avoid a basic parse to succeed without them (which is the thing use of %include breaks). So having a source-like Changelog: tag might be just the thing. I'll need to think about that some.

(which is the thing use of %include breaks)

But the assumption is that the included file is always available in the repository (ok, we could rather speak about some working folder, but it is not important distinction for this case), either maintained by maintainer or by some automation, so there should be no issue. If there is an issue, I believe that the error message is clear enough to identify the missing file, which in the worst case can be created by simple touch command.

Yes, but sooner than later those specs will find their way out of the repo. People expect to toss, pass and parse spec files around without having to also pass additional sources. That's one of the pros of spec format compared to eg dpkg, and using %include breaks that. People will be unhappy.

People expect to toss, pass and parse spec files around without having to also pass additional sources. That's one of the pros of spec format compared to eg dpkg, and using %include breaks that. People will be unhappy.

Ok, right, I see. But then there does not appear to beother option then Changelog keyword working fine without the actual changelog file. Unfortunately, this will take ages to propagate to every distribution.

If I should choose between Changelog widely accepted in years and %include available right now with some minor issues, then I choose the %include and hope that the Changelog is not too far away

Can we define (on Packaging Guidelines level) that %include is only allowed with %{SOURCEX} and later alter it to "... or with %{CHANGELOG}"?
And the %include macro can be defined on redhat-rpm-config level using Lua.

Can we define (on Packaging Guidelines level) that %include is only allowed with %{SOURCEX}

Do I understand correctly that you say that the current guidelines do not mention the proper usage of %include together with Source, which should probably be corrected in separate FPC ticket? On top of that, we would not need to mention it in the Changelog guidelines section?

and later alter it to "... or with %{CHANGELOG}"?

Now I realize that my idea was that the Changelog directive would somehow include the changelog file automatically. But seeing your comment, I realize that the Source and Patch just include the files into SRPM and they need to be used later somehow, in this case it would be %include in %changelog section. So the Changelog directive would not be as useful as I thought originally :see_no_evil:

Can we define (on Packaging Guidelines level) that %include is only allowed with %{SOURCEX} and later alter it to "... or with %{CHANGELOG}"?
And the %include macro can be defined on redhat-rpm-config level using Lua.

Please no redefining RPM primitives. That will definitely break things.

The actual work to make the buildsystem automatically do changelog transformation and concatenation is pretty easy. It can be added to fedpkg-minimal as it fetches sources, or we can do something else in the buildSRPMfromSCM task.

Lets be careful about assuming what rpm could/would implement here.

The hypothetical Changelog: tag in spec would obviously have (much) more side-effects than a Source: does, it would include the specified file as if it was the %changelog section, and skip if missing on parse-only mode (which the %include directive cannot really do).

But getting such changes properly designed and propagated takes time and patience, not sure it's really worth that.

My point really just is: think thrice and then thrice again before planning anything around %include. Other distros have implemented external changelog without using it, in fact nobody has ever widely used %include for anything. There are reasons for that.

Please no redefining RPM primitives.

What? RPM already can handle '%include'? I have just learned something new today. Can we start first with documenting what %include actually does (or should do)?

The only documentation I found is the code itself:
https://github.com/rpm-software-management/rpm/blob/master/build/parseSpec.c#L527

and one answer on Stackoverflow:
https://stackoverflow.com/questions/6136056/can-an-rpm-spec-file-include-other-files#8919211

I found nothing on rpm.org website.

Hi

%changelog accepting file option similar to %files section

That would be the nicest most natural and probably most reliable option. Just add a -f switch to %changelog

And then what actually needs to be decided is the level of packaging where we absolutely want file-backed changelogs to appear:

  1. if the only thing that matters practically is that changelogs exist in the deployment packages, the -f can point to anything in sources, source archives, or even generated %build files
  2. if we need them in the final srpm too, they can point to anything that exists at the end of %prep (like dynamic buildrequires). That would make using files in the upstream git possible
  3. the only reason to restrict the to separate %{SOURCEX}is if we absolutely need them in the first-pass SRPM. Do we? I’m not sure the slight win is worth the level of pain

Alternatively, I’d argue for the %changelog -f switch to be non-blocking from an rpm build process, and only generate a warning if the file target is absent.

May be we should get back to the content of this PR and what is currently available in RPM :innocent:

May be we should get back to the content of this PR and what is currently available in RPM 😇

I would not dismiss the feedback of rpm maintainers. If they feel the PR is unsafe, and are ready to commit implementing something better, more power to them and let’s do it, even if it takes a release to finish.

A short term win is not worth papering over rpm limits. rpm and Fedora are too interdependent, workaround-ing those limits instead of fixing them is a death spiral.

(of course if rpm maintainers have nothing better to propose in a reasonable time horizon the show must go on)

The problem with "%changelog -f <file>" is that it requires having that one-liner boilerplate in every single spec. Not end of the world but everybody hates redundant boilerplate.

I was envisioning something along the lines of a new ChangelogFile: spec tag which gets populated from a macro if one is set (so the boilerplate is avoided) and overridable in the spec, with logic to allow parse to ignore a missing changelog-file. As long as the tag is populated from a macro, basic backwards compatibility is preserved in that the spec can be still parsed by older rpm versions, only changelog would be missing.

I don't know when we'd have time to implement such a thing (although how hard can it be...), but then whatever rpm implementation doesn't mean you couldn't go on with splitting the changelog out of specs. How that gets pulled into the final is a mere implementation detail really.

My bottom line is simply: whatever you do, do not use the existing %include directive. It's bad karma and will break dnf builddep foo.spec for everything using it, and thus ultimately the entire distro. See https://bugzilla.redhat.com/show_bug.cgi?id=1560291 for a nice short summary about the problem.

My bottom line is simply: whatever you do, do not use the existing %include directive. It's bad karma and will break dnf builddep foo.spec for everything using it, and thus ultimately the entire distro. See https://bugzilla.redhat.com/show_bug.cgi?id=1560291 for a nice short summary about the problem.

In understand you concerns, but that is the only thing we have ATM. If we are not going to use %include, then we soon end up with

Simply cat'ing an external changelog file to the end of spec

and all the discussions about ChangelogFile or %changelog -f <file> will be moot.

My point is that there is simple transition path from %include to all the proposals above, because whatever the infrastructure tooling is going to be, it will always touch just external file and its inclusion into package is RPM task. There is no transition patch from cat`ing. It will be completely out of RPM hands.

Or we can stick with the current status quo.

The problem with "%changelog -f <file>" is that it requires having that one-liner boilerplate in every single spec. Not end of the world but everybody hates redundant boilerplate.

Yes me the first as you well know :)

I was envisioning something along the lines of a new ChangelogFile: spec tag which gets populated from a macro if one is set (so the boilerplate is avoided) and overridable in the spec,

That only moves the required boilerplate around in the spec. And from an automation POW tags are really painful, anything that fits in a variable is a lot easier to manipulate (that’s why a lot of historical tags had to be mapped to variables).

If you want to do a backwards-compatible implementation without boilerplate, you’ll end up with something like:

  • a magic standard rpm variable that contains the changelog file name

    • distros like Fedora can preset it in redhat-rpm-config to whatever they like best
    • the spec writer can override it easily with another macro, a global or define
  • a %changelog verb that:

    • uses the content of the file specified by -f <file> if present and the file exists
    • fallbacks to the file defined in the magic variable if this file exists and -f <file> is not set
    • and completes with all the static changelog entries present after the %changelog line (much like %file -f mixes the file and static entries

That’s how my automation macros ended up behaving, due to organic pressure by their users, not because I started with this design.

That only moves the required boilerplate around in the spec.

Sigh. I'm not THAT dumb you know.
You misunderstood what I wrote, it'd obviously be autopopulated from a macro similar to how BugURL works. Somewhat like what you go on describing, but details differ for various reasons.

Anyway, I'm done here: I've now warned against using %include. If people don't listen that's not something I can help with.

That only moves the required boilerplate around in the spec.

Sigh. I'm not THAT dumb you know.

I know I know, believe me, I apologize for the unfortunate wording, I only wanted to clarify things others may have missed in your answer.

In understand you concerns, but that is the only thing we have ATM.

Nah. It's merely the worst possible tool for the job.

"%include foo" does nothing that "%(cat foo)" wouldn't do, only with a macro you're free to implement whatever logic around it. To avoid forking a shell, use %{lua:...} to implement.

If we are not going to use %include, then we soon end up with

Simply cat'ing an external changelog file to the end of spec

and all the discussions about ChangelogFile or %changelog -f <file> will be moot.

And there isn't necessarily anything wrong with that.
All the other distros achieve external changelog without using %include or any special support from rpm, so there isn't a whole lot incentive for rpm to implement anything special there. It'd have to make sense on many levels.

3 new commits added

  • Introduce `%external_changelog` macro.
  • Use simple `cat` instead of `%include`.
  • Define, that the 'changelog' files muste be listed among sources.
5 months ago

So I just pushed 3 additional commits. They can be squashed, I just want to keep them around to preserve my line of thoughts. Or if decided, not all have to be included.

So the first just specifies, that the changelog must be listed as a Source. Although I am not sure should there be some default value recommended. That could later help with some defaults.

The second replaces the %include by simple cat, as @pmatilai suggested.

The third introduces %external_changelog macro (the %changelog is already taken, therefore I like this name, because it is selfexplanatory IMHO). This would need addition PR against (if deemed suitable) redhat-rpm-config. The simplest implementation could be just cat again:

# Reads changelog entries from external file specified by parameter.
%define external_changelog() %(cat %{*})

The Lua based macro could look as follows:

# Reads changelog entries from external file specified by parameter.
%define external_changelog(s) %{lua:
local changelog_filename = rpm.expand("%{*}")
local file, msg = io.open(changelog_filename, "rb")
if not file then
  rpm.expand("%{warn:External changelog file '" .. changelog_filename .. "' was not found.}")
  return
end
local content = file:read("*all")
file:close()
print(content)
}

This macro could be even more complex, provide more error checking etc, but I decided to keep it simple for now.

Metadata Update from @churchyard:
- Pull-request tagged with: meeting

3 months ago

@pingou could you plese provide input on the most recent proposal in the latest comment above this one? How does thet interfere with rpmautospec? (See the macros in comment and the diff of this PR to get context.)

@vondruch releng uses rpmdev-bumpspec during the mass rebuilds (and we use it during the Python rebuilds, and many other groups or provenpackagers us it when rebuilding packages) -- how does that work here? I believe it would only bump the release but it would not be able to add a new changelog entry, because it doesn't really understand that changelog is in a different file. I consider that a blocker for recommending this globally.

@vondruch releng uses rpmdev-bumpspec during the mass rebuilds (and we use it during the Python rebuilds, and many other groups or provenpackagers us it when rebuilding packages) -- how does that work here?

It works okaish:

$ rpmdev-bumpspec ruby.spec -c blasdf
error: %changelog entries must start with *

$ echo $?
0

IOW while it unnecessarily complains, it bumps the release and adds the changelog entry into spec file.

Now you might argue, that it should update the changelog file. Sure, but unless we know how this works, then rpmdev-bumpsepec cannot be adjusted :hatched_chick:🥚 Nevertheless, since rpmdev-bumpspec newer worked reliably, i think this problem is easy to correct as other issues created by rpmdev-bumpspec.

@pingou could you plese provide input on the most recent proposal in the latest comment above this one? How does thet interfere with rpmautospec? (See the macros in comment and the diff of this PR to get context.)

rpmautospec relies on two macros %autorel and %autochangelog the later is actually more of a place holder that rpmautospec replaces with the content of the auto-generated changelog + the content of a changelog file if present.

So the macro in https://pagure.io/packaging-committee/pull-request/942#comment-108542 should not conflict with rpmautospec's, but thanks for checking :)

@vondruch releng uses rpmdev-bumpspec during the mass rebuilds (and we use it during the Python rebuilds, and many other groups or provenpackagers us it when rebuilding packages) -- how does that work here?

It works okaish:
$ rpmdev-bumpspec ruby.spec -c blasdf
error: %changelog entries must start with *

$ echo $?
0

IOW while it unnecessarily complains, it bumps the release and adds the changelog entry into spec file.
Now you might argue, that it should update the changelog file. Sure, but unless we know how this works, then rpmdev-bumpsepec cannot be adjusted 🐥🥚 Nevertheless, since rpmdev-bumpspec newer worked reliably, i think this problem is easy to correct as other issues created by rpmdev-bumpspec.

Please make it work. Even though rpmdev-bumpspec doesn't work in some cases, I use` it heavily.

@sgallagh Do you have any opinions on this?

So, I was working on another macro rework to pave the way for packaging go modules, and I realised it would change the way releases are bumped, and people would complain bitterly I broke their habits.

So, I decided to go the full Monthy, and see if i could make autobumping to work.

Turns out, it was quite easy to achieve compared to all the other stuff I was already doing (2 days work from idea to coding and testing).

Unfortunately, while it works fine at the rpm level (you can rpmbuild -ba, and it will autobump, and it will update the changelog with new build events) it blocks at the mock layer, because mock will filter away the srpm that includes the new changelog, and serve you the starting point srpm (I hope this can be fixed with mock maintainers, i could not believe how convenient autobumping was while I tested it).

Therefore I split the work in 2:

  1. detached changelog works, and fits well with the other parts I coded, so it is proposed as a part of
    https://fedoraproject.org/wiki/Changes/Patches_in_Forge_macros_-_Auto_macros_-_Detached_rpm_changelogs
    https://src.fedoraproject.org/rpms/redhat-rpm-config/pull-request/95

  2. auto-bumping that builds upon this base is split into
    https://fedoraproject.org/wiki/Changes/rpm_level_auto_release_and_changelog_bumping

to give some room to figure out with mock/koji maintainers what it blocking something that works at the rpm level.

I've no particular opinion at the implementation, I looked at @vondruch code and it is strikingly similar to mine. The main difference is that my implementation is evolvable to autobumping (since it is a partial backport of a full autobumping implementation), while @vondruch coded his one in isolation.

I'm pretty sure rpmautospec would not know how to deal with a spec that uses my macros but it would not have to, if the mock/copr/koji limitations were lifted the spec would autobump itself without external help.

I've no particular opinion at the implementation, I looked at @vondruch code and it is strikingly similar to mine. The main difference is that my implementation is evolvable to autobumping (since it is a partial backport of a full autobumping implementation), while @vondruch coded his one in isolation.

I think the difference is that you are proposing way more changes then just simple external changelog.

My proposal does not really care how the external changelog is created, but supposedly it is generated with help of infrastructure, because that is the only possibility IMO.