-
Notifications
You must be signed in to change notification settings - Fork 346
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: implement self-update for nali #177
Conversation
b38f115
to
cc570ef
Compare
Our binary increased by 40% due to the introduction of a lot of non-essential code, which could have been avoided. We can no longer use The logic here is simple:
|
d300edc
to
8999671
Compare
issue: 171
internal/repo/github.go
Outdated
func download(ctx context.Context, assetId int64) (data []byte, err error) { | ||
var rc io.ReadCloser | ||
|
||
rc, _, err = client.Repositories.DownloadReleaseAsset(ctx, constant.Owner, constant.Repo, assetId, http.DefaultClient) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not use the default http client, which has no timeout control and does not use proxies.
See
Line 30 in f91569c
func GetHttpClient() *HttpClient { |
internal/repo/github.go
Outdated
hash := fmt.Sprintf("%s", vData[:sha256.BlockSize]) | ||
calculatedHash := fmt.Sprintf("%x", sha256.Sum256(data)) | ||
|
||
if calculatedHash != hash { | ||
return fmt.Errorf("expected %q, found %q: sha256 validation failed", hash, calculatedHash) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of converting bytes to a string (and you're not dealing with case issues), decode the hexadecimal sha256 and then use
https://pkg.go.dev/bytes#Equal
internal/repo/update.go
Outdated
|
||
return latest.GreaterThan(cur) | ||
} | ||
return true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unknown version
means that the user compiled it manually instead of downloading it from the release, in which case we don't take the liberty of updating it to a potentially older version.
return true | |
return false |
internal/repo/update.go
Outdated
// get the directory the executable exists in | ||
updateDir := filepath.Dir(cmdPath) | ||
filename := filepath.Base(cmdPath) | ||
|
||
// Copy the contents of new binary to a new executable file | ||
newPath := filepath.Join(updateDir, fmt.Sprintf(".%s.new", filename)) | ||
fp, err := os.OpenFile(newPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0755) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is necessary to check the permissions of folders and files before updating, because some of our users install nali through package management, which results in no write permissions, in which case we give a friendly reminder to the user to inform about the latest version and skip the automatic update.
internal/repo/update.go
Outdated
} | ||
|
||
if _, err = io.Copy(fp, bytes.NewReader(newBytes)); err != nil { | ||
fp.Close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fp.Close() | |
_ = fp.Close() |
internal/repo/update.go
Outdated
fp, err := os.OpenFile(newPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0755) | ||
|
||
if err != nil { | ||
fp.Close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fp.Close() | |
_ = fp.Close() |
internal/repo/update.go
Outdated
} | ||
// if we don't call fp.Close(), windows won't let us move the new executable | ||
// because the file will still be "in use" | ||
fp.Close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should handle the error here, although the probability of the error here is extremely low, we need to make sure that we don't cause file corruption here, and if an error occurs we need to cancel the update process
func canWriteFile(path string) bool { | ||
f, err := os.OpenFile(path, os.O_WRONLY, 0644) | ||
if err == nil { | ||
defer f.Close() |
Check warning
Code scanning / CodeQL
Writable file handle closed without error handling Warning
call to OpenFile
LGTM |
Thanks |
implemented self-update for nali, used creativeprojects/go-selfupdate.
locally tested, this is the log:
issue: #171