-
Notifications
You must be signed in to change notification settings - Fork 10
RSDK-12870: Properly support file:// for subsystem links.
#167
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
Conversation
| DefaultsFilePath = "/etc/viam-defaults.json" | ||
| CLIDebug = false | ||
| CLIWaitForUpdateCheck = false | ||
| DevMode = false |
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.
driveby
dad8a33 to
4724f02
Compare
| // the case where the file was overwritten in place. Being this update logic is run | ||
| // every five seconds, it's common to catch a binary while its being | ||
| // compiled/written to. | ||
| data.brokenTarget = 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.
This brokenTarget logic I changed. The "re-check subsystems to see if things changed" happens every 5 seconds. I was routinely hitting the window where a viam-server recompile removed the old file, but had not yet written out the new one.
| // TODO handle compressed formats, for now, the raw download is the same file | ||
| verData.UnpackedPath = verData.DlPath | ||
| verData.DlSHA = actualSha | ||
| actualSha, err := utils.GetFileSum(verData.DlPath) |
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.
Everything else here should be exactly the same except for the level of indentation.
I've misread this code -- trying again
| //nolint:gosec | ||
| in, err := os.Open(filepath) | ||
| func GetFileSum(path string) (outSum []byte, errRet error) { | ||
| resolvedPath, err := filepath.EvalSymlinks(path) |
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.
so we were just getting the checksum of the symlink before? and that's why file:// links weren't triggering a checksum mismatch and redownload?
|
This doesn't work. There are four pieces of state that are important:
But this code obfuscates which variables relate to which state. I also think it assumes the version cache file is the source of truth (as opposed to what's in the cache directory). And has no notion of cache invalidation. This all to say, I think the code needs a bigger rewrite. Maybe a later attempt will yield a better result. |
No description provided.