PR#826: document the use of forge macros for issue 719 - packaging-committee

packaging-committee

#826 document the use of forge macros for issue 719

Merged 4 years ago by james. Opened 5 years ago by nim.

nim/packaging-committee forgedoc into master

add more empty lines

Nicolas Mailhot • 5 years ago

f64369e

forge: tweak release comments

Nicolas Mailhot • 5 years ago

6403a73

Forge: sync with redhat-rpm-config PR47

Nicolas Mailhot • 5 years ago

d31d886

document the use of forge macros

Nicolas Mailhot • 5 years ago

522142a

guidelines/modules/ROOT/examples/spectemplate-forge-branch.spec

file added

+49

		`@@ -0,0 +1,49 @@`
		`+ # This template shows how to package a branch state using the forgemeta macro.`
		`+ #`
		`+ # BRANCH STATES ARE NOT REPRODUCIBLE AND SHOULD NEVER BE SHARED WITH OTHERS.`
		`+ #`
		`+ # For more explanations on forgemeta, read the forgemeta-release template.`
		`+ #`
		`+ # The project url on the forge`
		`+ %global forgeurl`
		`+ #`
		`+ # The branch being packaged`
		`+ %global branch`
		`+ #`
		`+ # – use the “-i” flag to display the variables forgemeta reads and sets`
		`+ # – use the “-v” flag if you want verbose processing`
		`+ # – remove “-i” and “-v” before commit`
		`+ %forgemeta`
		`+`
		`+ # The following lines use variables computed by forgemeta as default values.`
		`+ # You can replace them with manual definitions.`
		`+ # forgemeta will prepend branch information to dist. Release ordering is`
		`+ # controlled by the packager with x%{?dist}/0.x%{?dist} number chains.`
		`+ Name:`
		`+ Version:`
		`+ Release: 1%{?dist}`
		`+ Summary:`
		`+ URL: %{forgeurl}`
		`+ Source: %{forgesource}`
		`+ %description`
		`+`
		`+`
		`+ %prep`
		`+ # – an autosetup wrapper, forgeautosetup, is also provided`
		`+ # – use the “-v” flag if you want verbose processing; remove it before commit`
		`+ # – call forgesetup/autosetup directly if you do not like the result`
		`+ %forgesetup`
		`+ #`
		`+ # After this point the archive extraction is done. forgemeta is no longer used.`
		`+`
		`+ %build`
		`+`
		`+ %install`
		`+`
		`+ %check`
		`+`
		`+ %files`
		`+ %license`
		`+ %doc`
		`+`
		`+ %changelog`

guidelines/modules/ROOT/examples/spectemplate-forge-commit.spec

file added

+46

		`@@ -0,0 +1,46 @@`
		`+ # This template shows how to package a commit state using the forgemeta macro.`
		`+ # For more explanations on forgemeta, read the forgemeta-release template.`
		`+ #`
		`+ # The project url on the forge`
		`+ %global forgeurl`
		`+ #`
		`+ # The commit being packaged (when using git, a full hash)`
		`+ %global commit`
		`+ #`
		`+ # – use the “-i” flag to display the variables forgemeta reads and sets`
		`+ # – use the “-v” flag if you want verbose processing`
		`+ # – remove “-i” and “-v” before commit`
		`+ %forgemeta`
		`+`
		`+ # The following lines use variables computed by forgemeta as default values.`
		`+ # You can replace them with manual definitions.`
		`+ # forgemeta will prepend commit information to dist. Release ordering is`
		`+ # controlled by the packager with x%{?dist}/0.x%{?dist} numbers chains.`
		`+ Name:`
		`+ Version:`
		`+ Release: 1%{?dist}`
		`+ Summary:`
		`+ URL: %{forgeurl}`
		`+ Source: %{forgesource}`
		`+ %description`
		`+`
		`+`
		`+ %prep`
		`+ # – an autosetup wrapper, forgeautosetup, is also provided`
		`+ # – use the “-v” flag if you want verbose processing; remove it before commit`
		`+ # – call forgesetup/autosetup directly if you do not like the result`
		`+ %forgesetup`
		`+ #`
		`+ # After this point the archive extraction is done. forgemeta is no longer used.`
		`+`
		`+ %build`
		`+`
		`+ %install`
		`+`
		`+ %check`
		`+`
		`+ %files`
		`+ %license`
		`+ %doc`
		`+`
		`+ %changelog`

guidelines/modules/ROOT/examples/spectemplate-forge-multi.spec

file added

+81

		`@@ -0,0 +1,81 @@`
		`+ # This template shows how to package multiple source archives using the`
		`+ # forgemeta macro.`
		`+ #`
		`+ # PACKAGING MULTIPLE PROJECT ARCHIVES IN A SINGLE SPEC IS DISCOURAGED. IT OFTEN`
		`+ # REQUIRES ERROR-INDUCING VERSIONNING DECISIONS. In rpm, versioning is not an`
		`+ # administrative decoration. Versioning controls the upgrade logic. If upstream`
		`+ # deemed necessary to release a project through multiple archives, mirror its`
		`+ # decision using multiple spec files. That will be safer, simpler, and`
		`+ # ultimately less work.`
		`+ #`
		`+ # For more explanations on forgemeta, read the forgemeta-release template.`
		`+ #`
		`+ # Start by declaring the characteristics of each source using a number-suffixed`
		`+ # variable block:`
		`+ # – no suffix and zero suffix blocks are aliases`
		`+ # – they are “special” and identify the main source archive`
		`+ # — they use Version: to identify a release`
		`+ # – other blocks use a version<number> variable for the same need`
		`+ # – the syntax is otherwise identical to single archive mode; see the various`
		`+ # forge templates for examples.`
		`+ #`
		`+ # Main archive. In this example we package a full release`
		`+ %global forgeurl0`
		`+ Version:`
		`+`
		`+ # Second archive.`
		`+ %global forgeurl1`
		`+ %global version1`
		`+`
		`+ # Third archive. This time a tag. Continue as necessary.`
		`+ %global forgeurl2`
		`+ %global tag2`
		`+`
		`+ # – use the “-a” flag to process all the source archives in a single pass`
		`+ # – use “-z <number>” to process only the declaration block suffixed with`
		`+ # <number>`
		`+ # — without “-a” of “-z <number>” only the main archive is processed`
		`+ # – use the “-i” flag to display the variables forgemeta reads and sets`
		`+ # – use the “-v” flag if you want verbose processing`
		`+ # – remove “-i” and “-v” before commit`
		`+ %forgemeta -a`
		`+`
		`+ # The following lines use variables computed by forgemeta as default values.`
		`+ # You can replace them with manual definitions.`
		`+ # Release ordering is controlled by the packager with x%{?dist}/0.x%{?dist}`
		`+ # numbers chains.`
		`+ Name:`
		`+ Release: 1%{?dist}`
		`+ Summary:`
		`+ URL: %{forgeurl0}`
		`+ # https://github.com/rpm-software-management/rpm/issues/539`
		`+ Source0: %{forgesource0}`
		`+ Source1: %{forgesource1}`
		`+ Source2: %{forgesource2}`
		`+ %description`
		`+`
		`+`
		`+ %prep`
		`+ # – use the “-a” flag to process all the source archives in a single pass`
		`+ # – use “-z <number>” to process only the declaration block suffixed with`
		`+ # <number>`
		`+ # — without “-a” of “-z <number>” only the main archive is processed`
		`+ # – an autosetup wrapper, forgeautosetup, is also provided`
		`+ # – forgeautosetup does not understand “-a”`
		`+ # – use the “-v” flag if you want verbose processing; remove it before commit`
		`+ # – call forgesetup/autosetup directly if you do not like the result`
		`+ %forgesetup -a`
		`+ #`
		`+ # After this point the archive extraction is done. forgemeta is no longer used.`
		`+`
		`+ %build`
		`+`
		`+ %install`
		`+`
		`+ %check`
		`+`
		`+ %files`
		`+ %license`
		`+ %doc`
		`+`
		`+ %changelog`

guidelines/modules/ROOT/examples/spectemplate-forge-release.spec

file added

+58

		`@@ -0,0 +1,58 @@`
		`+ # This template shows how to package a commit state using the forgemeta macro.`
		`+ # That should be the default target use case.`
		`+ # Other less common use cases are documented in separate templates.`
		`+ #`
		`+ # The project url on the forge, for example`
		`+ # https://gitlab.gnome.org/GNOME/gtk/`
		`+ %global forgeurl`
		`+ #`
		`+ # Packaging a release requires setting Version before calling forgemeta.`
		`+ # Because most forges follow the git model, and git made no provision for`
		`+ # release objects, forgemeta will try to guess the customary way to write`
		`+ # release tags on the selected forge.`
		`+ # If it guesses wrong use the forge-tag template instead of this one.`
		`+ Version:`
		`+ #`
		`+ # forgemeta converts the suppplied rpm variables to variables that can be used`
		`+ # in the spec file. Most of those can be overriden before or after the`
		`+ # forgemeta call.`
		`+ # – use the “-i” flag to display the variables forgemeta reads and sets`
		`+ # – use the “-v” flag if you want verbose processing`
		`+ # – remove “-i” and “-v” before commit`
		`+ %forgemeta`
		`+`
		`+ # The following lines use variables computed by forgemeta as default values.`
		`+ # You can replace them with manual definitions. For example, replace forgeurl`
		`+ # with the project homepage if it exists separately from the repository URL.`
		`+ # Only replace the variables when it adds value to the spec file and you`
		`+ # understand the consequences. Release ordering is controlled by the packager`
		`+ # with x%{?dist}/0.x%{?dist} number chains.`
		`+ Name:`
		`+ Release: 1%{?dist}`
		`+ Summary:`
		`+ URL: %{forgeurl}`
		`+ Source: %{forgesource}`
		`+ %description`
		`+`
		`+`
		`+ %prep`
		`+ # forgesetup calls setup with the correct flags for archives downloaded from`
		`+ # the selected forge. A forgeautosetup autosetup wrapper is also provided. If`
		`+ # you disagree with the computed setup/autosetup flags, just call`
		`+ # setup/autosetup directly.`
		`+ # – use the “-v” flag if you want verbose processing`
		`+ %forgesetup`
		`+ #`
		`+ # After this point the archive extraction is done. forgemeta is no longer used.`
		`+`
		`+ %build`
		`+`
		`+ %install`
		`+`
		`+ %check`
		`+`
		`+ %files`
		`+ %license`
		`+ %doc`
		`+`
		`+ %changelog`

guidelines/modules/ROOT/examples/spectemplate-forge-tag.spec

file added

+46

		`@@ -0,0 +1,46 @@`
		`+ # This template shows how to package a tag state using the forgemeta macro.`
		`+ # For more explanations on forgemeta, read the forgemeta-release template.`
		`+ #`
		`+ # The project url on the forge`
		`+ %global forgeurl`
		`+ #`
		`+ # The tag being packaged`
		`+ %global tag`
		`+ #`
		`+ # – use the “-i” flag to display the variables forgemeta reads and sets`
		`+ # – use the “-v” flag if you want verbose processing`
		`+ # – remove “-i” and “-v” before commit`
		`+ %forgemeta`
		`+`
		`+ # The following lines use variables computed by forgemeta as default values.`
		`+ # You can replace them with manual definitions.`
		`+ # forgemeta will prepend tag information to dist. Release ordering is`
		`+ # controlled by the packager with x%{?dist}/0.x%{?dist} numbers chains.`
		`+ Name:`
		`+ Version:`
		`+ Release: 1%{?dist}`
		`+ Summary:`
		`+ URL: %{forgeurl}`
		`+ Source: %{forgesource}`
		`+ %description`
		`+`
		`+`
		`+ %prep`
		`+ # – an autosetup wrapper, forgeautosetup, is also provided`
		`+ # – use the “-v” flag if you want verbose processing; remove it before commit`
		`+ # – call forgesetup/autosetup directly if you do not like the result`
		`+ %forgesetup`
		`+ #`
		`+ # After this point the archive extraction is done. forgemeta is no longer used.`
		`+`
		`+ %build`
		`+`
		`+ %install`
		`+`
		`+ %check`
		`+`
		`+ %files`
		`+ %license`
		`+ %doc`
		`+`
		`+ %changelog`

guidelines/modules/ROOT/pages/SourceURL.adoc

file modified

+50

		`@@ -14,6 +14,56 @@`

		`Here are some specific examples:`

		`+ == Using Forges (Hosted Revision Control)`
		`+`
		+ Any software publishing website, permitting the download of source archives via normalized URLs, that can be deduced from a project root URL and version, commit, tag, scm, extension… values is a _“forge”_ that can be supported by the `redhat-rpm-config` `+%{forgemeta}+` macro. Common Forge examples are _GitLab_ and _GitHub_. `+%{forgemeta}+` centralizes and abstracts our knowledge about those forges, so packagers do not have to handle download quirks manually.
		`+`
		+ `+%{forgemeta}+` makes it easy to switch from release to tag to commit source archives. Using `+%{forgemeta}+`, forge download URLs or guideline changes are propagated to spec files without manual refactoring.
		`+`
		+ When those changes result in a different naming or structure of the source archive, the source file needs to be uploaded to the build system before rebuilding existing spec files. That is the main drawback of using `+%{forgemeta}+`.
		`+`
		+ The following examples are taken from the `redhat-rpm-templates` package, that provides their latest version.
ignatenkobrain commented 4 years ago there is no such package in Fedora.
		`+`
		`+ === Release Example`
		`+`
		`+ .spectemplate-forge-release.spec`
		`+ [source]`
		`+ ----`
		`+ include::{examplesdir}/spectemplate-forge-release.spec[]`
		`+ ----`
		`+`
		`+ === Tag Example`
		`+`
		`+ .spectemplate-forge-tag.spec`
		`+ [source]`
		`+ ----`
		`+ include::{examplesdir}/spectemplate-forge-tag.spec[]`
		`+ ----`
		`+`
		`+ === Commit Example`
		`+`
		`+ .spectemplate-forge-commit.spec`
		`+ [source]`
		`+ ----`
		`+ include::{examplesdir}/spectemplate-forge-commit.spec[]`
		`+ ----`
		`+`
		`+ === Branch Example`
		`+`
		`+ .spectemplate-forge-branch.spec`
		`+ [source]`
		`+ ----`
		`+ include::{examplesdir}/spectemplate-forge-branch.spec[]`
		`+ ----`
		`+`
		`+ === Multiple Sources Example`
		`+`
		`+ .spectemplate-forge-multi.spec`
		`+ [source]`
		`+ ----`
		`+ include::{examplesdir}/spectemplate-forge-multi.spec[]`
		`+ ----`
		`+`
		`== Using Revision Control`

		`In some cases you may want to pull sources from upstream's revision control system because there have been many changes since the last release and you think that a tarball that you generate from there will more accurately show how the package relates to upstream's development. Here's how you can use a comment to show where the source came from:`

nim commented 5 years ago

https://pagure.io/packaging-committee/issue/719

1 new commit added

Forge: sync with redhat-rpm-config PR47

5 years ago

1 new commit added

forge: tweak release comments

5 years ago

ignatenkobrain commented on line 39 of guidelines/modules/ROOT/pages/SourceURL.adoc 5 years ago

missing empty line

ignatenkobrain commented on line 7 of guidelines/modules/ROOT/pages/SourceURL.adoc 5 years ago

please reformat according to semantic line breaks

1 new commit added

add more empty lines

5 years ago

decathorpe commented 5 years ago

After looking through the changes, I don't see anything obvious that needs changing, and I expect that you have already tested using the templates for packages to verify that they actually produce correct packages.

That said, the current draft adds only a bare minimum of text to the Guidelines, and only includes the current version of the example templates (which contain annotations explaining the usage) on top of that.

The current structure probably makes the documentation hard to parse for readers who just want to look up usage information for one specific macro (and its arguments and which environment it expects), because they have to look through the templates for that information.

It would be nice to have at least a small overview in textual form for the Guidelines themselves. For example, which macros are available (%forgemeta, %forgeurl, %forgesource, ...) and the macros they expect to be set when they are called (exactly one of version, tag, commit, branch, andforgeurl, etc.).

For example, adding a paragraph like this ↓ would be easy, and would probably give most users almost all the information they need, and could refer them to the templates for more details:

Available macros for processing forge sources:
- %forgemeta: processes forgeurl, tag, Version, commit, branch macros (with and without number prefix), and computes the resulting %forgesource values
- %forgesource (with and without number prefix): provides Source URLs, use for the Source tag (with the appropriate number)
- %forgesetup: wrapper around the %autosetup macro, calling it with appropriate arguments

Maybe also mentioning regularly used arguments, like -a or -z N for %forgemeta and %forgesetup, and additional information on how the expected input macros have to be named for multiple sources (especially %versionN wasn't obvious to me).

I also expect this addition to blow up the length of the SourceURL page due to the included templates. Maybe that section could be extracted to a separate page, with a link to it below the explanatory text in the main SourceURL page.

nim commented 5 years ago

First, thanks a lot for the review.

After looking through the changes, I don't see anything obvious that needs changing, and I expect that you have already tested using the templates for packages to verify that they actually produce correct packages.

The code corresponding to those templates has been merged and in use in rawhide since mid-october:
https://koji.fedoraproject.org/koji/buildinfo?buildID=1154732

The documentation could not be written before this merge, since the code review may request variable and macro renamings (the usual way to do those would be to have a single code + doc PR, with matching changes, but that's not how Fedora organized itself).

So the basic functionality is already in use and known to work. It will still need code fixes:

when totally broken forges like bitbucket or pagure fix themselves, or
when upstreams get really creative about their release tagging, like here: https://src.fedoraproject.org/fork/nim/rpms/redhat-rpm-config/c/9ad828b68da0065f27d9b01a1dd700a1d71fb28f

(I so which someone could beat some sense in git upstream so the x.y.z way of releasing pretty much everyone needs and uses is supported natively by git without room for human syntax “creativity”).

But that does not require any template update. That's just fixes, not changes to the overall design.

What could change templates is new features like:
https://pagure.io/go-rpm-macros/issue/3

But, not sure if I will ever bother to code it (or push it to Fedora if I code it for my own needs), given how my previous rounds of contributions are been received.

That said, the current draft adds only a bare minimum of text to the Guidelines, and only includes the current version of the example templates (which contain annotations explaining the usage) on top of that.

It's likely to be the only documentation that will ever be since the redhat-rpm-config maintainers do not want their package to document itself, so future code changes won't have any matching documentation unless someone finds the energy to bother with separate guidelines PR-ing.

The current structure probably makes the documentation hard to parse for readers who just want to look up usage information for one specific macro (and its arguments and which environment it expects), because they have to look through the templates for that information.

I'm not sure how useful “looking up usage information for one specific macro” would be. It all performs basically a single function. The splitting in multiple calls is a side-effect of the way rpm splits source and setup in different places of the spec, nothing more.

Yes the design is modular so you can replace parts of the processing with your own thing, but to do it safely without hitting all kinds of breakage you need to be aware of the high-level view. Otherwise you will change things, thinking it only matters at the point you do changes, and hit side effects in the next section of the spec.

Individual macro usage is already documented in the macro files themselves
https://src.fedoraproject.org/rpms/redhat-rpm-config/blob/master/f/macros.forge

That makes sure the documentation does not stray too far from what the code does. But, it quickly gets overwhelming without the high level recommended usage template view.

If you feel strongly about it a can complete this PR with individual macro documentation but I fear that will be the same feel-good not needed documentation bloat that made people ignore https://fedoraproject.org/wiki/Forge-hosted_projects_packaging_automation and forced a complete documentation rewrite. Plus, it's almost sure to lose sync with the code quickly, since it won't be possible to document behavior changes in the commit that makes those changes.

I also expect this addition to blow up the length of the SourceURL page due to the included templates. Maybe that section could be extracted to a separate page, with a link to it below the explanatory text in the main SourceURL page.

I've no opinion one way or another, that's really getting in general guidelines editorial land, not the documentation of a specific feature.

Metadata Update from @decathorpe:
- Request assigned

5 years ago

ignatenkobrain commented 4 years ago

@nim is this still actual version?

nim commented 4 years ago

@ignatenkobrain it's still actual version (as in https://src.fedoraproject.org/rpms/redhat-rpm-config/pull-request/51), the user interface has seen no changes since last summer, the core has been extended to support pagure but without changes packager-side

ignatenkobrain commented on line 13 of guidelines/modules/ROOT/pages/SourceURL.adoc 4 years ago

there is no such package in Fedora.

ignatenkobrain commented 4 years ago

So I have looked at this once more and I think more appropriate way would be to document macros instead of dumping huge specfiles.

@nim Can you please restructure this?

nim commented 4 years ago

@ignatenkobrain the macros are incomprehensible without a template, because you have several lines of declarations interacting with one another.

That’s why the documentation is provided in template form, to give users example of various useful use patterns. And provided in actual templates they can copy in their own spec files, not prettified asciidoc text that can not be cut and pasted and will become obsolete as soon as the docs teams changes its rich text format (has happened to wiki doc)

A call-by call doc would be the kind of unreadable auto-generated doc you find in programming languages (worse, because rpm syntax does not provide structure helpers). But, if you want thins kind of thing, you can just copy the inlined comments in the templates, or the inline comments in the macro code.