Created attachment 1364961 [details] Proposed macro file See also : https://fedoraproject.org/wiki/Automated_forge_handling https://pagure.io/packaging-committee/issue/719
Created attachment 1365011 [details] Proposed macro file Same file without the duplicate archiveext fallback
Created attachment 1365014 [details] Proposed macro file Even better: move the archiveext fallback after known forge handling
Created attachment 1365185 [details] Proposed macro file Add GitLab support as requested by Matthew Miller Generalize GitHub support to hosted GitHub
Created attachment 1365262 [details] Proposed macro file Relax URL constrains as requested by Zbigniew Jędrzejewski-Szmek
Created attachment 1365275 [details] Proposed macro file Allow overriding just archivename or archiveurl
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.
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
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 :)
Created attachment 1365500 [details] Proposed macro file Integrate %setup automation as suggested in https://github.com/rpm-software-management/rpm/issues/368
Created attachment 1365577 [details] Proposed macro file Integrate a few error messages to answer common questions raised during FPC review
Created attachment 1365580 [details] Proposed macro file Fix spelling in error message
Created attachment 1365640 [details] Proposed macro file Make %setup adjustment transparent
Created attachment 1365641 [details] Proposed macro file Fix spelling
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.
Reassigned, as requested by Panu
Created attachment 1366029 [details] Proposed macro file %setup → %forgesetup + native rpm errors and warnings
> 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.
(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.
Created attachment 1366090 [details] Proposed macro file This one should be cleaner Simplify leftover cruft
Created attachment 1366092 [details] Proposed macro file Attach the right file :(
Created attachment 1367893 [details] Proposed macro file Be more explicit to avoit packager surprises Clean up and consolidation as requested by FPC
Created attachment 1367902 [details] Proposed macro file Simplify some more
Created attachment 1367908 [details] Proposed macro file Simplify some more
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)
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)
Created attachment 1368131 [details] Proposed macro file remove shortcommit remnants
Created attachment 1368135 [details] Proposed macro file Add a convenience %forgeautosetup macro
Created attachment 1368136 [details] Proposed macro file Remove a no longer necessary level of expand
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.
Created attachment 1368797 [details] Proposed macro file Fix macro expansion case in debugging
Created attachment 1369154 [details] Proposed macro file Fix autosetup helper
Created attachment 1369487 [details] Proposed macro file Relay more parameters to %setup
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?
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)
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
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.
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
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)
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.
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.
@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.
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.
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?)
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.
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.
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.
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.
(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
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
(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
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 :(
And the PR is done here: https://src.fedoraproject.org/rpms/redhat-rpm-config/pull-request/11