marler8997 / zigup

Download and manage zig compilers.
MIT No Attribution
790 stars 61 forks source link

Simplified build, replaced ziget dependency with std.http and added option to download zls #98

Open FabricatorZayac opened 1 year ago

FabricatorZayac commented 1 year ago

I also added a zigup-init.sh script that downloads and installs the latest version of zigup

FabricatorZayac commented 1 year ago

I originally just wanted to just add a zls downloader (kinda like rustup component add rust-analyzer), but then I bumped into an issue with ziget erroring instead of redirecting on code 302 Found. So I replaced it with std.http and it worked

marler8997 commented 1 year ago

Wow you've done alot here!

Note that there's currently an intermittent issue around TLS with std.http which is why I haven't switched to it yet (see https://github.com/marler8997/zigup/pull/75).

Really like that you've made a zigup-init.sh file, that's been on the TODO list for a while. I don't know enough about ZLS and it's use cases to know whether zigup should also be managing it, I'll have to look into that more. I might go through this PR and cherry-pick the commits I can take right away and then maybe we can look into the other parts more.

marler8997 commented 1 year ago

Here's how I'd like to see these changes organized. First, let's get "zig fmt" out of the way (https://github.com/marler8997/zigup/pull/99 done). Here's a list of PR's I would split this into:

There is already a PR for switching to std.http (https://github.com/marler8997/zigup/pull/75). If you feel like it's too much work to organize your changes in this way not a big deal, I can cherry-pick your commits and rebase and do the magic git foo myself, but, I thought I'd let you know what I'd like to see if you're up for it.

FabricatorZayac commented 1 year ago

I'm not super experienced with github PRs, so I am not exactly sure how to split up my changes into multiple PRs. I've only done very minor contributions in the past.

I am just going to look up how to do that

FabricatorZayac commented 1 year ago

As I understand, to do that I have to branch from the old master and cherry pick commits into the new branches and then make PRs from those?

FabricatorZayac commented 1 year ago

One problem I am running into trying to cherry-pick the "remove deprecated names" commits is that I did that alongside removing the ziget dependency and removing the first build step that fetched it. It should probably be separate from other misc changes

FabricatorZayac commented 1 year ago

ZLS installation didn't work with ziget, so that can only be added on top of migrating to std.http. The only change I can fully separate is the zigup-init.sh script

FabricatorZayac commented 1 year ago

Created the zigup-init pr (#100)