containerd / nerdctl

contaiNERD CTL - Docker-compatible CLI for containerd, with support for Compose, Rootless, eStargz, OCIcrypt, IPFS, ...
Apache License 2.0
7.62k stars 565 forks source link

pkg/defaults: add defaults_darwin.go #3135

Closed sondavidb closed 1 week ago

sondavidb commented 1 week ago

Before this change, people using nerdctl as a library could not build if their dependency tree took pkg/defaults. This adds a dummy file that implements the missing necesssary variables so that it can compile on Mac.

I understand we do not natively support building on Mac, but as this is a library as well as a binary I felt this change might be helpful for a few people.

sondavidb commented 1 week ago

Wonder if we should just add a dummy file in pkg/defaults to avoid this in the future

pendo324 commented 1 week ago

Makes sense to me if the intention of pkg/api/types is to separate all of the non-platform specific code so that its more broadly usable

AkihiroSuda commented 1 week ago

Not sure if this should be in "api".

What fails on macOS?

sondavidb commented 1 week ago

Not sure if this should be in "api".

All the other structs are in pkg/api/types, so this was my reason for putting it here. Do you think it makes less sense here?

What fails on macOS?

If we pull any libraries that requires pkg/defaults down the dependency tree, it will fail with build constraints exclude all Go files in /Users/dummyuser/go/pkg/mod/github.com/containerd/nerdctl/vs@v2.0.0-beta.5/pkg/defaults. On looking closer it seems that the only non-platform specific or binary-related function call to this library is in the config, hence my suggested refactor.

Another thought I had was to just make a dummy file defaults_darwin.go and implement all the necessary functions with a no-op. Would this be preferred and more logical to you? It would also not break any other changes, while the proposed solution could be breaking.

AkihiroSuda commented 1 week ago

api

IIUC, the api package was created for third party projects that import Go packages of nerdctl. So, unless the config is consumed by a third party project, it doesn't make much sense to move the config to the "api" package.

defaults_darwin.go

SGTM

sondavidb commented 1 week ago

Thanks. Changed commit title and PR title + message.