Skip to content

Conversation

kateinoigakukun
Copy link
Member

@kateinoigakukun kateinoigakukun commented Dec 17, 2024

Explanation: Fix build issues in WASI platform, which are introduced in #1083
Scope: Impacts FileManager.copyItem(atPath:toPath:) on non-Windows platforms
Original PR: #1094 and #1095
Risk: Low - replace a legacy libc function call with a modern one, gate a few parts behind os(WASI)
Testing: swift-ci testing
Reviewer: @jmschonfeld

Extended attributes don't exist in WASI, so we need to exclude the use
of xattr-related APIs including `flistxattr`.
* Gate `fchown` and `fchmod` calls behind `os(WASI)`

They are not available on WASI, so we gate them behind `os(WASI)`.

* Add missing constant shims for wasi-libc

* Use `futimens` instead of legacy `futimes`

wasi-libc does not provide `futimes` as it is a legacy function.
https://github.com/WebAssembly/wasi-libc/blob/574b88da481569b65a237cb80daf9a2d5aeaf82d/libc-top-half/musl/include/sys/time.h#L34
@jmschonfeld
Copy link
Contributor

@swift-ci please test

@jmschonfeld jmschonfeld requested a review from parkera December 17, 2024 04:17
@finagolfin
Copy link
Member

Anything holding this up? I need it for Android too.

@kateinoigakukun
Copy link
Member Author

I'm waiting @parkera's approval just in case.

@finagolfin
Copy link
Member

@parkera, just need your approval here, can get this in.

@finagolfin
Copy link
Member

@kateinoigakukun, please add my Android change from main, 90700c1, to this pull, would like to get that into 6.1 also.

Normal users don't have permission to change these, even for their own files.
@kateinoigakukun
Copy link
Member Author

@swift-ci test

@finagolfin
Copy link
Member

Thanks, Yuta.

@itingliu, OK to merge? Most of this pull is gated to WASI and Android, other than the futimens() change.

@itingliu itingliu merged commit bca2f46 into swiftlang:release/6.1 Jan 3, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants