-
Notifications
You must be signed in to change notification settings - Fork 370
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
Question on bin files without .exe suffix #131
Comments
/remove-lifecycle stale |
I think for windows, having a symlink that ends in ".exe" should be fine. We don't follow symlinks to check the original file's extension. This being said, I'll try to double check that this works on windows |
Yeah the part I am not sure is whether execve equivalent of Windows actually runs files without an executable suffix, my guess would be not. |
/remove-lifecycle stale |
/label good-first-issue |
yeah the /label is failing us on default labels that aren't tracked by the test-infra here. |
Just had the time to verify this on windows: The target of the symlink (the file it's pointing to) does not have to have an executable suffix (like So the value in the That said, since we add .exe to every executable, if your plugin is actually not a win32 application, it will not work. For example, plugins that have a .ps1 or .bat extension currently don't work on Krew. I'm thinking possibly we need to preserve the file extension based on the |
So to clarify, if a plugin is not a win32 application, we need to install a symlink with a correct file extension, such as Do you happen to know a good way to determine the proper file extension for a given file on windows? Then this could easily be done without relying on plugin authors to do it correctly. I'm a bit worried that if we have to verify this manually, it will be difficult for plugin authors and reviewers alike to get it right. |
The only clean solution that comes to my mind is that
Thoughts? |
It's not pretty, but it will do the job. Plugins with non-exe endings are probably rare exceptions anyway, in particular their architecture will be limited to just windows. So we can somewhat rely on the plugin authors to have verified that it works on windows machines. I'd add the correct file extension to all existing manifests, though. Then I would add the hard validation for the bin fields of windows plugins, otherwise we would need to keep that in mind when reviewing manifests in the future. Seems cleaner and won't be too much work with the current number of plugins. |
Actually I was surprised one of the PRs in krew-index mentioned that Windows’s WSL support can actually support non .exe files (e.g. bash scripts). kubernetes-sigs/krew-index#208 (comment) I'm not wondering what sort of trickery is this. I expect WSL to return GOOS=linux as it's a linux emulation of some sort. In that case, windows platform spec should not be matching. |
I've tried this on windows. Here are some findings. On Windows:
Not sure where/if we can document this. The only thing to know out of this is, the value of /lifecycle frozen |
I think we should document somewhere that only .exe extensions are allowed for windows and don't support the rest. |
Or should we support either |
@smaftoul Support request for ps1/bat files has not come up at all despite 115 of 181 plugins (63%) distribute on Windows. I'm inclined to think this is not necessary and there's not enough proof that this is required. Most plugins are implemented in Go and bash, and for Windows support, people already opt in to use Go, and I'm yet to see a plugin developed in PS, so I think this is a wontfix for the time being. The task here is probably just to document that only ".exe" entrypoints work on windows somewhere on https://krew.sigs.k8s.io/docs/developer-guide/plugin-manifest/. |
If a plugin's windows
platform
spec specifies something likebin: foo
, we:foo
I am not at all sure that
kubectl-foo.exe
which points tofoo
(that has no .exe extension) can be executed. Can someone verify if this works or not?Depending on that, (1) we need to make sure if a platform spec matches
windows
, itsbin
must end with ".exe" (2) we need to update existing plugin specs in krew-index.The text was updated successfully, but these errors were encountered: