leogdion / PackageListValidator

A tool for validating the SwiftPM Library/Swift Package Index master package list.
MIT License
1 stars 0 forks source link

Migrate from using `Process` to direct API for "dump package" validation #1

Open leogdion opened 4 years ago

leogdion commented 4 years ago

@daveverwer

If you have any notes or details on removing the need to use Process for this, I'd appreciate it.

daveverwer commented 4 years ago

So here's the code that @mordil passed me for reading the Manifest. The code still requires access to the Swift compiler, but it might be more efficient than running dump-package.

https://github.com/apple/swift-package-manager/blob/master/Examples/package-info/Sources/package-info/main.swift

However, after thinking about it a bit I'm not sure that it's the right approach. The security issues of executing random packages might not be a huge issue inside a GitHub Action virtual machine, but I can't see switching the actual SwiftPM Library over to this method because of fact that it'd prevent sandboxing it with firejail or similar.

Also, I'm not sure how much of an issue the performance of dump-package actually is in reality. The vast majority of runs of this tool are going to be running on 1 package and the overhead of running it there is trivial. Running it locally on our development machines to validate all packages is a much bigger issue, but with the tweaking of the concurrency and timeout parameters, it does work.

leogdion commented 4 years ago

Thanks for the notes @daveverwer

I'll think about this further to see if there are alternatives. For now, we have something that at least works with Process but it would be nice to move away from that eventually. I'll keep this issue open as we find other ways to manage it in the future.

daveverwer commented 4 years ago

As a note, @finestructure and I have discussed this quite a bit and come down firmly on the opinion that running dump-package is better than trying to process this file directly.

Should we close this issue?

leogdion commented 4 years ago

Sure. I was thinking keep it around until something better comes along. I started looking at SwiftSyntax as a possibility and in the SwiftPM slack there was talk about another alternative.

However if you want to close it for now, feel free.