Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FoundationEssentials: implement file operations #566

Merged
merged 1 commit into from Apr 29, 2024
Merged

Conversation

compnerd
Copy link
Collaborator

This adds an initial implementation for file operations on Windows. With this, FileManager should at least build on Windows, which unblocks future work as well as brings us closer to enabling FoundationEssentials on Windows.

Copy link
Member

@parkera parkera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll leave the detailed review to @jmschonfeld but this looks good to me. Thanks for taking on this porting work @compnerd!

Copy link
Contributor

@jmschonfeld jmschonfeld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor comments but otherwise this looks great, thanks!

Copy link
Contributor

@jmschonfeld jmschonfeld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Running CI just to make sure the conditionals didn't accidentally break the other builds

@jmschonfeld
Copy link
Contributor

@swift-ci please test

@jmschonfeld
Copy link
Contributor

/build/swift-foundation/Sources/FoundationEssentials/FileManager/FileOperations.swift:845:159: error: cannot convert value of type 'UnsafeMutablePointer<CChar>' (aka 'UnsafeMutablePointer<Int8>') to expected argument type 'String'

These build failures do look related - it looks like this change breaks the macOS/linux build. Let me know if you need any assistance addressing that before we merge this

@compnerd
Copy link
Collaborator Author

@swift-ci please test

1 similar comment
@compnerd
Copy link
Collaborator Author

@swift-ci please test

@compnerd
Copy link
Collaborator Author

@swift-ci please test

@compnerd
Copy link
Collaborator Author

@swift-ci please test

@compnerd
Copy link
Collaborator Author

@swift-ci please test

This adds an initial implementation for file operations on Windows. With
this, `FileManager` should at least build on Windows, which unblocks
future work as well as brings us closer to enabling
`FoundationEssentials` on Windows.
@compnerd
Copy link
Collaborator Author

@swift-ci please test

@compnerd
Copy link
Collaborator Author

Going to merge this for now - if there are any more comments, happy to address them in a follow up. This should allow us to get to the file diffing changes.

@compnerd compnerd merged commit aabac27 into apple:main Apr 29, 2024
2 checks passed
@compnerd compnerd deleted the fops branch April 29, 2024 22:35
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.

None yet

3 participants