regression: ArArchiveBuilder can no longer handle output to temporary files managed by Python

[amyspark]

Code

I tried this code:

x = ['rustc.EXE', '--print=native-static-libs', '--target', 'x86_64-pc-windows-msvc', '--crate-type', 'staticlib', 'nul', '-o']                                                                    
from tempfile import TemporaryFile()
y = TemporaryFile()  
x += [y.name]
import subprocess 
subprocess.run(x, check=True, text=True)

I expected to see this happen:

note: Link against the following native artifacts when linking against this static library. The order and any duplication can be significant on some platforms.

note: native-static-libs: kernel32.lib advapi32.lib ntdll.lib userenv.lib ws2_32.lib dbghelp.lib /defaultlib:msvcrt

Instead, this happened:

error: failed to build archive at `C:\Users\Amalia\AppData\Local\Temp\tmpk51p4sru`: failed to rename archive file: Acceso denegado. (os error 5)

error: aborting due to 1 previous error

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Program Files\Python310\lib\subprocess.py", line 524, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['rustc.EXE', '--print=native-static-libs', '--target', 'x86_64-pc-windows-msvc', '--crate-type', 'staticlib', 'nul', '-o', 'C:\\Users\\Amalia\\AppData\\Local\\Temp\\tmpk51p4sru']' returned non-zero exit status 1.

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: #122723

Version it worked on

It most recently worked on: 1.77.2

Version with regression

rustc --version --verbose:

rustc 1.82.0 (f6e511eec 2024-10-15)
binary: rustc
commit-hash: f6e511eec7342f59a25f7c0534f1dbea00d01b14
commit-date: 2024-10-15
host: x86_64-pc-windows-msvc
release: 1.82.0
LLVM version: 19.1.1

Backtrace

N/A

@rustbot modify labels: +regression-from-stable-to-stable -regression-untriaged
cc @sdroege @nirbheek @federicomenaquintero

[workingjubilee]

@amyspark #12272 was opened in 2014, are you sure?

[ChrisDenton]

Might have meant #122723

[amyspark]

My bad, copy-pasted monched one character. #122723 is the one.

[ChrisDenton]

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

[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.

[amyspark]

@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.

[the8472]

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

[bjorn3]

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.