Bug 1523779 - [RFE] Forge-hosted projects packaging automation
Summary: [RFE] Forge-hosted projects packaging automation
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: redhat-rpm-config
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Florian Festi
QA Contact: Fedora Extras Quality Assurance
URL: https://fedoraproject.org/wiki/Forge-...
Whiteboard:
Keywords:
Depends On:
Blocks: 1526721
TreeView+ depends on / blocked
 
Reported: 2017-12-08 17:30 UTC by Nicolas Mailhot
Modified: 2018-01-23 16:54 UTC (History)
11 users (show)

(edit)
Clone Of:
(edit)
Last Closed: 2018-01-12 02:10:44 UTC


Attachments (Terms of Use)
Proposed macro file (4.55 KB, text/plain)
2017-12-08 17:30 UTC, Nicolas Mailhot
no flags Details
Proposed macro file (4.47 KB, text/plain)
2017-12-08 19:02 UTC, Nicolas Mailhot
no flags Details
Proposed macro file (4.51 KB, text/plain)
2017-12-08 19:05 UTC, Nicolas Mailhot
no flags Details
Proposed macro file (6.22 KB, text/plain)
2017-12-09 08:06 UTC, Nicolas Mailhot
no flags Details
Proposed macro file (6.48 KB, text/plain)
2017-12-09 12:26 UTC, Nicolas Mailhot
no flags Details
Proposed macro file (7.28 KB, text/plain)
2017-12-09 13:24 UTC, Nicolas Mailhot
no flags Details
Proposed macro file (7.61 KB, text/plain)
2017-12-10 08:47 UTC, Nicolas Mailhot
no flags Details
Proposed macro file (8.04 KB, text/plain)
2017-12-10 12:15 UTC, Nicolas Mailhot
no flags Details
Proposed macro file (8.04 KB, text/plain)
2017-12-10 12:17 UTC, Nicolas Mailhot
no flags Details
Proposed macro file (8.58 KB, text/plain)
2017-12-10 16:38 UTC, Nicolas Mailhot
no flags Details
Proposed macro file (8.58 KB, text/plain)
2017-12-10 16:40 UTC, Nicolas Mailhot
no flags Details
Proposed macro file (8.79 KB, text/plain)
2017-12-11 14:08 UTC, Nicolas Mailhot
no flags Details
Proposed macro file (8.58 KB, text/plain)
2017-12-11 15:46 UTC, Nicolas Mailhot
no flags Details
Proposed macro file (8.55 KB, text/plain)
2017-12-11 15:48 UTC, Nicolas Mailhot
no flags Details
Proposed macro file (7.40 KB, text/plain)
2017-12-14 09:02 UTC, Nicolas Mailhot
no flags Details
Proposed macro file (7.37 KB, text/plain)
2017-12-14 10:00 UTC, Nicolas Mailhot
no flags Details
Proposed macro file (7.22 KB, text/plain)
2017-12-14 10:35 UTC, Nicolas Mailhot
no flags Details
Proposed macro file (7.22 KB, text/plain)
2017-12-14 13:01 UTC, Nicolas Mailhot
no flags Details
Proposed macro file (7.71 KB, text/plain)
2017-12-14 13:02 UTC, Nicolas Mailhot
no flags Details
Proposed macro file (7.52 KB, text/plain)
2017-12-14 18:45 UTC, Nicolas Mailhot
no flags Details
Proposed macro file (7.62 KB, text/plain)
2017-12-14 19:21 UTC, Nicolas Mailhot
no flags Details
Proposed macro file (7.61 KB, text/plain)
2017-12-14 19:26 UTC, Nicolas Mailhot
no flags Details
Proposed macro file (9.62 KB, text/plain)
2017-12-16 12:12 UTC, Nicolas Mailhot
no flags Details
Proposed macro file (9.87 KB, text/plain)
2017-12-16 13:30 UTC, Nicolas Mailhot
no flags Details
Proposed macro file (9.98 KB, text/plain)
2017-12-17 20:12 UTC, Nicolas Mailhot
no flags Details
Proposed macro file (9.99 KB, text/plain)
2017-12-18 13:13 UTC, Nicolas Mailhot
no flags Details
Proposed macro file (11.22 KB, text/plain)
2018-01-23 10:58 UTC, Nicolas Mailhot
no flags Details

Comment 1 Nicolas Mailhot 2017-12-08 19:02 UTC
Created attachment 1365011 [details]
Proposed macro file

Same file without the duplicate archiveext fallback

Comment 2 Nicolas Mailhot 2017-12-08 19:05 UTC
Created attachment 1365014 [details]
Proposed macro file

Even better: move the archiveext fallback after known forge handling

Comment 3 Nicolas Mailhot 2017-12-09 08:06 UTC
Created attachment 1365185 [details]
Proposed macro file

Add GitLab support as requested by Matthew Miller

Generalize GitHub support to hosted GitHub

Comment 4 Nicolas Mailhot 2017-12-09 12:26 UTC
Created attachment 1365262 [details]
Proposed macro file

Relax URL constrains as requested by Zbigniew Jędrzejewski-Szmek

Comment 5 Nicolas Mailhot 2017-12-09 13:24 UTC
Created attachment 1365275 [details]
Proposed macro file

Allow overriding just archivename or archiveurl

Comment 6 Pavel Raiskup 2017-12-10 06:51:39 UTC
Seeing there's is a lot of work done -> wouldn't it be better to have
dedicated package for it, with man page, wiki page, etc., with separate
release cycle?

The development of redhat-rpm-config is too slow.  Unless something
dramatically changes (e.g. by moving the upstream code out from fedora-rawhide
and making fedora-rawhide a regular "downstream"), the redhat-rpm-config
is place for rather small and rather trivial general purpose macros.

Comment 7 Nicolas Mailhot 2017-12-10 08:45:06 UTC
I was rather hoping to avoid a separate package :
- there is only a single file to ship
- it would require build group modification Fedora and downstream
- the macro is totally generic and not linked to a particular Fedora SIG
- I expect people to look for it in fedora-rpm-macro
- the file is long because lua is extremely verbose. But if you look at the code it's extremely simple
- a man page? Why? What other macro requires a man page? There is a wiki page because that's part of FPC's process (and FPC will own the page if it's approved)

It's quite hard to find reliable long-term maintainers for single-file packages that do not change for years. The maintenance overhead would be terrible.

There is quite a lot of activity right now because the proposal is being reviewed by FPC, and everyone suggests his pet corner case/twist to handle, but after that phase and initial packager appropriation I fully expect the file to go dormant for years except when there is the need to add a new hosting service (or when one of the existing ones changes).

Is there anything specific that prevents a timely fedora-rpm-macro update when needed? The macro is contained in a separate file, there is no risk of side-effects for the rest of fedora-rpm-macro

Comment 8 Nicolas Mailhot 2017-12-10 08:46:14 UTC
But anyway I expect FPC to decide where is the best place to put this file, it works the same regardless of the package that ships it :)

Comment 9 Nicolas Mailhot 2017-12-10 08:47 UTC
Created attachment 1365500 [details]
Proposed macro file

Integrate %setup automation as suggested in https://github.com/rpm-software-management/rpm/issues/368

Comment 10 Nicolas Mailhot 2017-12-10 12:15 UTC
Created attachment 1365577 [details]
Proposed macro file

Integrate a few error messages to answer common questions raised during FPC review

Comment 11 Nicolas Mailhot 2017-12-10 12:17 UTC
Created attachment 1365580 [details]
Proposed macro file

Fix spelling in error message

Comment 12 Nicolas Mailhot 2017-12-10 16:38 UTC
Created attachment 1365640 [details]
Proposed macro file

Make %setup adjustment transparent

Comment 13 Nicolas Mailhot 2017-12-10 16:40 UTC
Created attachment 1365641 [details]
Proposed macro file

Fix spelling

Comment 14 Panu Matilainen 2017-12-11 10:13:24 UTC
As already mentioned on devel@, please don't override %setup, no good will come out of that. Just use something specific like %forgesetup instead. 

Also I'd rather see fedora-rpm-macros package killed (it's introduction was somewhat controversion to begin with), this belongs to redhat-rpm-config.

Comment 15 Nicolas Mailhot 2017-12-11 11:03:48 UTC
Reassigned, as requested by Panu

Comment 16 Nicolas Mailhot 2017-12-11 14:08 UTC
Created attachment 1366029 [details]
Proposed macro file

%setup → %forgesetup

+ native rpm errors and warnings

Comment 17 Panu Matilainen 2017-12-11 14:37:01 UTC
> print("%{warn:Defining %%forgesetup as %%setup %%{?setupargs}\\n}")

Leftover from an earlier version? I see no reason to warn about *that*.

Haven't looked closely yet, I'll try to do so sometime this week. One immediate concern is namespacing, this uses several very very common names like "tag" and "commit" which are easy source of conflicts, might not hurt to use forge-prefix on everything.

Comment 18 Nicolas Mailhot 2017-12-11 15:17:53 UTC
(In reply to Panu Matilainen from comment #17)
> > print("%{warn:Defining %%forgesetup as %%setup %%{?setupargs}\\n}")
> 
> Leftover from an earlier version? I see no reason to warn about *that*.

Yes leftover, sorry, much too complex for what it ended up being

> Haven't looked closely yet, I'll try to do so sometime this week. One
> immediate concern is namespacing, this uses several very very common names
> like "tag" and "commit"

That's intentional, they are generic and intended to be shared with non %forge macros. (and will be soonish)

Their content is exactly what they mean.

Comment 19 Nicolas Mailhot 2017-12-11 15:46 UTC
Created attachment 1366090 [details]
Proposed macro file

This one should be cleaner

Simplify leftover cruft

Comment 20 Nicolas Mailhot 2017-12-11 15:48 UTC
Created attachment 1366092 [details]
Proposed macro file

Attach the right file :(

Comment 21 Nicolas Mailhot 2017-12-14 09:02 UTC
Created attachment 1367893 [details]
Proposed macro file

Be more explicit to avoit packager surprises

Clean up and consolidation as requested by FPC

Comment 22 Nicolas Mailhot 2017-12-14 10:00 UTC
Created attachment 1367902 [details]
Proposed macro file

Simplify some more

Comment 23 Nicolas Mailhot 2017-12-14 10:35 UTC
Created attachment 1367908 [details]
Proposed macro file

Simplify some more

Comment 24 Nicolas Mailhot 2017-12-14 13:01 UTC
Created attachment 1367974 [details]
Proposed macro file

Cleanups

Add a forgemetatest macro to test if an url can be handled by forgemeta as requested during review (and to simplify Handover with Go macros)

Comment 25 Nicolas Mailhot 2017-12-14 13:02 UTC
Created attachment 1367975 [details]
Proposed macro file

Cleanups

Add a forgemetatest macro to test if an url can be handled by forgemeta as requested during review (and to simplify Handover with Go macros)

Comment 26 Nicolas Mailhot 2017-12-14 18:45 UTC
Created attachment 1368131 [details]
Proposed macro file

remove shortcommit remnants

Comment 27 Nicolas Mailhot 2017-12-14 19:21 UTC
Created attachment 1368135 [details]
Proposed macro file

Add a convenience %forgeautosetup macro

Comment 28 Nicolas Mailhot 2017-12-14 19:26 UTC
Created attachment 1368136 [details]
Proposed macro file

Remove a no longer necessary level of expand

Comment 29 Nicolas Mailhot 2017-12-16 12:12 UTC
Created attachment 1368794 [details]
Proposed macro file

Add some parameters to forgemeta to make FPC happy

Merge the function of forgemetacheck and forgemetatest in forgemeta

Debugging output on rawhide it a tad ugly due to the added end of lines which are necessary under EL7

# Map forge information to rpm metadata. This macro will compute default spec
# variable values.
#
# The following spec variables SHOULD be set before calling the macro:
#
#   forgeurl  the project url on the forge, strongly recommended;
#             alternatively, use -u <url>
#   Version   if applicable, set it with Version: <version>
#   tag       if applicable
#   commit    if applicable
#
# The macro will attempt to compute and set the following variables if they are
# not already set by the packager:
#
#   forgesource    an URL that can be used as SourceX: value
#   forgesetupargs the correct arguments to pass to %setup for this source
#                  used by %forgesetup and %forgeautosetup
#   archivename    the source archive filename, without extentions
#   archiveext     the source archive filename extensions, without leading dot
#   archiveurl     the url that can be used to download the source archive,
#                  without renaming
#   scm            the scm type, when packaging code snapthots: commits or tags
#
# If the macro is unable to parse your forgeurl value set at least archivename
# and archiveurl before calling it.
#
# Most of the computed variables are both overridable and optional. However,
# the macro WILL REDEFINE %{dist} when packaging a snapshot (commit or tag).
# The previous %{dist} value will be lost. Don’t call the macro if you don’t
# wish %{dist} to be changed.
#
# Optional parameters:
#   -u <url>  Ignore forgeurl even if it exists and use <url> instead. Note
#             that the macro will still end up setting <url> as the forgeurl
#             spec variable if it manages to parse it.
#   -s  Silently ignore problems in forgeurl, use it if it can be parsed,
#       ignore it otherwise.
#   -v  Be verbose and print every spec variable the macro sets.
#   -i  Print some info about the state of spec variables the macro may use or
#       set at the end of the processing.

Comment 30 Nicolas Mailhot 2017-12-16 13:30 UTC
Created attachment 1368797 [details]
Proposed macro file

Fix macro expansion case in debugging

Comment 31 Nicolas Mailhot 2017-12-17 20:12 UTC
Created attachment 1369154 [details]
Proposed macro file

Fix autosetup helper

Comment 32 Nicolas Mailhot 2017-12-18 13:13 UTC
Created attachment 1369487 [details]
Proposed macro file

Relay more parameters to %setup

Comment 33 Jason Tibbitts 2017-12-20 22:51:07 UTC
At this point I'm not fundamentally opposed to adding this, at least to the rawhide package.  However, once this goes in it gets much more difficult and time consuming to make changes.

Since we've now gone more than a day without an update, I'll ask: do you think this is ready?  And if so, for which releases?

Comment 34 Nicolas Mailhot 2017-12-21 08:18:21 UTC
Thanks for the approval

At this point the core of the macro should be rock-solid. It will only change when adding a new software hosting service or when one of the existing ones changes its URL interface.

Peripheral code saw quite a lot of churn to accommodate the changes requested during review, but this seems to have settled down and I'm not finding any more stray side effects in testing. So it should be pretty solid too. 

The one part I'm not 100% sure of is the %forgesetup and %forgeautosetup helpers, they were added quite late in the game and they jungle an awful ot of parameters. It is likely someone will find a parameter combo that does not quite work as it should later

So it's not going anywhere else without more real-world testing, I'd say push it to rawhide and at least testing for current Fedora releases. It's not as if it can activate without explicit packager invocation anyway, and people that will try it will probably appreciate being able to rebuild the same spec for all current releases.

EL7 is another thing. The macro set works fine for me on el7 (without the autosetup part) but I can live with a private el7 macro package. Most of Fedora's current Go macros have not been pushed to EL7 anyway (EL7 Go state is awful)

Comment 35 Nicolas Mailhot 2018-01-08 14:52:39 UTC
Hi

Is there something that still needs to be done there for the files to be merged?

It needs to be done so we can advance to Go-specific packaging in other packages

Comment 36 Jason Tibbitts 2018-01-09 18:58:32 UTC
I was kind of hoping someone else would chime in with some support, but it's been long enough so I'll just say that I'm going to go ahead and import some version of this within the next couple of days unless someone objects.

That said, I do have a couple of questions.

Is the attachment from Dec. 18 the one you would want to import?

Is there any portion of these macros which you want to somehow declare as less "done" than some other part?  I'm wondering if there's a way to explicitly make no guarantees about interface stability or even hide the parts which are still "experimental" behind an "I know what I'm doing" setting.

Comment 37 Nicolas Mailhot 2018-01-09 19:32:26 UTC
Thank you for answering

Attachment 1369487 [details] is still golden on my side. I haven't found the need to modify it in any way since (despite creating a few hundreds of packages that use it on el7 and rawhide), and no one has requested any other changes

Like I wrote before, the only part that may use some tweaking is the two foosetup helpers, but that's pretty much independent of the main macro

The next step (Go specific, in Go packages) is way more hairy given the current state of Go in Fedora

Comment 38 Nicolas Mailhot 2018-01-09 19:41:50 UTC
If you don't want to push it to EL you can drop the \\n s at the end of the
%{warn
%{error
%{echo

lines as tooling in devel will add an end of line contrary to EL7

Otherwise you'll have a lot of ugly empty lines in debugging output

But that's purely cosmetic, it does not change the functionality itself

You have lots of example packages that use the macros in
https://copr.fedorainfracloud.org/coprs/nim/More_Go_Packaging

when I'm not scratching it up to verify bootstraping

(with some Go macro glue over)

Comment 39 Jason Tibbitts 2018-01-12 02:10:44 UTC
For the record, this is done in rawhide, though of course only in koji until the next rawhide compose finishes.

I assume you want to let it sit for a bit before we think about pushing it down to the release branches.  EL7 is certainly a possibility as well, though that will have to go through epel-rpm-macros.  I don't know about EL6, but we should do it if it's at all possible even if we have to strip some things out.

Comment 40 Panu Matilainen 2018-01-12 06:09:24 UTC
Erm, this fell through the cracks of me leaving it alone while it was still changing every day, going to vacation and not seeing any new activity until suddenly it's in rawhide.

Still haven't gotten that closer lookup at it, but as noted in comment #17 already, I DO OBJECT to those extremely common macro names, "scm", "tag" and "commit" in particular but "archive" things are very generic too and something that rpm itself might want to use one day. Please change them to be forge-something before this spreads any further.

Comment 41 Nicolas Mailhot 2018-01-12 12:58:54 UTC
@Jason: Thank you very much, I can move to the next step now

@panu: Yes the names are generic because the concepts are generic and pervasive and needed. They have spread to hundreds of Fedora packages years ago since they are generic variable names and used as such in our packaging guidelines. I'd *love* for rpm to adopt them but you can't forbid the world to use them while rpm is still mulling what to do with scm git and commits.

Comment 42 Panu Matilainen 2018-01-12 13:04:13 UTC
Individual specs can do whatever they will because that's limited to the spec, but these are system-wide. And, since they are special to your rpmforge* macros they should indicate that in the naming too.

Comment 43 Nicolas Mailhot 2018-01-12 13:43:18 UTC
Panu,

The macro automates existing spec practices
Practices mandated (and enforced) in years-old FPC guidelines
The variables have the same name and value as in FPC guidelines

And FPC only legitimated what everyone was already doing, since a "commit" is a hash (in git) both in Fedora and everywhere else (it's enforced by git tooling). A "tag" is whatever upstream chose to put in the tag FPC or rpm has not choice over it either

So, really, what is the point of changing obvious variable names everyone is already using (and have been using for years) just in case someday rpm may decide to change the definition everyone uses in the software world and has enough weigh to enforce this change (you *really* thing rpm can go tell git and other scms commit means something else from now on?)

Comment 44 Panu Matilainen 2018-01-12 13:52:47 UTC
The thing is, rpm doesn't own those words and abreviations, but neither do you. If I were the one writing those utility macros I'd want to make sure joerandom's usage of the same names don't mess up mine. As in, name spacing considered good, not bad. But whatever, I've voiced my concern, I'll stfu now.

Comment 45 Nicolas Mailhot 2018-01-12 14:18:15 UTC
I agree with the concern, but when something is generic and in general use it *is* generic and it is counter-productive to try to de-genericize it

The macro set in general (with the Go part in bug #1526721 that will go elsewhere) manipulates three macro namespaces:
— forge* for forge-specific stuff
— go* for Go specific stuff (I didn't choose this one, I aligned with existing Fedora Go macros)
– nil namespace for general generic concepts already in use

It was a PITA to sort those. The easy way to do it would have been to dump everything in a new namespace without thinking things up. But that would have severely hampered adoption and ease of use.

Comment 46 Jason Tibbitts 2018-01-12 16:22:30 UTC
For the record, nothing happened "suddenly".  There's been discussion about this forever.

Also for the record, I agree entirely with Panu and honestly think that the magic action at a distance macros should go away to be replaced entirely with parameters.  The functionality of %setup and %autosetup are the model for this.  I intend to make sure any documentation I touch only uses parameters.

However, I merged this (in rawhide only) because at some point we have to make some progress somehow.  Otherwise we'd just end up with someone having done a lot of work that gets ignored because two sides can't agree.  We should still work on improving itg, even if that does end up breaking early adopters.  Such is life.

So Panu, if you do feel strongly about this, please feel free to tweak what's there.

Comment 47 Jason Tibbitts 2018-01-12 16:27:21 UTC
And ad an addendum to that final sentence, I should add: If you feel _really_ strongly about this, just take it back out.  The last thing I want to do is make things worse.

Comment 48 Nicolas Mailhot 2018-01-12 17:14:46 UTC
(In reply to Jason Tibbitts from comment #46)

> Also for the record, I agree entirely with Panu and honestly think that the
> magic action at a distance macros should go away to be replaced entirely
> with parameters. 

That can not work because the same info (commit id, tag ,etc) is used all round the specs from ldflags to autodeps.

(and even if it wasn't hashes are long enough no one sane would pass them as parameters without putting them in a variable first)

commit is *not* an ancillary concept in scm-centered software. You can't hide it away under a parameter. That would be about as sane as removing %{version} because it can be passed as parameter to the spec parts that need it

Comment 49 Nicolas Mailhot 2018-01-23 10:58 UTC
Created attachment 1384796 [details]
Proposed macro file

1. Add bitbucket support (as usual FPG were slightly wrong and misleading). This finalizes support for all forges Fedora knew about
2. Small error path enhancements that proved useful when debugging bitbucket weirdness

Comment 50 Igor Gnatenko 2018-01-23 11:08:14 UTC
(In reply to Nicolas Mailhot from comment #49)
> Created attachment 1384796 [details]
> Proposed macro file
> 
> 1. Add bitbucket support (as usual FPG were slightly wrong and misleading).
> This finalizes support for all forges Fedora knew about
> 2. Small error path enhancements that proved useful when debugging bitbucket
> weirdness

I think at this point, it is better to open Pull Request for redhat-rpm-config instead of attaching macro here.

https://src.fedoraproject.org/rpms/redhat-rpm-config

Comment 51 Nicolas Mailhot 2018-01-23 14:12:23 UTC
Yes, sorry, I started it and then stopped since pagure does not allow https push so I need to do it from another system

And I forgot to update state here :(

Comment 52 Nicolas Mailhot 2018-01-23 16:54:18 UTC
And the PR is done here:
https://src.fedoraproject.org/rpms/redhat-rpm-config/pull-request/11


Note You need to log in before you can comment on or make changes to this bug.