stepchowfun / toast

Containerize your development and continuous integration environments. 🥂
Other
1.57k stars 39 forks source link

Adding a PowerShell installation script #433

Open AntoniosBarotsis opened 2 years ago

AntoniosBarotsis commented 2 years ago

Description

Adding a PowerShell installation script would make installation easier for windows users over the current very manual installation.

I have some experience with PowerShell but I have never written something like this before. I've made a first draft so if this issue gets a green flag I can start working on a pull request however I would really appreciate it if someone more experienced than me in PowerShell would review this :)

stepchowfun commented 2 years ago

Hi @AntoniosBarotsis, thanks for filing this issue! This seems like it would be a nice thing to have.

Unfortunately I don't have much PowerShell experience, and most of my Windows knowledge is outdated by about a decade. So I probably wouldn't feel very confident reviewing a PR for this. I don't run Windows, except in rare situations such as when I was writing the (manual, as you noted) Windows installation instructions in the first place.

I wish we had some Windows experts who we could consult for this, but for now unfortunately I'm the bottleneck here. So I don't want to make any promises regarding my ability to accept a pull request here.

I'm happy to discuss solutions here and even take a look at any pull requests, but I don't want to get your hopes up. There's a good chance I will just be too out of my depth and will be unable to move forward on it.

AntoniosBarotsis commented 2 years ago

Thanks for the reply! Just pushed the file. Honestly, if you've ever written PowerShell you should be able to read the important parts of it so that should be fine, nothing complicated is going on in the file. I was more concerned about potential bad practices I may be using.

I have run it locally (although incrementally as I was finishing it up) and it seems to be working fine. It will always download the latest available version which might be something you want changed although from what I see that is what your install.sh does as well.

Let me know if you have any questions

stepchowfun commented 2 years ago

Honestly, if you've ever written PowerShell you should be able to read the important parts of it so that should be fine

Well, as I mentioned, I don't really know PowerShell. I know that might be a frustrating reason for my reluctance about this change, but it's the truth. So I can't reach the same conclusion that it "should be fine"—even if it probably is.

I have questions about almost every line in the script. For example, when I see set-location $PSScriptRoot, I'm immediately wondering if $PSScriptRoot should be quoted to handle spaces (as would be needed in Bash). I think the answer is "no", but that's only after a few minutes of Googling around (and I'm still not 100% sure). Furthermore, I don't know the implications of UAC, and if that gives the whole script administrator permissions, that seems like too much to me. Those are some examples of questions that I have, but listing all my questions would be an exercise in itself.

So basically it would take me some effort to review this PR, and that effort would be essentially equivalent to just figuring it out myself (e.g., exploring all the command-line flags to make sure they're doing what we want). I find that this is often the case with pull requests, unless they are simple changes like small bugfixes, typo fixes, etc. Also the README would need to be updated to tell users how to use this script (is it even simpler than just downloading the binary from the releases page and adding it to your PATH?), we should add a line break at the end of the file for consistency with every other file in the repo, etc. Also while the script does match the behavior of the Linux/macOS one in that it downloads the latest version, it doesn't have feature parity with the existing installer script (e.g., the ability to explicitly pin to a specific version which is useful for reproducibility).

The unfortunate reality is that, while I think this might be a good contribution, I don't think I'm up for putting in the work to validate it and playing ping pong with you to implement whatever nitpicky changes I ask for. I know this is disappointing, and I hope my first message above set expectations appropriately. I hope I haven't wasted too much of your time.

AntoniosBarotsis commented 2 years ago

That's understandable, I'll just reply to the points you made in case this gets reviewed sometime in the future.

Honestly, if this (understandably) never gets merged I still had my bit of fun writing it and learned a few things on the way. Thanks for the replies!

If I may add to this, there are a few package managers for windows such as Chocolatey and Scoop (which I haven't used so not sure how that works). They handle paths and installations for you if you have the patience to decipher their docs. I am pretty sure that you could make a package that is just the exe file.

stepchowfun commented 2 years ago

Honestly, if this (understandably) never gets merged I still had my bit of fun writing it and learned a few things on the way.

😄

If I may add to this, there are a few package managers for windows such as Chocolatey and Scoop (which I haven't used so not sure how that works).

Yeah, I think that's another part of my hesitation here. I don't really know what Windows users expect when installing software. E.g., what is the most popular way to install software on Windows? Is creating a PowerShell script the right way to go?

I think I'd need to understand the landscape better before investing more in any particular solution (beyond just asking users to just download the binary manually).

I know this might not be the best outcome for your change, but I hope this gives you and any passersby a bit of confidence in Toast after seeing how persnickety I am about what goes into it and my requirement that I can vet everything that the code is doing. And now everyone knows how ignorant I am about Windows, too. 😬