Skip to content
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

Fix issues updating Windows binaries on non-C: drives. #69

Merged
merged 3 commits into from
Feb 24, 2021
Merged

Fix issues updating Windows binaries on non-C: drives. #69

merged 3 commits into from
Feb 24, 2021

Conversation

t-mw
Copy link
Contributor

@t-mw t-mw commented Feb 20, 2021

Fixes #67.

I'm not sure why TEMP is required on Windows, but it might've been to force the temp file to be cleaned up by the OS. The tempfile crate should have been doing this even outside the TEMP directory, but couldn't because the running executable was still referencing the temp file at the time that tempfile::TempDir was dropped. I've tested on Windows and this PR seems to fix those issues, and fs::rename(self.source, dest); still works when overwriting the file of the running executable.

@jaemk jaemk merged commit 96d7171 into jaemk:master Feb 24, 2021
@t-mw t-mw deleted the patch-3 branch February 24, 2021 01:31
@jaemk
Copy link
Owner

jaemk commented Feb 24, 2021

@t-mw after this change, running the example on my windows machine is giving me a permissions error

cargo run --example github --features "archive-tar archive-zip compression-flate2 compression-zip-deflate"

... snip ...

Do you want to continue? [Y/n] Y
Downloading...
[00:00:00] [========================================] 1.09MB/1.09MB (0s) Done
Extracting archive... Done
Replacing binary file... [ERROR] IoError: Access is denied. (os error 5)
error: process didn't exit successfully: target\debug\examples\github.exe (exit code: 1)

Any ideas?

@jaemk
Copy link
Owner

jaemk commented Feb 24, 2021

Looks like it's failing on the fs::rename(temp, dest)?; for some reason

@jaemk
Copy link
Owner

jaemk commented Feb 24, 2021

interestingly, it works if I revert only the first copy back to a rename

@jaemk
Copy link
Owner

jaemk commented Feb 24, 2021

seems like the removal of env::var_os("TEMP") was the real fix, so I'm going to revert rename->copy change before I do a release

@t-mw
Copy link
Contributor Author

t-mw commented Feb 24, 2021

Sorry, I see the same error and thought I'd fixed it after testing with my own project. Reverting to rename is fine, but it prevents the temporary file from being cleaned up. I think TEMP was being used as a workaround, but that creates the issues with not being able to update on non-C: drives.

@jaemk
Copy link
Owner

jaemk commented Feb 24, 2021

released in 0.25.0

@t-mw
Copy link
Contributor Author

t-mw commented Feb 24, 2021

Thanks. Not ideal that those temp folders build up on Windows now. Reading around it seems like the only way to avoid that is to delete them when the updated executable runs. I could work on that if that sounds ok.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update on windows produces error when exe is not on C drive
2 participants