-
Notifications
You must be signed in to change notification settings - Fork 614
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
Windows part 1 #184
Windows part 1 #184
Conversation
@@ -273,7 +272,7 @@ func runAction(clicontext *cli.Context) error { | |||
oci.WithMounts([]specs.Mount{ | |||
{Type: "cgroup", Source: "cgroup", Destination: "/sys/fs/cgroup", Options: []string{"ro", "nosuid", "noexec", "nodev"}}, | |||
}), | |||
oci.WithoutRunMount, // unmount default tmpfs on "/run": https://github.com/containerd/nerdctl/issues/157 | |||
WithoutRunMount(), // unmount default tmpfs on "/run": https://github.com/containerd/nerdctl/issues/157 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just if runtime.GOOS == "linux" { opts = append(opts, oci.WithoutRunMount) }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did it this way becuase oci.WithoutRunMount
is in a Linux tagged file so it fails when building. 😞
Thanks, could you squash commits |
a64bb8f
to
552f67c
Compare
552f67c
to
940d337
Compare
func appBashComplete(clicontext *cli.Context) { | ||
return | ||
} | ||
|
||
func bashCompleteNamespaceNames(clicontext *cli.Context) { | ||
return | ||
} | ||
|
||
func bashCompleteSnapshotterNames(clicontext *cli.Context) { | ||
return | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to disable bash completion output on Windows builds? Or is this due to lack of Windows support in the nerdctl packages the bash-completion functions use?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These functions wouldn't compile due to assumption for Linux. I didn't think that bash completion would function Windows so left them blank.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair 'nuff for now. We can come back to it later, if ever. Most of my team uses bash on Window as their default shell (which surprised me when I joined them, and brings a bunch of its own issues), but without this PR, we don't have bash completion output on Windows anyway, so it's not a regression.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of my team uses bash on Window as their default shell
WSL bash? Cygwin bash? MinGW bash?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Git Bash, I believe, so Cygwin-derived via MSYS2, but has a different env-var for disabling /
-path conversion.
940d337
to
eb0f8a3
Compare
LGTM except CI failure, thanks! |
Signed-off-by: James Sturtevant <[email protected]>
eb0f8a3
to
ffd9d45
Compare
This is the first part step of adding Windows support. See #28 (comment) for the plan to do this in stages. This is focused on make it build and mostly move Linux specific functionality to
_linux.go
files and stubs in_windows.go
files. The next PR will add ability tonerdctl run
containersHaving all the changes in one PR (#164) made it difficult to see where differences were introduced and felt riskier than doing it in stages and should make reviewing faster.
This only enables Nerdctl to be built on Windows. It doesn't add full support, which will come in later PR's. Some functionality does work though, such as :
Pulling images
listing images
listing running containers
listing namespaces: