Hi Anyway, to answer some of the questions posted during review and in: https://meetbot.fedoraproject.org/fedora-meeting-2/2017-12-13/fpc.2017-12-13-18.00.log.html 1. I just posted the second part of the proposal (the Go-specific bits). Read it there https://fedoraproject.org/wiki/More_Go_packaging to understand some of the choices of this draft. 2. I added a %forgeautosetup helper for %autosetup users (though I don't use it myself, some testing would be appreciated) 3. I added some optional parameters for parameter lovers. Though the macro processes many rpm variables, and sets many others, which are used in other parts of the spec including in the proposed Go autoprovides, so full parametrization is neither suitable nor technically possible. This is not a single-purpose macro that influences a single part of the spec. Forge and SCM data has effects on all parts of the spec, like %{name} or %{version}. A %{commit} or %{tag} is just a form of %{version} anyway. People will have to set the variables to specific names anyway for the rest of the automation to work, our guidelines already mandate lots of variables to handle forge hosting, this proposal actually shrinks the number of variables needed to handle this kind of project (and don't make me start on the way different parts of our guidelines name the same variable in slightly different ways, causing a mess when you try to put everything together). 4. There is a bug in EL7 that causes spectool not to process the resulting files. rpmbuild and mock work fine though. I added a -i switch to the macro that prints the resolved source url, you can then dump it in curl, wget or whatever in EL7. Alternatively, get someone to fix the EL7 toolchain. 5. The macro does not handle nor intends to handle multiple downloaded tarballs. This is quite a rare case and it becomes infinitesimal when you add modern scm-based software publishing services to the mix. Plus, the Go automation is centered on a single root import path. 6. Most specs lag the current GitHub guidelines, the current GitHub guidelines are broken as written (because GiHub changed), and I'm pretty sure the resulting packages would fail a source download test (either because the url no longer exists or because it names the downloaded file some other way). That pretty conclusively shows the current way of handling such services does not work. If you want more proof, go look at some Go spec files that rely on GitHub (for example golang-googlecode-google-api-client). I haven't tested the other services our guidelines cover, they are probably also broken in some way. 7. the way of handling corner cases is already documented in the proposal (that's why it's so long, it treats all cases). If you have some other corner case in mind, please post it. 8. the macro could certainly grow as more software hosting services are added. That's not a problem because the complexity is not in the handling of a specific service, it's in the other common parts. Adding a service it mostly writing the Lua patterns (~regexes) to extract the needed parts from the project url, and then combining them the same way our guidelines currently do. The difference is that individual packagers do not need to read or apply those guidelines, the result is made available to the whole distro as soon as the macro package is updated. So the process complexity actually shrinks. Here is the full length of the GitHub-specific block for example if (string.match(forge, "^github[%.-]") or string.match(forge, "[%.-]github[%.]")) then forgeurl = string.match(forgeurl, "https://[^/]+/[^/]+/[^/#?]+";) if (forgeurl == "") then if not silent then rpm.expand("%{error:GitHub URLs must match https://(…[-.])github[-.]…/owner/repo !\\n}") end else explicitset("forgeurl", forgeurl) safeset("archiveext", "tar.gz") safeset("forgesetupargs", "-n %{archivename}") if (commit ~= "") or (tag ~= "") then safeset("scm", "git") end local owner = string.match(forgeurl, "^[^:]+://[^/]+/([^/]+)") local repo = string.match(forgeurl, "^[^:]+://[^/]+/[^/]+/([^/]+)") if (tag ~= "") then safeset("archivename", repo .. "-%{tag}") safeset("archiveurl", "%{forgeurl}/archive/%{tag}.%{archiveext}") else if (commit ~= "") then safeset("archivename", repo .. "-%{commit}") safeset("archiveurl", "%{forgeurl}/archive/%{commit}/" .. repo .. "-%{commit}.%{archiveext}") else safeset("archivename", repo .. "-%{version}") safeset("archiveurl", "%{forgeurl}/archive/v%{version}.%{archiveext}") end end end end Don't tell me that's overly difficult to understand or adapt. I'm sure the mediawiki markup dedicated to GitHub in our guidelines is actually longer (plus, it does not work and it is not applied consistently) It could be condensed by removing error handling, but what would be the point. A macro that actually does error handling is nice, for a change. 9. added with the new Go-specific proposal this proposal achieves a shrinking of some Go specs by 90%, so it is way over the "half spec" simplification bar And I think I covered all the raised objections. Regards, -- Nicolas Mailhot _______________________________________________ devel mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxx
