Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

go.mod: change imports to github.com/distribution/distribution/v3 #3225

Merged
merged 1 commit into from Feb 9, 2021

Conversation

thaJeztah
Copy link
Member

Go 1.13 and up enforce import paths to be versioned if a project
contains a go.mod.

The current v2.x branches (and releases) do not yet have a go.mod,
and therefore are still allowed to be imported with a non-versioned
import path (go modules add a +incompatible annotation in that case).

However, now that this project has a go.mod file, incompatible
import paths will not be accepted by go modules, and attempting
to use code from this repository will fail.

This patch uses v3 for the import-paths (not v2), because changing
import paths itself is a breaking change, which means that the
next release should increment the "major" version to comply with
SemVer (as go modules dictate).

@thaJeztah
Copy link
Member Author

@manishtomar @olegburov @dmcgowan @stevvooe PTAL: adding go modules in this repository forces us to update the import paths (because this repository already has v2.xx tags); because of that, the next release must be a v3.xx release (unfortunately, go modules doesn't allow separating binary from golang releases for the whole repo)

@thaJeztah
Copy link
Member Author

Note that the recommendation is to move all code to a vXX subdirectory to support vendoring tools that do not understand versioned import-paths; I decided not to make that change (because this project uses release branches)


// Version indicates which version of the binary is running. This is set to
// the latest release tag by hand, always suffixed by "+unknown". During
// build, it will be replaced by the actual version. The value here will be
// used if the registry is run after a go get based install.
var Version = "v2.7.0+unknown"
var Version = "v3.0.0+unknown"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed this version string; updated to v3.0.0 to match the change

@thaJeztah thaJeztah force-pushed the fix_gomod branch 3 times, most recently from aa3c99e to 31b0ae6 Compare August 26, 2020 11:12
Makefile Outdated Show resolved Hide resolved
@ob1dev
Copy link
Contributor

ob1dev commented Sep 4, 2020

This patch uses v3 for the import-paths (not v2), because changing import paths itself is a breaking change, which means that the next release should increment the "major" version to comply with

If there's still a plan for 2.x release(s), maybe it make sense to merge this after and start 3.x releases.

@thaJeztah
Copy link
Member Author

If there's still a plan for 2.x release(s), maybe it make sense to merge this after and start 3.x releases.

There's release/2.x branches, so for changes to go into a 2.x release, I guess those will have to be cherry picked into those branches

@thaJeztah
Copy link
Member Author

Rebased this one

Base automatically changed from master to main January 27, 2021 15:51
@thaJeztah
Copy link
Member Author

With this repository being moved to https://github.com/distribution/distribution, we should probably rename the module (and imports) accordingly, further qualifying next release to be v3.0.0

@milosgajdos
Copy link
Member

milosgajdos commented Jan 27, 2021

Whilst I'm waiting for the GH org invite so I can start GH review, @thaJeztah could you please change the import paths to the new GH org URL? (including the go module name directive, indeed)

@thaJeztah
Copy link
Member Author

Yup; will update the PR to use the new home 👍

@thaJeztah thaJeztah changed the title go.mod: change imports to github.com/docker/distribution/v3 go.mod: change imports to github.com/distribution/distribution/v3 Jan 27, 2021
@thaJeztah
Copy link
Member Author

Updated 👍

@@ -1,4 +1,4 @@
module github.com/docker/distribution
module github.com/distribution/distribution/v3

go 1.12
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we bump this to 1.13 at least? Or ideally, match it with what's in travis.yaml i.e. 1.14?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was considering changing it, but it's not directly related to this change. Generally I try to be a conservative to allow projects to build with other Golang versions (although Golang does not (yet) "enforce" the go version, it's possible they start doing so).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably reflect the minimum required version to build the project. I don't think we rely in any 1.13+ feature, so seems OK to keep it as-is.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes; and even if building the "whole" project requires Go version X, it's possible there are consumers that only use a single package from this repo, which may in itself not require that version; we could document that the project was "tested on" / "requires" / "recommends" a specific Go version to build (e.g., in "BUILDING.md").

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Sounds good. I was thinking more along the lines of "dropping the support for older versions of Go", but this is ok with me. Adding BUILD.md file with notes and instructions sounds good.

@wy65701436
Copy link
Collaborator

Can we merge it? Then the dependencies can update their source.

@joaodrp
Copy link
Collaborator

joaodrp commented Feb 7, 2021

Can we merge it? Then the dependencies can update their source.

I think so, it just needs a rebase.

Go 1.13 and up enforce import paths to be versioned if a project
contains a go.mod and has released v2 or up.

The current v2.x branches (and releases) do not yet have a go.mod,
and therefore are still allowed to be imported with a non-versioned
import path (go modules add a `+incompatible` annotation in that case).

However, now that this project has a `go.mod` file, incompatible
import paths will not be accepted by go modules, and attempting
to use code from this repository will fail.

This patch uses `v3` for the import-paths (not `v2`), because changing
import paths itself is a breaking change, which means that  the
next release should increment the "major" version to comply with
SemVer (as go modules dictate).

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

Rebased, and did a quick find & replace to check if I missed renames from docker/distribution to distribution/distribution

@wy65701436 wy65701436 merged commit 22c0748 into distribution:main Feb 9, 2021
@thaJeztah thaJeztah deleted the fix_gomod branch February 9, 2021 08:01
@RainbowMango
Copy link

Hi, I wonder if there is a plan to tag a v3.x.x release?

For now, if people have a dependency on this repo, would get a pseudo version like github.com/distribution/distribution/v3 v3.0.0-20210505084013-d0deff9cd6c2.

It's not a big deal, but there is quite a long time since the last release, I mean v2.7.1.

@milosgajdos
Copy link
Member

We are in a process of cutting a patch release for 2.7.x, but we are still sorting out some details.
There is no immediate plan for 3.x release I'm afraid. Maintainers need to agree on the roadmap, but first we need to cut the 2.7.x release. Stay tuned @RainbowMango

@RainbowMango
Copy link

Thanks @milosgajdos. Good to know.

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.

None yet

6 participants