Skip to content

Conversation

@dmcgowan
Copy link
Member

@dmcgowan dmcgowan commented Sep 6, 2023

The "Where to put packages" section at the end is proposing the layout for packages for the containerd 2.0 release. While this may seem like a large breaking change, it will align with the go module /v2 change we already need to make.

Example package renames required by importers of v2
github.com/containerd/containerd -> github.com/containerd/containerd/v2/client
github.com/containerd/containerd/archive -> github.com/containerd/containerd/v2/pkg/archive
github.com/containerd/containerd/sys -> github.com/containerd/containerd/v2/pkg/sys

Actual changes to move packages and/or create new go submodules will be in follow up. This is also an opportunity to split out some packages into their own repository when it makes sense (this has been proposed for log).

@dmcgowan dmcgowan added the status/needs-discussion Needs discussion and decision from maintainers label Sep 6, 2023
Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

should probably resolve this against the https://github.com/containerd/containerd#getting-started path so we don't have two different entry points.. one that skips over this one, let's add the link to this contributors guide there ^

Copy link
Member

@laurazard laurazard left a comment

Choose a reason for hiding this comment

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

LGTM, left a nit re: file name agreeing with what @AkihiroSuda said

@dmcgowan dmcgowan force-pushed the add-contributors-guide branch from 04387ec to 0e66697 Compare September 7, 2023 23:42
@dmcgowan
Copy link
Member Author

dmcgowan commented Sep 7, 2023

Updated

@mikebrow yeah our getting started flow and website makes me cringe a little bit. I added a link, but we really need some help setting up a more unified experience for getting started.

@akhilerm
Copy link
Member

akhilerm commented Sep 8, 2023

This is also an opportunity to split out some packages into their own repository when it makes sense (this has been proposed for log).

@dmcgowan This doesnt include splitting the api package right? The one which we planned to do via automatic syncing.

Signed-off-by: Derek McGowan <derek@mcg.dev>
Replace link to BUILDING since CONTRIBUTING already points to it

Signed-off-by: Derek McGowan <derek@mcg.dev>
@dmcgowan dmcgowan force-pushed the add-contributors-guide branch from 0e66697 to 1a7490c Compare September 8, 2023 18:23
@thaJeztah
Copy link
Member

The "Where to put packages" section at the end is proposing the layout for packages for the containerd 2.0 release. While this may seem like a large breaking change, it will align with the go module /v2 change we already need to make.

Example package renames required by importers of v2
github.com/containerd/containerd -> github.com/containerd/containerd/v2/client

Oh, just asking; was your intent with those examples to put packages in a v2 subdirectory ? Because there's two "options"

  • Use go's invented alternative to git branches; instead of using branches, you copy all the code to a v2 directory, and maintain all code of all versions in a single branch effectively (v2 and v1 live in the same branch, but "v2" contains code for v2.x.x)
  • Or, just tag as v2.0.0 but rename the module (and v1 continues to live in the release branches for v1)

The "use a subdirectory" approach was also for compatibility with old dependency tools.

When moving to v2, you're renaming the module, or to put it in other words "v2" and "v1" are completely separate modules, separate projects, no relation whatsoever (their names may just look similar).
And even with the "v2 subdirectory" approach, these are separate modules (who happens to live in tthe same branch).

It's possible to associate them, but in that case it would require aliases to be provided, so v1 must depend on v2, and make the v1 types aliases for v2.

package foo // this is "foo" in the v1 module


// Import the v2 module, which will be code that's vendored
// so the vendor directory will have a copy of containerd from
// a v2 tag.
//
// go.mod will have something like:
//
// require github.com/containerd/containerd/v2 v2.0.0-alpha.0
import "github.com/containerd/containerd/v2/foo"

// Hello is the v1 alias for v2.
type Hello = foo.Hello

I created a quick example repository with that situation; look at the commit (and tag) history to get a bit of an idea what the flow was for that.

Perhaps there's other options possible; the biggest challenge is how to match signatures. The alternative could be to have v1 -> v2 adaptor code live in the v2 version (to type-cast v1 types to v2)

@thaJeztah
Copy link
Member

And of course I didn't post a link to my example repository; here it is; https://github.com/thaJeztah/mod-migrate

@dmcgowan
Copy link
Member Author

dmcgowan commented Sep 8, 2023

Or, just tag as v2.0.0 but rename the module (and v1 continues to live in the release branches for v1)

Was the approach I was thinking based on the previous attempt to rename the module. We already have release branches for maintaining the "v1" module. My understanding is there is no clean way to avoid "/v2" in our import paths once we create the "v2.0.0" tag.

@dmcgowan
Copy link
Member Author

dmcgowan commented Sep 8, 2023

@dcantah Cleaned up the BUILDING and CONTRIBUTING links

I think we can move the v2 module discussion to that PR for now. We know we need it but still figuring out the how.

@thaJeztah
Copy link
Member

Ah, yes, sorry for somewhat derailing this PR (and we should / can definitely pick up that discussion elsewhere). I thought for a bit that the intent of that comment was "going forward, create a v2 directory, and put your new things under that".

@dmcgowan and I caught up a bit on Slack, and discussed some of the issues at hand (lots of further brainstorming will still be needed though!)

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@dmcgowan dmcgowan requested a review from fuweid September 8, 2023 22:10
Copy link
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

LGTM

@dmcgowan dmcgowan removed the status/needs-discussion Needs discussion and decision from maintainers label Sep 12, 2023
Copy link
Member

@laurazard laurazard left a comment

Choose a reason for hiding this comment

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

LGTM

@AkihiroSuda AkihiroSuda merged commit c07cadd into containerd:main Sep 12, 2023
@dmcgowan dmcgowan deleted the add-contributors-guide branch September 12, 2023 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.