Fix issue with upgrading on Windows #646
Conversation
This changes ensures that the upgrade process does not try to replace the exercism binary with a non executable file. There is probably a more extensive check we can do to ensure we have the correct file. I did not choose to use the full name as Configlet is also using the cli pkg for upgrading its binary.
| @@ -164,6 +164,10 @@ func extractBinary(source *bytes.Reader, os string) (binary io.ReadCloser, err e | |||
| } | |||
|
|
|||
| for _, f := range zr.File { | |||
| info := f.FileInfo() | |||
| if info.IsDir() || !strings.HasSuffix(f.Name, ".exe") { | |||
There was a problem hiding this comment.
Why iterating over all files?
Why not just extract exercism.exe?
There was a problem hiding this comment.
Why iterating over all files?
I don't want to assume that the first file is the executable, even though that is probably the case.
Why not just extract exercism.exe?
If I extract the whole zip I would have to create the structure locally, then write each of the files. I am essentially extracting the executable without writing the file to disk (less to clean up). I could check for the exact name, read it's contents, then return in case we decide to add more binaries to the release package.
There was a problem hiding this comment.
Makes totally sense, thanks for the explanation.
There was a problem hiding this comment.
Anytime! Thanks for the question and review.
|
@kytrinyx For future reference, am I okay to cut releases again? During the mentorcism releases I think you were doing something custom. But now it should be a standard release, right? |
Yes! You're good to cut releases. For mentorcism I was changing the default base URL and then calling |
|
Thanks for cutting the release and info on mentorcism. If the situation araises again I cut any neccessary releases to help get fixes out and lighten your load 😉 |
This changes ensures that the upgrade process does not try to replace
the exercism binary with a non executable file. There is probably a more
extensive check we can do to ensure we have the correct file. I did not
choose to use the full name as Configlet is also using the cli pkg for
upgrading its binary.
closes #645