axodotdev / cargo-dist

📦 shippable application packaging
https://axodotdev.github.io/cargo-dist/
Apache License 2.0
1.46k stars 64 forks source link

axoupdater probably dumping weird tempdir garbage all over the place on windows #1374

Open Gankra opened 2 weeks ago

Gankra commented 2 weeks ago

Two things building up on my system:

PATH starts with this huge mess

C:\Users\ninte\AppData\Local\Temp\.tmpgnZRgs\bin;C:\Users\ninte\AppData\Local\Temp\.tmpwuPfF4\bin;C:\Users\ninte\AppData\Local\Temp\.tmpEzIVd6\bin;C:\Users\ninte\AppData\Local\Temp\.tmpIAVvIP\bin;C:\Users\ninte\AppData\Local\Temp\.tmp8odYwJ\bin;C:\Users\ninte\AppData\Local\Temp\.tmpxryyGS\bin;C:\Users\ninte\AppData\Local\Temp\.tmpr3cko9\bin............................

and .cargo is filling up with these:

image

Seemingly the raw powershell installer doesn't have this problem, so I'm forced to conclude this is axoupdater's doing.

Gankra commented 2 weeks ago

clarification: this doesn't break anything but is disgusting if you ever notice it

mistydemeo commented 2 weeks ago

Just had a thought. Does a new one of those appear when you cargo test on axoupdater? The tests are probably leaking changes to your real environment, which we should work harder to disable.

It looks like the shell installer respects a INSTALLER_NO_MODIFY_PATH environment variable, but the PowerShell installer doesn't. It'd be worth unifying that behaviour.

Those temporary directories look like the result of binary relocation we do. Specifically, we relocate the running binary to a temporary directory on the same drive so we can then replace it with the new binary at its original path. This directory is supposed to be deleted when the installation completes, but it looks like that must not be working.

mistydemeo commented 2 weeks ago

Just realized what must be happening. It's failing to delete the temporary directory because the running EXE is still in it, so it's leaving the directory and the old version of the EXE in place. This wasn't an issue before when we rooted the temporary directory in the system temp dir, since it would get cleaned up, but after https://github.com/axodotdev/axoupdater/pull/121 the temporary directories are surviving indefinitely.

I feel like I remembered somewhere seeing some kind of Windows rm command that acts after a lock is cleared...

mitsuhiko commented 2 weeks ago

I wonder if you could use self_replace. For very selfish reasons. I use it in a few other places and it would be good to collect all these fixes for windows in one central crate :)