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

pkg/containerfs: move to internal #48097

Merged
merged 3 commits into from
Jul 1, 2024

Conversation

thaJeztah
Copy link
Member

pkg/containerfs: remove CleanScopedPath and make it internal

The container package is the only consumer of this function in our code
and there's no known external users;
https://grep.app/search?q=.CleanScopedPath%28&filter[lang][0]=Go

pkg/containerfs: cleanup GoDoc, and make Windows a proper wrapper

  • Improve some GoDoc to use docs links
  • Change the Windows stub to an actual wrapper function, as we don't
    want it to be updateable, and it currently shows as "variable" on
    pkg.go.dev, which is confusing.
  • Remove "import" comments in preparation of moving this package

pkg/containerfs: move to internal

The only external consumer are the graphdriver and graphdriver/shim
packages in github.com/docker/go-plugins-helpers, which depended on
ContainerFS, which was removed in 9ce2b30.

graphdriver-plugins were deprecated in 6da604a,
and support for them removed in 555dac5,
so removing this should not be an issue.

Ideally this package would've been moved inside daemon/internal, but it's used
by the daemon (cleanupContainer), plugin package, and by graphdrivers,
so needs to be in the top-level internal/ package.

- Description for the changelog

pkg/containerfs: move to internal

- A picture of a cute animal (not mandatory but encouraged)

The container package is the only consumer of this function in our code
and there's no known external users;
https://grep.app/search?q=.CleanScopedPath%28&filter[lang][0]=Go

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah added area/storage status/2-code-review kind/refactor PR's that refactor, or clean-up code area/go-sdk Changes affecting the Go SDK impact/go-sdk Noteworthy (compatibility changes) in the Go SDK labels Jun 30, 2024
@thaJeztah thaJeztah added this to the 28.0.0 milestone Jun 30, 2024
@thaJeztah thaJeztah self-assigned this Jun 30, 2024
@thaJeztah thaJeztah requested a review from cpuguy83 as a code owner June 30, 2024 16:51
@thaJeztah thaJeztah mentioned this pull request Jun 30, 2024
74 tasks
- Improve some GoDoc to use docs links
- Change the Windows stub to an actual wrapper function, as we don't
  want it to be updateable, and it currently shows as "variable" on
  pkg.go.dev, which is confusing.
- Remove "import" comments in preparation of moving this package

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The only external consumer are the `graphdriver` and `graphdriver/shim`
packages in github.com/docker/go-plugins-helpers, which depended on
[ContainerFS][1], which was removed in 9ce2b30.

graphdriver-plugins were deprecated in 6da604a,
and support for them removed in 555dac5,
so removing this should not be an issue.

Ideally this package would've been moved inside `daemon/internal`, but it's used
by the `daemon` (cleanupContainer), `plugin` package, and by `graphdrivers`,
so needs to be in the top-level `internal/` package.

[1]: https://github.com/docker/go-plugins-helpers/blob/6eecb7beb65124bb44a23848bb46e98b4f50ae18/graphdriver/api.go#L218

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the internalize_containerfs branch from e89c952 to f2970e5 Compare June 30, 2024 17:14
@cpuguy83 cpuguy83 merged commit f3d377e into moby:master Jul 1, 2024
126 checks passed
@thaJeztah thaJeztah deleted the internalize_containerfs branch July 1, 2024 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/go-sdk Changes affecting the Go SDK area/storage impact/go-sdk Noteworthy (compatibility changes) in the Go SDK kind/refactor PR's that refactor, or clean-up code status/2-code-review
Projects
Development

Successfully merging this pull request may close these issues.

3 participants