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.