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.

Sign up for GitHub

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

Jump to bottom

Replace custom async file system wrapper with NIOFileSystem #76

Open
MaxDesiatov opened this issue Jan 24, 2024 · 7 comments
Open

Replace custom async file system wrapper with NIOFileSystem #76

MaxDesiatov opened this issue Jan 24, 2024 · 7 comments
Labels
good first issue Good for newcomers

Comments

@MaxDesiatov
Copy link
Contributor

MaxDesiatov commented Jan 24, 2024 •
edited
Loading

We currently have our own, not necessarily well-tested or valid implementation of async file system APIs in https://github.com/apple/swift-sdk-generator/tree/main/Sources/GeneratorEngine/FileSystem. With NIOFileSystem now available in SwiftNIO 2.63, we should use that instead.
@MaxDesiatov MaxDesiatov added the good first issue Good for newcomers label Jan 24, 2024
@MaxDesiatov MaxDesiatov mentioned this issue Jan 24, 2024
Merged
@zaneenders
Copy link
Contributor

Hi, I was wondering if I could work on this?

As I was poking around I noticed that running ./Utilities/soundness.sh on main fails for UnixName.swift

=> Checking license headers... 
   * swift-or-c... missing headers in file './Sources/SwiftSDKGenerator/SystemUtils/UnixName.swift'!
--- /dev/fd/63  2024-08-03 15:39:55
+++ /tmp/.swift-package-manager-soundness_nooVYa        2024-08-03 15:39:55

Also this is my first time using swiftformat but when running swiftformat . on main, 35 files are reformatted.

➜  swift-sdk-generator git:(main) swiftformat .
Running SwiftFormat...
Reading config file at .../swift-sdk-generator/.swiftformat
warning: --varattributes option is deprecated. Use with `--storedvarattrs` or `--computedvarattrs` instead.
SwiftFormat completed in 0.46s.
34/60 files formatted.

@MaxDesiatov
Copy link
Contributor Author

Yes, as this is an issue marked as "good first issue" and is unassigned, it's free for anyone for anyone to work on.

Feel free to fix soundness and formatting issues, but those should be submitted as separate PRs so that those changes don't pollute other PRs one may submit as part of their work.

@compnerd
Copy link
Member

compnerd commented Aug 5, 2024

This will effectively prevent porting to windows until NIO properly supports windows. I would prefer that we didn't do this.

@MaxDesiatov
Copy link
Contributor Author

It won't change the status quo WRT Window support, as Swift SDK Generator already depends on SwiftNIO.

@compnerd
Copy link
Member

compnerd commented Aug 5, 2024

Ah, that's rather unfortunate.

@zaneenders
Copy link
Contributor

Could the new Foundation help with the NIO dependency? Though it doesn't look like SubProcess has made it in yet.
swiftlang/swift-foundation#439

@MaxDesiatov
Copy link
Contributor Author

No, it won't help since relevant Foundation APIs are not async.

zaneenders added a commit to zaneenders/swift-sdk-generator that referenced this issue Sep 30, 2024
@zaneenders
Use _NIOFileSystem for file system operations.
61caa68 
Motivation:
Add support for _NIOFileSystem to move away from OSFileSystem as per issue swiftlang#76.

Modifications:
Add SDKFileSystem that uses _NIOFileSystem. Change default AsyncFileSystem to SDKFileSystem

Result:

Using Swift NIO for file system operations.
@zaneenders zaneenders mentioned this issue Sep 30, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@compnerd @MaxDesiatov @zaneenders