#1307 Clarification needed regarding the naming of Golang packages
Closed: fixed 8 months ago by james. Opened 2 years ago by eclipseo.

Guideline page needing clarification:

https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/#multiple

Explanation

An issue occurred during a package renaming request of a Golang package: https://pagure.io/releng/fedora-scm-requests/issue/55886#comment-871881

We were asking a rename from golang-k8s-klog to golang-k8s-klog-2:

This is a rename of golang-k8s-klog to match the import path k8s.io/klog/v2

rpm \# https://github.com/kubernetes/klog %global goipath k8s.io/klog/v2 %global forgeurl https://github.com/kubernetes/klog Version: 2.100.1

as specified in https://github.com/kubernetes/klog/blob/v2.100.1/go.mod

@tibbs noted that it wasn't correct per the guidelines:

I'm curious as to why there is an extra separator in the name. From https://docs.fedoraproject.org/en-US/packaging-guidelines/Naming/#multiple:

If the base package name does not end with a digit, the version MUST be directly appended to the package name with no intervening separator.

So unless I'm missing something, the package should be named golang-k8s-klog2

The issue is that it was always done like this within the Go ecosystem and it is hardcoded this way within our macros.

The macro computing the name of the package is as follow:

local function rpmname(goipath, compatid)
  -- lowercase and end with '/'
  local   goname = string.lower(rpm.expand(goipath) .. "/")
  -- remove eventual protocol prefix
  goname         = string.gsub(goname, "^http(s?)://",         "")
  -- remove eventual .git suffix
  goname         = string.gsub(goname, "%.git/+",              "")
  -- remove eventual git. prefix
  goname         = string.gsub(goname, "^git%.",               "")
  -- remove FQDN root (.com, .org, etc)
  -- will also remove vanity FQDNs such as "tools"
  goname         = string.gsub(goname, "^([^/]+)%.([^%./]+)/", "%1/")
  -- add golang prefix
  goname         = "golang-" .. goname
  -- compat naming additions
  local compatid = string.lower(rpm.expand(compatid))
  if  (compatid ~= nil) and (compatid ~= "") then
    goname       = "compat-" .. goname .. "-" .. compatid
 end
  -- special-case x.y.z number-strings as that’s an exception in our naming
  -- guidelines
  repeat
    goname, i    = string.gsub(goname, "(%d)%.(%d)",           "%1:%2")
  until i == 0
  -- replace various separators rpm does not like with -
  goname         = string.gsub(goname, "[%._/%-~]+",            "-")
  -- because of the Azure sdk
  goname         = string.gsub(goname, "%-for%-go%-",          "-")
  -- Tokenize along - separators and remove duplicates to avoid
  -- golang-foo-foo-bar-foo names
  local result = ""
  local tokens = {}
  tokens["go"]     = true
  for token in string.gmatch(goname, "[^%-]+") do
     if not tokens[token] then
        result = result .. "-" .. token
        tokens[token] = true
     end
  end
  -- reassemble the string, restore x.y.z runs, convert the vx.y.z
  -- Go convention to x.y.z as prefered in rpm naming
  result = string.gsub(result, "^-", "")
  result = string.gsub(result, ":", ".")
  -- some projects have a name that end up in a number, and *also* add release
  -- numbers on top of it, keep a - prefix before version strings
  result = string.gsub(result, "%-v([%.%d]+)$", "-%1")
  result = string.gsub(result, "%-v([%.%d]+%-)", "-%1")
  return(result)
end

The goal is to convert the import path of a Golang package to a Fedora package name

so k8s.io/klog becomes golang-k8s-klog
and k8s.io/klog/v2 becomes golang-k8s-klog-2

This has been this way since the Go-sig creation and macroization (2018) by @nim .

The scheme has been to replace the /vX by -X in the package name. I think the /vX was not considered part of the versioning but part of the package identification, as the import path of a Golang library is what separate one library from one another.As such, separate "version" of a library are not considered compat packages but separate libraries. Some project have multiple versions active for separate API support for example.

Thus for us it is not part of the version but part of the name derived from the import path.

Second point is, if we have to change the macros now, it will break hundred of packages because their name is computed from the Lua macro above. If we change that macro, the name of all packages with a number in the import path changes. Potentially we would need to request hundreds of new repos and provides upgrade path for them. We already behind our maintenance, and don't really have extra time to spare regarding this.

So we would need a clarification regarding the naming guidelines for Golang packages, both in the naming guidelines and in the Golang guidelines.


Some context

  1. golang is serious about its semver versionning, a vX is an API break and not substitutable to vX-1
  2. there is a very long tradition in Fedora packaging of treating such API break changes as a new package, and naming it foo-version (for example all the java-version packages starting from the 2000s in RHL before Fedora was even born)
  3. it is sad when the usual NIHism leads to enshrining practices, that differ, from the ones Fedora used for decades

I'm fine with documenting this pattern as an acceptable for naming packages for different versions of the same project. The current rules always struck me as odd, requiring no separator (unless the base name ends with a digit).

Side note: Rust crates are also namespaced by <name>-<version> everywhere, having no separator in the name for the corresponding compat packages makes things inconsistent as well. So I f the "separating base name with version identifier with a hyphen" will become acceptable, I will very likely do the same thing for Rust packages.

The current rules always struck me as odd, requiring no separator (unless the base name ends with a digit).

That’s because the current rules are inspired by the practices used by glib and other projects where the api break came as a deep unpleasant surprise to maintainers, so they just slapped a number at the end of the project name with little other thought (it was not supposed to happen, it was not supposed to happen again and the API overlap was supposed to be short requiring no naming thought because unicorns and C). Sometimes there was no major version break so the full version was used removing dots because there was no way 1.3 would ever collide with 13 in the future (haha).

Languages that had to think about dealing with API breaks as something that would happen again and again with potentially years of API overlaps and the API version potentially not matching the full software version have been more serious about it and did not forget to specify a separator.

The go macros just replace the /vX go-ism with -X, using the usual rpm naming separator as java and others before them. That’s simple stupid legible and robust and does not require special casing project names that end in a number, nor inventing secondary rpm name separators.

In go v1 is implicit because like everyone else they thought they would avoid API breaks at first, but at least they’ve been smart enough to define a convention that deals with future API breaks in a systematic way while grandfathering existing component naming.

Also what’s the point of defining special separators for API breaks instead of using the usual rpm hyphen ? No special separator is going to tell you X12 is Wayland, a lot of projects take advantage of the break to rename themselves like the xorg guys did, so defining a special separator to make it machine parsable has little practical value. It won’t even tell you which one is the latest and greatest (see rpm5), the only actual requirement is to make it reasonably collision-free.

Can we add this to the next meeting, we can't move forward if this is not resolved.

I've tagged it accordingly. Not sure if we can reach consensus on this this week, but we should at least talk about it.

Metadata Update from @decathorpe:
- Issue tagged with: meeting

2 years ago

We discussed this during today's meeting, and there was consensus for two things:

  • we don't like that package Name is controlled by a macro where its implementation can change and could lead to situations where source package name != dist-git package name
  • we don't like the situation we have ended up with wrt/ the Go package naming, since the guidelines around compat packages have been around when the diverging scheme for Go packages was devised, and don't want this to set a precedent for "ignore the guidelines for long enough and it will become accepted practice"

Our suggestions are:

  • If the package names would continue to use a hyphen as separator for the version specifier ("-vN"), we would like the package suffix to be "-vN" as well.
  • If you would like the current Go naming practices to remain in place, we would consider documenting the exception for using a hyphen as a separator for this purpose.

For the first option, I think no changes to the Naming Guidelines would be needed. For the second option, an exception would need to be documented in either the Naming Guidelines, or in the Go Packaging Guidelines directly.

(if I am mis-characterizing something in this summary, please correct me).

Also worth noting that (on F37) there's only 22 binary packages with -2, -3 or *-4 in their name. So it might not be that painful to just change/fix things.

we don't like that package Name is controlled by a macro where its implementation can change and could lead to situations where source package name != dist-git package name

Right, this is one of the reasons I'm hesitant to make that change at this point. We could always add a flag to enable the "new" scheme, but I'm not sure that's worthwhile.

If you would like the current Go naming practices to remain in place, we would consider documenting the exception for using a hyphen as a separator for this purpose.

I think that's the best way forward.

Also worth noting that (on F37) there's only 22 binary packages with -2, -3 or *-4 in their name. So it might not be that painful to just change/fix things.

There are much more than that:

$ fedrq pkgs -s -Fname \* | rg 'golang-.*-\d+$' | wc -l
134

Right, this is one of the reasons I'm hesitant to make that change at this point. We could always add a flag to enable the "new" scheme, but I'm not sure that's worthwhile.

Rather than changing the macro implementation, we can just change the guidelines to say maintainers SHOULD NOT use %{goname}. This is already the case for "well-known application" like containerd, etcd, hugo, and more. Libraries would switch from using %{goname} to an explicit name that follows the package naming guidelines. That wouldn't make the process of requesting a new dist-git repo any easier, but it would make the spec file changes trivial. At some point after all usage of %{goname} is absent from rawhide spec files, the macro could be changed to trigger an error if it's used.

I would prefer we pick the option that makes the most technical sense, rather than the one that has the most inertia. I'd be open to a "going forward" policy, letting non-compliant package names age out over time (or get converted gradually as people get to them).

In any case, the current situation is blocking for the rest of the ecosystem. We need to adopt a solution quickly.

This is already the case for "well-known application" like containerd, etcd, hugo, and more. Libraries would switch from using %{goname} to an explicit name that follows the package naming guidelines

For some well-known binary package, we do that.

What could we do?
- New packages should adopt the correct guidelines.
- Older packages should be kept as is for compatibility.
- We implement an alternative goname macro that compute the name correctly?
- In go2rpm we do not use the %goname macro but hardcode the result of it.

I'm not sure where is used goname internally? There's Name: but also all the autogenerated -devel package Name. We can't really switch that out.

Current affected packages:

fedrq pkgs -s -Fname * | rg 'golang-.*-\d+$'

golang-bug-serial-1
golang-github-ahmetb-linq-3
golang-github-alecaivazis-survey-2
golang-github-alecthomas-assert-2
golang-github-alecthomas-chroma-2
golang-github-apache-beam-2
golang-github-apapsch-jsonmerge-2
golang-github-apparentlymart-textseg-13
golang-github-aws-sdk-2
golang-github-bmatcuk-doublestar-3
golang-github-bmatcuk-doublestar-4
golang-github-casbin-2
golang-github-cheggaaa-pb-3
golang-github-colinmarc-hdfs-2
golang-github-containerd-btrfs-2
golang-github-containerd-cgroups-3
golang-github-d5-tengo-2
golang-github-distribution-3
golang-github-gdamore-tcell-2
golang-github-git-5
golang-github-git-billy-5
golang-github-git-fixtures-4
golang-github-git-lfs-gitobj-2
golang-github-git-lfs-wildmatch-2
golang-github-gocolly-colly-2
golang-github-gofiber-fiber-2
golang-github-golangci-lint-1
golang-github-google-renameio-2
golang-github-gorp-3
golang-github-grpc-ecosystem-gateway-2
golang-github-hanwen-fuse-2
golang-github-hashicorp-hcl-2
golang-github-hashicorp-lru-2
golang-github-iguanesolutions-systemd-5
golang-github-jackc-chunkreader-2
golang-github-jedib0t-pretty-6
golang-github-jose-3
golang-github-jwt-4
golang-github-jwt-5
golang-github-labstack-echo-4
golang-github-lestrrat-backoff-2
golang-github-masterminds-semver-1
golang-github-minio-6
golang-github-mitchellh-hashstructure-2
golang-github-moby-swarmkit-2
golang-github-nathanaelle-syslog5424-2
golang-github-nats-io-jwt-2
golang-github-ncw-swift-2
golang-github-nicksnyder-i18n-2
golang-github-onsi-ginkgo-2
golang-github-oracle-oci-sdk-24
golang-github-pelletier-toml-2
golang-github-peterbourgon-ff-3
golang-github-pin-tftp-3
golang-github-playground-assert-2
golang-github-playground-validator-10
golang-github-posener-complete-2
golang-github-quay-clair-3
golang-github-quic-qtls-go1-20
golang-github-r3labs-diff-3
golang-github-schollz-cli-2
golang-github-schollz-pake-3
golang-github-schollz-progressbar-2
golang-github-schollz-progressbar-3
golang-github-sebdah-goldie-1
golang-github-stomp-3
golang-github-twpayne-vfs-4
golang-github-urfave-cli-2
golang-github-vektah-gqlparser-2
golang-github-vmihailenco-msgpack-5
golang-github-vmihailenco-tagparser-2
golang-github-vultr-govultr-2
golang-github-vultr-govultr-3
golang-github-zmap-zlint-3
golang-gonum-1
golang-gopkg-alecthomas-kingpin-2
golang-gopkg-asn1-ber-1
golang-gopkg-check-1
golang-gopkg-cheggaaa-pb-1
golang-gopkg-errgo-2
golang-gopkg-gcfg-1
golang-gopkg-gorp-1
golang-gopkg-h2non-gock-1
golang-gopkg-inf-0
golang-gopkg-ini-1
golang-gopkg-jcmturner-aescts-1
golang-gopkg-jcmturner-dnsutils-1
golang-gopkg-jcmturner-goidentity-2
golang-gopkg-jcmturner-goidentity-3
golang-gopkg-jcmturner-gokrb5-7
golang-gopkg-jcmturner-rpc-0
golang-gopkg-jcmturner-rpc-1
golang-gopkg-ldap-2
golang-gopkg-logex-1
golang-gopkg-macaron-1
golang-gopkg-macaroon-1
golang-gopkg-mgo-2
golang-gopkg-natefinch-lumberjack-2
golang-gopkg-neurosnap-sentences-1
golang-gopkg-ns1-2
golang-gopkg-olivere-elastic-2
golang-gopkg-op-logging-1
golang-gopkg-pipe-2
golang-gopkg-readline-1
golang-gopkg-redis-6
golang-gopkg-rethinkdb-6
golang-gopkg-retry-1
golang-gopkg-robfig-cron-3
golang-gopkg-russross-blackfriday-1
golang-gopkg-russross-blackfriday-2
golang-gopkg-seborama-govcr-4
golang-gopkg-sourcemap-1
golang-gopkg-square-jose-2
golang-gopkg-src-d-billy-4
golang-gopkg-src-d-git-4
golang-gopkg-src-d-git-fixtures-3
golang-gopkg-stack-0
golang-gopkg-tomb-1
golang-gopkg-tomb-2
golang-gopkg-tylerb-is-1
golang-gopkg-vmihailenco-msgpack-2
golang-gopkg-warnings-0
golang-gopkg-yaml-1
golang-gopkg-yaml-2
golang-gopkg-yaml-3
golang-helm-3
golang-modernc-cc-3
golang-modernc-cc-4
golang-modernc-ccgo-3
golang-modernc-ccgo-4
golang-modernc-gc-2
golang-mvdan-sh-3
golang-oras-1
golang-oras-2

So what was done finally:

  • In go-rpm-macros, we have added a flag -L to the gometa macro to enable the new (read correct) versioning for new packages. This should be the default for every new packages. It enables a global flag go_use_new_versioning.

  • In [go2rpm]https://pagure.io/GoSIG/go2rpm/c/8b6977dbeca1a7e54921bfe08136520a039b279f?branch=master)(, we have added -L to gometa by default in the template, but we can disable it with --no-use-new-versioning.

  • the Name: field is now hardcoded and not dynamically computed by %goname

In fine, we have 134 "bad" packages that will stay as is to avoid breakage. Future packages will be correctly named.

Do we need to change the doc to reflect this?

Metadata Update from @james:
- Issue close_status updated to: fixed
- Issue status updated to: Closed (was: Open)

8 months ago

Do we need to change the doc to reflect this?

Yes, none of the text or examples at https://docs.fedoraproject.org/en-US/packaging-guidelines/Golang/ show an invocation of %gometa with any parameters.

Log in to comment on this ticket.

Metadata