feat: add tier-1 platform support for change_time

[juliusl]

fix: remove en-us from doc links to support globalization

Related: #121478
r? tgross35

[juliusl]

@tgross35 unfortunately there are still cases where None would be returned, but I updated the documentation to clarify when that is the case

[juliusl]

Note: Currently the doc-url I link to that explains ChangeTime is dev blog. I have a PR out to update the official docs so that it reflects the information from there MicrosoftDocs/sdk-api#1863

[tgross35]

Suggested change

	/// [`ChangeTime`], is the last time file metadata was changed, such as
	/// [`ChangeTime`] is the last time file metadata was changed, such as

Spurious comma

[tgross35]

Looks like this has some highlighting, https://devblogs.microsoft.com/oldnewthing/20100709-00/?p=13463 seems to work

[tgross35]

I don't know what the failure conditions are here, is accessing FILE_BASIC_INFO expected to never fail? Looks like uwp already does this.

If failures are possible, you can just do self.basic_info().ok().map(|info| c::FILETIME { ... }).

[tgross35]

Cool, thanks! The above looks reasonable to me with the possible failure concern addressed.

Since this works on all platforms, is a test possible now?

[ChrisDenton]

Hm, this adds an extra sys call for every use of metadata? I'd rather avoid that, especially as change time is more niche. Multiple sys calls were done for UWP targets out of necessity but for our tier 1 platforms we do try to be performant.

[tgross35]

Per usual with Windows

r? @ChrisDenton

[juliusl]

@ChrisDenton

Hm, this adds an extra sys call for every use of metadata?

Yeah, I thought about that over the weekend, going to do a bit more tinkering to see if I can find a way to avoid an extra call.

Alternatively, how do you feel about an extended_metadata() api so that the perf cost is opt-in?

[juliusl]

Okay wow this was a trip to research. So apparently cpython ran into a similar issue which eventually led windows to add this api:

https://learn.microsoft.com/windows/win32/api/winbase/nf-winbase-getfileinformationbyname

This API can return an information class type that returns a struct called FILE_STAT_BASIC_INFORMATION. This struct has all of the previous information, plus the ChangeTime I'm trying to get to.

Here's the real cool part. According to documentation, this api doesn't open the file in order to return metadata, which means that it should actually improve performance along this code path since GetFileInformationByHandle/GetFileInformationByHandleEx actually need to open/close a file handle.

This routine returns the requested information about the file specified by FileName. It does so without opening and returning a handle to the file.

Also, the struct includes the reparse tag if set, meaning the current additional syscall can be avoided when the reparse attribute is set.

For now, I'll try it out and see if I can get it working and I'll update the PR accordingly.

[juliusl]

So slightly blocked by this PR getting merged microsoft/win32metadata#1934 in order for bindings.txt/windows-bindgen to pick up the new API. (https://github.com/microsoft/win32metadata/blob/3b275fdb05e7dbab989c00b8a86e985853c6b65e/generation/WinSDK/RecompiledIdlHeaders/um/WinBase.h#L9394)

However, I confirmed that the API already exists in winbase.h, by just running my own bindgen, which means I could at least test the functionality w/ this Draft PR.

[juliusl]

Additional prior art -- libuv/libuv#4327

[riverar]

We can also look at getting this API in earlier than microsoft/win32metadata#1934, will file a separate issue to track that.

[ChrisDenton]

I've been looking into it using this. One concern is how new the API is. The vast majority of users will not have it and the API docs are still displaying a stability warning (though given Python's unusually early use of the API, I'm not sure how seriously to take that). We could fallback to NtQueryInformationByName which is supported since 1709 (for FILE_STAT_INFORMATION). We do tend to avoid NT APIs in std unless necessary but using it as fallback should be fine. Though even this still leaves Windows Server 2016 out in the cold.

Another issue is that not all filesystem drivers support it. Notably fat32 but also third party drivers or (worse) things that hook the filesystem APIs. Maybe using a FILE_BASIC_INFO fallback there would not be as bad because performance in such cases is already going to be affected.

To be clear, I'm not against it at all. Just that there's enough question that I'd rather think/ask/test a bit more.

[ChrisDenton]

In any case, there's no reason to block fixing the links. Could you either update this PR to just do the link changes or open a new PR for that? And separately open an issue for using GetFileInfromationByName? I don't mind if you also want to experimentally implement it in a PR but I'd rather not tie it to the link changes.

[riverar]

Right, GetFileInformationByName(..., FileStatBasicByNameInfo, ...) doesn't succeed on FAT32 volumes.

[juliusl]

@ChrisDenton - Created a new PR for the links, #130168, I'll start a new issue for this particular thing

[bors]

☔ The latest upstream changes (presumably #130253) made this pull request unmergeable. Please resolve the merge conflicts.

[Dylan-DPC]

@juliusl what's the status of this? thanks

[juliusl]

@Dylan-DPC hey there, was waiting on the windows build that includes this API to get to retail, I'll need to circle back to see what the status is but I think it's either released or is close to releasing.