Open amyspark opened 1 day ago
@amyspark https://github.com/rust-lang/rust/pull/12272 was opened in 2014, are you sure?
Might have meant https://github.com/rust-lang/rust/pull/122723
My bad, copy-pasted monched one character. #122723 is the one.
My suggestion would be to essentially revert it for Windows only (i.e. use archive_tmpfile.persist
). The reasoning for that PR is UNIX specific. We could duplicate some of the tempfile Windows-specific code instead but that seems redundant.
cc @bjorn3
Why is the temporary directory created in %LocalAppData%\Temp
in the first place? We are specifying the output directory as the directory to place the temporary directory in, precisely to avoid cross-filesystem renames. It needs to be a rename rather than a copy either way to prevent another rustc instance from observing a partially written file.
@bjorn3 did you mean my snippet? Python's tempfile
uses whatever folder has been specified in the TEMP
environment variable. For instance, had I run it in our (GStreamer's) Cerbero repository, because we use the MSYS2 shell, it'd have been created in <install folder of MSYS2>\tmp
.
The issue here is that you can rename open files in Linux (I think it's because you can always keep references to a deleted inode), whereas this is not possible on Windows, at least with Python's way of opening the handle. This did not happen previously because the file can still be written to.
Python's tempfile uses whatever folder has been specified in the TEMP environment variable.
It has an optional Dir argument: https://docs.python.org/3/library/tempfile.html#tempfile.TemporaryFile
I missed your python snippet. I assumed what happened was: You tell rustc to output somewhere on a filesystem other than C:. Rustc decides to create a temporary directory in C: and then fails to do a cross-filesystem rename, in which case the step where rustc decides to create a temporary directory in C: shouldn't happen. I now see that the issue is with the output file you specified currently being open by another process, which on Windows would require either renaming the existing file before renaming the temp file to the specified output path or overwriting the existing output file. Before my PR the latter was presumably done. This is not correct behavior however as it would make it possible for other rustc invocations to accidentally see partially written output, which can cause corruption of their output or crashes. As such renaming the old file on windows or declaring that the python snippet you wrote should not work (and you need to either close the temp file first without deleting it before calling rustc or open it with the flag that allows deletion while it is still open) are the only acceptable options I think.
Code
I tried this code:
I expected to see this happen:
Instead, this happened:
This breaks librsvg building on MSVC, because I use this technique to query the
native-static-libs
for the Meson dependency setup.I traced the issue to the following pull request: https://github.com/rust-lang/rust/pull/122723
Version it worked on
It most recently worked on: 1.77.2
Version with regression
rustc --version --verbose
:Backtrace
N/A
@rustbot modify labels: +regression-from-stable-to-stable -regression-untriaged cc @sdroege @nirbheek @federicomenaquintero