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...
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