Skip to content

estimate: Delete incorrect or unnecessary code#309

Open
rhansen wants to merge 8 commits intoDebian:masterfrom
rhansen:delete
Open

estimate: Delete incorrect or unnecessary code#309
rhansen wants to merge 8 commits intoDebian:masterfrom
rhansen:delete

Conversation

@rhansen
Copy link
Copy Markdown
Contributor

@rhansen rhansen commented Mar 7, 2026

See the commit messages for details.

@rhansen rhansen force-pushed the delete branch 2 times, most recently from 488e6b6 to c070ea4 Compare March 7, 2026 23:24
@n-peugnet
Copy link
Copy Markdown
Contributor

I completely agree with the third commit, but the first two are really removing quality of life feature that I added on purpose.

estimate: Don't consider incompatible module versions
Go requires a different module path for a module with a different major version, so any attempt to build against a module with a different major version should fail. Don't pretend that a different major version could be compatible.

The idea was not to pretend that it would be compatible but to indicate that the package already exist in Debian and it might need to be updated, which is not really the same task as creating a new one from scratch. This is why it displays a warning with a link that allows to see the packaged version.

Also, oftentimes, Go packages in Debian don't include any major string even if it is not the v1. In these cases, then it might really be packaged with the correct version. Showing the link allows to see if it is the case or if it will need to be updated (This is the "has no version string in Debian" warning).

estimate: Don't search for packaged repo root
Despite its name, XS-Go-Import-Path names a module path, not a package import path. From Go's perspective, two modules with different module paths are completely unrelated, even if one module path is a prefix of another. It is always incorrect to assume that a Debian package providing example.com/foo can satisfy a requirement for example.com/foo/v2 and vice-versa.

For non-modules (which by definition don't have a module path), the go binary and the GOPROXY service (proxy.golang.org) use the repository root as the synthesized module path, so that is what XS-Go-Import-Path also contains. Thus, common prefixes don't apply to non-modules, either.

Well currently, quite a few Debian packages install multiple go modules. They should declare all the modules they install in XS-Go-Import-Path, keeping the first entry for the repo root, but some of them don't. So fallbacking to the repo root allows at least to handle gracefully packages that only list the repo root in this variable.


So maybe ideally we wouldn't have to have this code, it is still currently useful IMO.

@rhansen
Copy link
Copy Markdown
Contributor Author

rhansen commented Mar 8, 2026

@n-peugnet

The idea was not to pretend that it would be compatible but to indicate that the package already exist in Debian and it might need to be updated, which is not really the same task as creating a new one from scratch. This is why it displays a warning with a link that allows to see the packaged version.

You're right, it's not exactly the same task. However, if the source and binary package naming conventions and the one Go module per source package guidelines are followed, it is almost the same as creating a new package from scratch.

How about we print a hint and remove this part:

				// but consider that it is packaged and skip the children.

For example, we could change estimate to output something like this:

example.com/foo
  example.com/bar/v4 (see golang-example-bar-v3 for inspiration)
    example.com/baz
  example.com/asdf

instead of the current behavior:

2026/03/08 16:45:15 example.com/bar/v4 is v3 in Debian (golang-example-bar-v3)
... potentially many lines later ...
2026/03/08 16:45:15 Bringing example.com/foo to Debian requires packaging the following Go modules:
example.com/foo
  example.com/asdf

which I find misleading due to the missing example.com/bar/v4 and its dependencies.

Also, oftentimes, Go packages in Debian don't include any major string even if it is not the v1. In these cases, then it might really be packaged with the correct version. Showing the link allows to see if it is the case or if it will need to be updated (This is the "has no version string in Debian" warning).

The XS-Go-Import-Path metadata field should contain the full module path, including the major version suffix. The Debian package name might not have -v4 (or whatever) in its name, but its Go-Import-Path should have /v4. If not, that is a bug in the package.

The Go-Import-Path metadata is what estimate uses to find Debian packages for a Go module. If that field is set correctly, estimate should work correctly regardless of the Debian package name. If XS-Go-Import-Path erroneously lacks the major version suffix (or has the wrong major suffix), then my proposed change above would print something like:

example.com/foo
  example.com/bar/v4 (see golang-example-bar-v4 for inspiration)
  example.com/asdf

(Assuming the Debian package is correctly named golang-example-bar-v4 and its XS-Go-Import-Path is erroneously set to something like example.com/bar.)

Well currently, quite a few Debian packages install multiple go modules.

Yes, though hopefully new Debian packages won't thanks to the "one Go library module per Debian source package" best practice.

They should declare all the modules they install in XS-Go-Import-Path,

Yes.

keeping the first entry for the repo root,

No. XS-Go-Import-Path should only contain the module paths of the modules it installs (I opened a merge request to make this clearer). Otherwise dh-make-golang cannot reliably determine which packages provide a particular module.

Note that pre-module package collections don't have a go.mod so they do not have a module path. However, Go and the GOPROXY service (proxy.golang.org) synthesizes a module using the repo root as the synthesized module path. So if we're talking about packaging pre-module Go packages, then XS-Go-Import-Path contains the repo root but only because the repo root is also the synthesized module path. See the "Go packages that are not inside a Go module" best practice.

but some of them don't. So fallbacking to the repo root allows at least to handle gracefully packages that only list the repo root in this variable.

A package that does not list every module path for every module it installs is simply buggy. The package maintainer should fix the package; it should not be dh-make-golang's responsibility to apply heuristics to figure out what the package maintainer intends.

With my suggested change, dh-make-golang estimate would print the "see golang-* for inspiration" suggestion, which would be a hint that XS-Go-Import-Path is set incorrectly.

@rhansen rhansen force-pushed the delete branch 3 times, most recently from 37a715f to 8dde6df Compare March 13, 2026 22:26
@rhansen
Copy link
Copy Markdown
Contributor Author

rhansen commented Mar 24, 2026

How about we print a hint and remove this part:

				// but consider that it is packaged and skip the children.

For example, we could change estimate to output something like this:

example.com/foo
  example.com/bar/v4 (see golang-example-bar-v3 for inspiration)
    example.com/baz
  example.com/asdf

I decided to go ahead and code this up. Please take a look, @n-peugnet.

@n-peugnet
Copy link
Copy Markdown
Contributor

I just tested and I think this is better, but I still think that this pull request is a bit early. Considering the guidelines have just recently been changed.

BTW I did not notice this change have been merged, did I miss an announcement on the mailing-list? Also It would be nice to update the "Workflow Changes" page with a quick recap of what has been changed, so that everybody can know about what they need to change.

I feel like this PR is trying to match the new guidelines while most of the packages are still following the old ones. So for example, if I try to estimate livekit-server I get a lot of false positives:

$ go run . estimate -git_revision v1.8.4 github.com/livekit/livekit-server
[...]
  github.com/d5/tengo/v2 (see golang-github-d5-tengo for inspiration)
[...]
  github.com/jellydator/ttlcache/v3 (see golang-github-jellydator-ttlcache for inspiration)
[...]
    github.com/pion/sdp/v3 (see golang-github-pion-sdp for inspiration)
[...]
      github.com/redis/go-redis/v9 (see golang-github-redis-go-redis for inspiration)
[...]
      github.com/cespare/xxhash/v2 (see golang-github-cespare-xxhash for inspiration)
[...]
    github.com/puzpuzpuz/xsync/v3 (see golang-github-puzpuzpuz-xsync for inspiration)
[...]
  github.com/urfave/negroni/v3 (see golang-github-urfave-negroni for inspiration)

Out of these, half of them have the correct major version (tengo, ttlcache & xxhash). What could be nice would be if dh-make-golang could check the Debian version and verify that it is indeed the correct major.

If I understand correctly, you are pushing for packaging every major version as distinct Debian packages. This may simplify some things but it has never been the norm in the Debian Go team. So I don't think dh-make-golang can assume this is the case right now.

@rhansen
Copy link
Copy Markdown
Contributor Author

rhansen commented Mar 27, 2026

I just tested and I think this is better, but I still think that this pull request is a bit early. Considering the guidelines have just recently been changed.

Shouldn't the guidelines and the tools always be in sync? Tools aren't very useful if they go against the current guidelines.

I do recognize that not everyone has had a chance to critique the recent changes, so they may still be a bit in flux. The only feedback I've received so far is editorial, not normative, but that could change.

BTW I did not notice this change have been merged, did I miss an announcement on the mailing-list?

I recently sent an announcement. Thank you for prodding me to do that.

Also It would be nice to update the "Workflow Changes" page with a quick recap of what has been changed, so that everybody can know about what they need to change.

Good idea; I added a section to the workflow changes page.

I feel like this PR is trying to match the new guidelines while most of the packages are still following the old ones.

I disagree with the premise of that statement. I don't think of my edits to the packaging doc as changing the recommendations; they're just clarifying the wording to match what was already (sometimes unwritten) current best practice. Thus, most packages already conform with the new wording. There are certainly exceptions, but that's why I made those edits—to steer those wayward packages into consistency with all of the others (and to prevent the introduction of new wayward packages).

So for example, if I try to estimate livekit-server I get a lot of false positives:

[...]

Out of these, half of them have the correct major version (tengo, ttlcache & xxhash). What could be nice would be if dh-make-golang could check the Debian version and verify that it is indeed the correct major.

From Go's perspective, it's the module path that matters, not the major version. If something requires example.com/foo/v2 but no Debian packages claim to provide example.com/foo/v2 in its XS-Go-Import-Path, then either no package provides example.com/foo/v2 or the package that does provide example.com/foo/v2 has an incorrect XS-Go-Import-Path. It looks like tengo, ttlcache, and xxhash need to be fixed, and the others need to be packaged. (I haven't looked closely though; maybe there is a bug in this PR.)

I could change the hint

(see $package for inspiration)

to

(does $package $version have an incorrect XS-Go-Import-Path?)

if the package's major version matches the desired Go module path major version suffix. Is that what you meant?

If I understand correctly, you are pushing for packaging every major version as distinct Debian packages.

Kinda, but not really. I'm pushing for reliable discovery of the Debian package that provides a given Go module. Right now, due to limitations with XS-Go-Import-Path and dh-make-golang we don't have reliable lookups. This causes incorrect estimate output and the need to manually correct the Debian packages produced by dh-make-golang make.

If:

  • every Debian package that provides a Go module includes the complete module path in its Go-Import-Path metadata, and
  • every Debian package with Go-Import-Path metadata actually provides the modules listed,

then lookups would be reliable. Currently this is only feasible if every major version is packaged separately. Hopefully with some future improvements we can relax this.

This may simplify some things but it has never been the norm in the Debian Go team. So I don't think dh-make-golang can assume this is the case right now.

What's the alternative? Currently dh-make-golang only has access to flawed information, so garbage in, garbage out. This PR just changes the smell of the garbage. 😄 I believe this PR makes the flaws in the output more actionable while also making the output more useful when the information is correct.

rhansen added 8 commits March 29, 2026 01:52
Downloaded Go modules do not have any `vendor` directories since Go
ignores them when creating a zip archive for the
GOPROXY (proxy.golang.org) service:
<https://go.dev/ref/mod#zip-files>.  Thus, there's no value in
searching for them.
Despite its name, XS-Go-Import-Path names a module path, not a package
import path.  From Go's perspective, two modules with different module
paths are completely unrelated, even if one module path is a prefix of
another.  It is always incorrect to assume that a Debian package
providing example.com/foo can satisfy a requirement for
example.com/foo/v2 and vice-versa.

For non-modules (which by definition don't have a module path), the
`go` binary and the GOPROXY service (proxy.golang.org) use the
repository root as the synthesized module path, so that is what
XS-Go-Import-Path also contains.  Thus, common prefixes don't apply to
non-modules, either.

Logging a hint that an already published Debian package provides a
related module is already handled by the `findOtherVersion` call.
Use `golang.org/x/mod/module.SplitPathVersion` to find related Go
modules that have already been packged in Debian.  This is a bit more
robust than parsing the module paths ourselves, as it handles corner
cases such as gopkg.in version suffixes.

Also, return the "closest" match, defined as the Debian package that
provides the module whose major version is most similar to the desired
major version (with ties broken in favor of the older version).
The messages "$modPath has no version string in Debian" and "$modPath
is v$maj in Debian" are confusing.  They are meant as hints that a
related package is already published and can serve as inspiration for
a new package for the given module, so word the log message
accordingly.
Go requires a different module path for a module with a different
major version, so any attempt to build against a module with a
different major version should fail.  Don't pretend that a different
major version could be compatible.

This changes the current behavior:

    2026/03/08 16:45:15 See golang-example-bar-v3 for inspiration (it packages example.com/bar/v3) when packaging example.com/bar/v4
    ... potentially many lines later ...
    2026/03/08 16:45:15 Bringing example.com/foo to Debian requires packaging the following Go modules:
    example.com/foo
      example.com/asdf

to something less misleading:

    2026/03/08 16:45:15 See golang-example-bar-v3 for inspiration (it packages example.com/bar/v3) when packaging example.com/bar/v4
    ... potentially many lines later ...
    2026/03/08 16:45:15 Bringing example.com/foo to Debian requires packaging the following Go modules:
    example.com/foo
      example.com/bar/v4
        example.com/baz
      example.com/asdf
This makes the suggestion easier to see.  Before:

    2026/03/08 16:45:15 See golang-example-bar-v3 for inspiration (it packages example.com/bar/v3) when packaging example.com/bar/v4
    ... potentially many lines later ...
    2026/03/08 16:45:15 Bringing example.com/foo to Debian requires packaging the following Go modules:
    example.com/foo
      example.com/bar/v4
        example.com/baz
      example.com/asdf

After:

    ... potentially many lines later ...
    2026/03/08 16:45:15 Bringing example.com/foo to Debian requires packaging the following Go modules:
    example.com/foo
      example.com/bar/v4 (see golang-example-bar-v3 for inspiration)
        example.com/baz
      example.com/asdf
It's only called from one place so it's more readable to not have a
separate function.
For consistency with the "see $pkg for inspiration" hint and a planned
"upgrade from $pkg $ver" hint that will be added in the future.
@rhansen
Copy link
Copy Markdown
Contributor Author

rhansen commented Mar 31, 2026

@n-peugnet

I could change the hint

(see $package for inspiration)

to

(does $package $version have an incorrect XS-Go-Import-Path?)

if the package's major version matches the desired Go module path major version suffix.

I think I'll do that, but it requires fetching the Debian version from from the FTP-Master API. It will require some non-trivial infrastructure changes that I plan on making anyway. Are you OK with postponing that enhancement until later so that we can get this merged sooner? (This PR is blocking other work.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants