deprecate reference package, migrate to github.com/distribution/reference #4031

thaJeztah · 2023-08-30T16:10:52Z

relates to There is a surprising increase in transitive dependencies when we switch to distribution v3 #3587

This integrates the new module, which was extracted from this repository at commit b9b1940;

# install filter-repo (https://github.com/newren/git-filter-repo/blob/main/INSTALL.md)
brew install git-filter-repo

# create a temporary clone of docker
cd ~/Projects
git clone https://github.com/distribution/distribution.git reference
cd reference

# commit taken from
git rev-parse --verify HEAD
b9b19409cf458dcb9e1253ff44ba75bd0620faa6

# remove all code, except for general files, 'reference/', and rename to /
git filter-repo \
  --path .github/workflows/codeql-analysis.yml \
  --path .github/workflows/fossa.yml \
  --path .golangci.yml \
  --path distribution-logo.svg \
  --path CODE-OF-CONDUCT.md \
  --path CONTRIBUTING.md \
  --path GOVERNANCE.md \
  --path README.md \
  --path LICENSE \
  --path MAINTAINERS \
  --path-glob 'reference/*.*' \
  --path-rename reference/:

# initialize go.mod
go mod init github.com/distribution/reference
go mod tidy -go=1.20

codecov-commenter · 2023-08-30T16:18:13Z

Codecov Report

Patch coverage has no change and project coverage change: -0.95% ⚠️

Comparison is base (b9b1940) 57.99% compared to head (5370753) 57.04%.
Report is 2 commits behind head on main.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4031      +/-   ##
==========================================
- Coverage   57.99%   57.04%   -0.95%     
==========================================
  Files         101       96       -5     
  Lines        9991     9620     -371     
==========================================
- Hits         5794     5488     -306     
+ Misses       3560     3505      -55     
+ Partials      637      627      -10

Files Changed	Coverage Δ
notifications/bridge.go	`51.82% <ø> (ø)`
notifications/listener.go	`50.00% <ø> (ø)`
registry/api/v2/descriptors.go	`100.00% <ø> (ø)`
registry/api/v2/urls.go	`75.97% <ø> (ø)`
registry/client/repository.go	`54.62% <ø> (ø)`
registry/handlers/app.go	`48.91% <ø> (ø)`
registry/handlers/blobupload.go	`47.01% <ø> (ø)`
registry/handlers/manifests.go	`47.83% <ø> (ø)`
registry/proxy/proxyblobstore.go	`52.08% <ø> (ø)`
registry/proxy/proxymanifeststore.go	`42.30% <ø> (ø)`
... and 8 more

... and 5 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Jamstah · 2023-08-30T16:31:36Z

That was fast :)

I guess we should tag up the reference repo as 1.0.0 before merging this.

milosgajdos · 2023-08-30T18:10:14Z

Wild! Yeah we need to tag reference 🤔

thaJeztah · 2023-08-30T18:36:05Z

That was fast :)

😂 had to do similar moves a couple of times before, and now got the hang of it

I guess we should tag up the reference repo as 1.0.0 before merging this.

Yes, we should probably look if the repository still needs some cleaning up before (I'm guessing we'd want to add a minimal README that describes the purpose of the package).

Even though this package has been used in production (so technically v1.0.0 should be the correct version), perhaps we should tag a v0.x.x version first, to have some wiggle-room.

There are some deprecated types/functions in the package; I moved those as well, but those should definitely be removed at some point (I guess; ideally before v1.0.0).

I also want to go the rounds across some repositories to see if things explode if they would already switch to the new module even if they use a tagged version of distributions (i.e. any v2.x version).

I want to make sure that switching potential types doesn't cause builds to fail.

And, finally, given that not much has changed in the package (other than some performance improvements and bug fixes), we may want to consider switching the v2 branch to this package as well; that way v2 already would alias the types, which means there's no risk of Go (being a strong-typed language) from breaking things if a project happens to depend on both v3 and v2 in some form.

Jamstah · 2023-08-30T20:46:14Z

I'll take a look at the readme.

I agree that we should remove the deprecated functions before 1.0.0. How about now?

I plan to run round some repos and make some issues to make people aware.

I don't think we need to do the v2 stream. Repos will need to migrate at some point, and I don't think we need to support depending on v3 and v2 art the same time. Many projects will drop the distribution dependency and only need reference, I think.

thaJeztah · 2023-08-30T21:54:54Z

I don't think we need to do the v2 stream. Repos will need to migrate at some point, and I don't think we need to support depending on v3 and v2 art the same time. Many projects will drop the distribution dependency and only need reference, I think.

Yes, I think we're probably in the clear; v3 is already a different module (so a v2.Foo type would already be incompatible with a v3.Foo - even if they're otherwise identical), so I don't expect much issues there beyond some projects (temporarily) depending on both during the transition.

thaJeztah · 2023-08-30T23:13:08Z

I agree that we should remove the deprecated functions before 1.0.0. How about now?

Oh; forgot to answer this one; We need to check if those that are deprecated were already deprecated in the current (so: v2.8.x) release.

If not, then consumers may not have yet "received the message" (or more strictly; they haven't been deprecated yet, if no release was done with the deprecation in them).

I tend to follow a cycle of

mark deprecated (in a release), but provide aliases (or at least a pointer to where things went)
remove in the next release
where "release" is a "non-patch release"

milosgajdos · 2023-08-31T07:31:44Z

So we are due to make a minor 2.8.x release because of #4009

I wouldnt be against marking this package as deprecated in 2.8.x before cutting a release and pointing folks in the new direction.

thaJeztah · 2023-08-31T08:28:54Z

I'd be fine with tagging the distribution/reference package in its current state as v0.5.0 (so that users who get the notification can use a tagged version), and

either adding a //Deprecated: line to the package in the package line, and to the functions that are deprecated
or backporting this PR (but larger change of course)

If you're good with tagging as-is, could you create a v0.5.0 tag, @milosgajdos ?

Jamstah · 2023-08-31T09:37:27Z

The only deprecated function in distribution/reference is SplitHostname. It is also deprecated in v2.8.2. I think that means we can just remove the code.

Although I agree we should tag a v0.0.1 (or 0.5.0) first, then remove it.

milosgajdos · 2023-08-31T09:46:04Z

I agree with @Jamstah, let's remove that one pesky func once tagged.

As for this

either adding a //Deprecated: line to the package in the package line, and to the functions that are deprecated
or backporting this PR (but larger change of course)

I am in favour or deprecating instead of another "laborious" backport to the branch which is essentially in maintenance mode 🤷‍♂️

Now going forward, we need to agree on the first tag 😄 because we now have 2 options OR I'm a bit confused.

tag 0.0.1 as is i.e. the current commit onmain
tag 0.5.0 - @thaJeztah is there a reason for this specific tag?

thaJeztah · 2023-08-31T10:13:04Z

is there a reason for this specific tag?

Nah, just something I tend to use; "halfway there!" and it doesn't look as unstable as v0.0.1.

SemVer itself suggests starting at v0.1.0 (allowing the z version to be bumped for fixes (vx.y.z)) as a first tag, but given that this code was already used widely, I think it's not really a v0.1.0.

Jamstah · 2023-08-31T10:39:59Z

I'm happy with either, that one's just to say "This is the exact code that got copied in", and I still think we should immediately remove the deprecated code and tag 1.0.0.

thaJeztah · 2023-08-31T11:02:05Z

Opened #4032 to fix the "deprecation"; looks like that function was already marked deprecated in the 2.8 branch at least (but the format not correct, so not unlikely that linters and IDEs didn't pick it up)

Jamstah · 2023-08-31T12:27:38Z

@milosgajdos I think you're good to pick any version under 1.0.0 and tag the reference repo now.

thaJeztah · 2023-08-31T12:49:56Z

Yup! A tag would be great now, @milosgajdos then I start moving my PRs out of draft

milosgajdos · 2023-08-31T13:19:33Z

Ok pushed v0.5.0 as agreed https://github.com/distribution/reference/releases/tag/v0.5.0

milosgajdos · 2023-08-31T13:20:22Z

Great I realized that git cli is silly and doesn't remove comments when signing tags before pushing 🫠 🤌

thaJeztah · 2023-08-31T13:38:07Z

Awww, that's cute! Perfect for a v0.5.0!

…ence This integrates the new module, which was extracted from this repository at commit b9b1940; # install filter-repo (https://github.com/newren/git-filter-repo/blob/main/INSTALL.md) brew install git-filter-repo # create a temporary clone of docker cd ~/Projects git clone https://github.com/distribution/distribution.git reference cd reference # commit taken from git rev-parse --verify HEAD b9b1940 # remove all code, except for general files, 'reference/', and rename to / git filter-repo \ --path .github/workflows/codeql-analysis.yml \ --path .github/workflows/fossa.yml \ --path .golangci.yml \ --path distribution-logo.svg \ --path CODE-OF-CONDUCT.md \ --path CONTRIBUTING.md \ --path GOVERNANCE.md \ --path README.md \ --path LICENSE \ --path MAINTAINERS \ --path-glob 'reference/*.*' \ --path-rename reference/: # initialize go.mod go mod init github.com/distribution/reference go mod tidy -go=1.20 Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah · 2023-08-31T13:51:43Z

Updated to v0.5.0, and moved out of draft.

I also crated a "release" on the other repository to give a short summary of what it is, and where it came from (feel free to edit); https://github.com/distribution/reference/releases/tag/v0.5.0

Jamstah

Code is all fine, but I would argue for getting reference 1.0.0 up first. If you'd rather merge this that's fine too.

thaJeztah · 2023-08-31T14:12:50Z

I think it's fine to start with v0.5.0, as mentioned earlier; this is the "clean move" of the existing code, whereas v1.0.0 is removing bits from what's moved, so it would also make it more transparent that "something changed" when we move to v1.0.0.

See discussion at distribution/distribution#4031. Tilt wasn't directly using `distribution/distribution`, but this is the extracted reference parts and is what Moby & friends are moving to: https://github.com/moby/moby/blob/20f96354699d20eeb4674d3944486b5d73c256b2/vendor.mod#L37 It's an easy find-replace and should be the exact same code, but this will make sure we get reference fixes and don't break on a future Moby update, for example. Signed-off-by: Milas Bowman <milas.bowman@docker.com>

thaJeztah added impact/changelog dependencies Pull requests that update a dependency file impact/deprecation labels Aug 30, 2023

thaJeztah mentioned this pull request Aug 30, 2023

There is a surprising increase in transitive dependencies when we switch to distribution v3 #3587

Closed

thaJeztah mentioned this pull request Aug 30, 2023

migrate to new github.com/distribution/reference module moby/moby#46376

Merged

thaJeztah mentioned this pull request Aug 30, 2023

replace reference/docker for github.com/distribution/reference v0.5.0 containerd/containerd#9034

Merged

thaJeztah mentioned this pull request Aug 31, 2023

migrate to github.com/distribution/reference docker/compose#10954

Merged

thaJeztah mentioned this pull request Aug 31, 2023

migrate to github.com/distribution/reference compose-spec/compose-go#452

Merged

thaJeztah mentioned this pull request Aug 31, 2023

[release/2.8 backport] reference: replace deprecated function SplitHostname #4032

Merged

thaJeztah force-pushed the migrate_reference branch from 5370753 to 152af63 Compare August 31, 2023 13:48

thaJeztah changed the title ~~deprecate reference package, and migrate to new location~~ deprecate reference package, migrate to github.com/distribution/reference Aug 31, 2023

thaJeztah marked this pull request as ready for review August 31, 2023 13:49

milosgajdos approved these changes Aug 31, 2023

View reviewed changes

milosgajdos requested a review from Jamstah August 31, 2023 13:57

Jamstah approved these changes Aug 31, 2023

View reviewed changes

milosgajdos merged commit db4bd99 into distribution:main Aug 31, 2023

thaJeztah deleted the migrate_reference branch August 31, 2023 14:33

milas mentioned this pull request Sep 14, 2023

vendor: migrate to distribution/reference tilt-dev/tilt#6223

Merged

thaJeztah mentioned this pull request Sep 22, 2023

[release/2.8] deprecate reference package, migrate to github.com/distribution/reference #4063

Merged

This was referenced Oct 19, 2023

reference: fix broken alias for DomainRegexp #4113

Merged

[release/2.8 backport] reference: fix broken alias for DomainRegexp #4114

Merged

thaJeztah mentioned this pull request Jun 8, 2024

[release/1.7 backport] Migrate reference/docker package to github.com/distribution/reference containerd/containerd#10316

Merged

deprecate reference package, migrate to github.com/distribution/reference #4031

deprecate reference package, migrate to github.com/distribution/reference #4031

Uh oh!

Conversation

thaJeztah commented Aug 30, 2023

Uh oh!

codecov-commenter commented Aug 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Jamstah commented Aug 30, 2023

Uh oh!

milosgajdos commented Aug 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Aug 30, 2023

Uh oh!

Jamstah commented Aug 30, 2023

Uh oh!

thaJeztah commented Aug 30, 2023

Uh oh!

thaJeztah commented Aug 30, 2023

Uh oh!

milosgajdos commented Aug 31, 2023

Uh oh!

thaJeztah commented Aug 31, 2023

Uh oh!

Jamstah commented Aug 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

milosgajdos commented Aug 31, 2023

Uh oh!

thaJeztah commented Aug 31, 2023

Uh oh!

Jamstah commented Aug 31, 2023

Uh oh!

thaJeztah commented Aug 31, 2023

Uh oh!

Jamstah commented Aug 31, 2023

Uh oh!

thaJeztah commented Aug 31, 2023

Uh oh!

milosgajdos commented Aug 31, 2023

Uh oh!

milosgajdos commented Aug 31, 2023

Uh oh!

thaJeztah commented Aug 31, 2023

Uh oh!

thaJeztah commented Aug 31, 2023

Uh oh!

Jamstah left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Aug 31, 2023

Uh oh!

Uh oh!

codecov-commenter commented Aug 30, 2023 •

edited

Loading

milosgajdos commented Aug 30, 2023 •

edited

Loading

Jamstah commented Aug 31, 2023 •

edited

Loading