babashka / bbin

Install any Babashka script or project with one command
MIT License
139 stars 9 forks source link

Windows support #1

Closed rads closed 2 years ago

rads commented 2 years ago

I'm not aware of any blockers at the moment. After the macOS/Linux implementation solidifies a bit more, I should be able to port the scripts to Windows pretty easily.

rads commented 2 years ago

~This will be more difficult now that the bbin trust and bbin revoke commands exist. These commands require the ~/.bbin/trust directory to be owned by root, which means I'll need to understand how this all works from a Windows perspective.~

The bbin trust and bbin revoke commands were removed so this comment no longer applies.

borkdude commented 2 years ago

We could also ask if @littleli is interested to help out with Windows support.

borkdude commented 2 years ago

And/or @bobisageek

bobisageek commented 2 years ago

Windows gets interesting with respect to file ownership (and even 'sudo'). Typically on Windows, files don't have 'owners' per se, just ACLs with users/groups that have certain permissions. There's msys2, and I believe access to 'admin-y' things through a win-sudo package, but it's not built-in, and would only really apply to running stuff as admin, not file ownership. I assume it wouldn't be considered desirable to add dependencies on those things for Windows users.

The first thing that comes to mind is basically skipping those security checks on Windows, but from a cursory skim, I think it gets a bit more insidious than that, as we'd also need to filter out the chmods (and maybe some other things).

The other things that come to mind right away are:

I might get some time to play with this a bit over the weekend - I would expect that the PRs might be incremental (maybe see how big the changes are to get ls and install/uninstall up before diving into the security-focused commands).

borkdude commented 2 years ago

I discussed with @rads and suggested that the first version should probably not have the sudo / trust mechanism. This can be built later with feedback from real usage. So maybe you can ignore that part for now.

rads commented 2 years ago

As @borkdude mentioned, you can ignore the sudo / trust part right now. Going forward, this project (babashka/bbin) will not have any dependency on sudo or specific user privileges like it does now.

My current plan is to move the trust logic out of this repo this weekend and put it in a rads/bbin-strict repo, wrapping bbin to implement this. If you want to use bbin trust, you will be able to install io.github.rads/bbin-strict instead of io.github.babashka/bbin, but I will make it clear this is not an official Babashka repo, just a side project of mine.

I don't intend to suggest that bbin-strict is an alternative to bbin; it's more of a way for me (and others) to try out experimental stuff and avoid distractions from core features here. For example, I don't think I'd want to implement bbin exec without a security layer, so that will be included as part of my experimental rads/bbin-strict fork, but not the official babashka/bbin repo.

rads commented 2 years ago

The sudo / trust stuff has been removed in v0.0.10: https://github.com/babashka/bbin/blob/main/CHANGELOG.md#0010

bobisageek commented 2 years ago

I got some basic stuff "working" (e.g. making the tests complete with failures but no errors on Windows). Essentially, I'm at the point where install and uninstall 'work', but they're still writing a bash script instead of a Windows batch file (so you can install a thing, but when you try to run it, it explodes because cmd isn't bash). The changes so far are at https://github.com/bobisageek/bbin/tree/windows-start.

Obviously, the script bits are shell-dependent. I was brainstorming some ideas here but wanted some feedback/other perspectives on an approach to organizing the shell-dependent bits:

borkdude commented 2 years ago

@bobisageek Good progress! I'll defer to @rads for the approach. Perhaps it's also possible to do more in a bb wrapper script rather than shell-specific things? bb now supports (process/exec ...) which allows you to do an exec call, so you don't even pay the price of an extra bb process as the parent of the invocation (although that price is fairly small I'd say).

bobisageek commented 2 years ago

Using exec feels much nicer than dealing with the windows shell environment (and I think it's quite in the spirit of babashka).

To dodge the yak shaving about organization for now, I'm building the template strings inline conditionally. I'm also writing a batch file when bbin runs on windows to provide the executable 'feel' - that batch file is a pretty trivial call to a bb script that constructs the 'proper' exec call.

Currently, the existing tests are passing on windows. I haven't translated the tool mode template script yet (and I also want to add some testing around that).

To borkdude's point, if desired, I think we could throw a shebang at the top of the bb-templated script and (with the batch file on Windows), have more or less identical behavior across OSes, and only one set of non-trivial scripts to maintain. One aspect of this I haven't considered is how nicely it would play with the security stuff, so still an open question there.

rads commented 2 years ago

@borkdude @bobisageek: Using babashka.process/exec does seem like a win for portability. The tradeoff would be startup time since we are then calling bb twice — that means we incur the cost of bb plus any BABASHKA_PRELOADS twice.

I'm open to using babashka.process/exec even with the additional startup cost, though I'd like to know a little more about the challenges for the bash scripts. Are there any specific blockers for translating them to Windows? I admit I haven't looked into the specifics of Windows batch scripting too much myself yet.

rads commented 2 years ago

Regarding code organization, I suggest we get things working in the existing namespaces first (so mainly babashka.bbin.scripts). Once we get a first-pass at the Windows version we can look into refactoring the namespaces as needed.

borkdude commented 2 years ago

I suspect people will hardly notice the startup time: on linux it's below 10ms, on macOS around 20ms and Windows: I think somewhere in between. When people start noticing this, we could always optimize this by writing the OS-specific boilerplate.

rads commented 2 years ago

@borkdude: All right, I'm fine moving forward with the bb script approach over Bash/Batch scripts. As you suggested, it's probably a premature optimization to worry about two bb calls right now.

@bobisageek: Feel free to open a PR whenever you're ready with your branch.

rads commented 2 years ago

@bobisageek @borkdude: I've merged in https://github.com/babashka/bbin/pull/22 and released v0.0.11 which means we now have initial Windows support in bbin. Thanks @bobisageek for the PR!

It would be great to get some more Windows users trying this out. I'll announce the new version on Slack to help with this.

I'm going to close this issue since the initial version is working. We can make follow-up issues for more specific things we want to improve about the Windows implementation.

borkdude commented 2 years ago

Woohoo!