tristanisham / zvm

zvm (Zig Version Manager) lets you easily install/upgrade between different versions of Zig.
https://www.zvm.app
MIT License
484 stars 32 forks source link

FIX #71: old symlink should be removed on Windows #72

Closed guidoschmidt closed 7 months ago

guidoschmidt commented 7 months ago

Seems to be a longer journey 😅 ...

just tested version 0.6.2 and unfortunately the old symlink to .zvm/bin is not removed, the change to !errors.Is(err, os.ErrNotExist) seems to fix it. found os.Lstat which checks symbolic links.

Maybe there's a more appropriate way to do this? Looks like there's a newer fs package that can be used for these kind of filesystem checks.

tristanisham commented 7 months ago

Thanks for making another PR. These system specific bugs are always a pain but we'll fix this one. I'll be on my Windows developer environment tonight so I can work on this bug directly.

guidoschmidt commented 7 months ago

Oh nice, hope it doesn't give you too much of a headache

tristanisham commented 7 months ago

New bug unlocked. Totally breaks the progress bar when running in command prompt. image

tristanisham commented 7 months ago

@guidoschmidt could you please test the changes I've pushed to the PR. For I modified the Symlink function again because for users who are already admins or have enabled symlinks for regular users it was uneccessary and degraded the experience. I also tweaked the Lstat to use the err == nil method since just parsing the errors wasn't working for us.

tristanisham commented 7 months ago

Added a test for our symlink code. It passes so that bit in setBin should be good.

func TestSymlinkExists(t *testing.T) {
    if err := os.Symlink("use_test.go", "symlink.test"); err != nil {
        t.Error(err)
    }

    stat, err := os.Lstat("symlink.test");
    if err != nil {
        t.Errorf("%q: %s", err, stat.Name())
    }

    defer os.Remove("symlink.test")
}