-
-
Notifications
You must be signed in to change notification settings - Fork 5
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
Enable customization of arguments in Xcode package plugin #149
Conversation
Also enable the Xcode package plugin to run a Release binary. WIP. Still needs documentation.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #149 +/- ##
=======================================
Coverage 99.91% 99.91%
=======================================
Files 32 32
Lines 3467 3485 +18
=======================================
+ Hits 3464 3482 +18
Misses 3 3
|
// Force the command to wait until the async work is done. | ||
dispatchGroup.wait() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is nuts, but since the method is not async
(unlike the plugin for Package.swift files), we have to pull this trick to ensure that the main
method here only exits after all our work is done.
func performCommand( | ||
context: XcodeProjectPlugin.XcodePluginContext, | ||
arguments _: [String] | ||
) throws { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is quite similar to the above func performCommand(context: PackagePlugin.PluginContext, arguments _: [String]) async throws
, except our context is different and we aren't async
.
@@ -0,0 +1 @@ | |||
../Shared.swift |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is how I'm sharing code between Plugins, which can't have shared dependencies.
if context.hasSafeDIFolder, let safeDIVersion, downloadedToolLocation == nil { | ||
Diagnostics.error(""" | ||
\(context.safediFolder.path()) exists, but contains no SafeDITool binary for version \(safeDIVersion). | ||
|
||
To install the release SafeDITool binary for version \(safeDIVersion), run: | ||
\tswift package --package-path \(context.package.directoryURL.path()) --allow-network-connections all --allow-writing-to-package-directory safedi-release-install | ||
|
||
To use a debug SafeDITool binary instead, remove previous installs by running: | ||
\trm -rf \(context.safediFolder.path()) | ||
""") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we used to error if you previously installed the release version and then changed versions. I'm deleting that code path because:
- I'm not convinced it's super helpful. We still have the warnings
- It's harder (not impossible) to tell whether the folder exists because we've downloaded a release binary in the past now that we're reusing this folder for configuration setting
@@ -0,0 +1 @@ | |||
Subproject |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Examples/ExampleMultiProjectIntegration/ExampleMultiProjectIntegration
is already scanned because it's the root module. We just need to list the paths to the other module(s) in this file.
// This file was generated by the SafeDIGenerateDependencyTree build tool plugin. | ||
// Any modifications made to this file will be overwritten on subsequent builds. | ||
// Please refrain from editing this file directly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this file is now in derived data! much nicer
@@ -321,6 +294,10 @@ | |||
target = 3289B4002BF955710053F2E4 /* Subproject */; | |||
targetProxy = 3289B4052BF955720053F2E4 /* PBXContainerItemProxy */; | |||
}; | |||
32B72E1D2D39765B00F5EB6F /* PBXTargetDependency */ = { | |||
isa = PBXTargetDependency; | |||
productRef = 32B72E1C2D39765B00F5EB6F /* SafeDIGenerator */; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we're now using the plugin instead of a build script phase
// As of Xcode 16.0 Beta 6, the display version is of the form "Optional(version)". | ||
// This regular expression is duplicated by SafeDIGenerateDependencyTree since plugins can not share code. | ||
guard let versionMatch = try /Optional\((.*?)\)|^(.*?)$/.firstMatch(in: displayVersion), | ||
let versionSubstring = versionMatch.output.1 ?? versionMatch.output.2 | ||
else { | ||
Diagnostics.error("Could not extract version for SafeDI") | ||
exit(1) | ||
} | ||
let version = String(versionSubstring) | ||
let safediFolder = context.package.directoryURL.appending( | ||
component: ".safedi" | ||
) | ||
let expectedToolFolder = safediFolder.appending( | ||
component: version | ||
) | ||
let expectedToolLocation = expectedToolFolder.appending(component: "safeditool") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this code is mostly in Shared.swift
now
fileprivate func write(toPath filePath: String) throws { | ||
#if os(Linux) | ||
try write(to: URL(fileURLWithPath: filePath)) | ||
#else | ||
guard #available(macOS 13.0, iOS 16.0, tvOS 16.0, watchOS 9.0, *) else { | ||
try write(to: URL(fileURLWithPath: filePath)) | ||
} | ||
try write(to: URL(filePath: filePath)) | ||
#endif | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no longer used
} | ||
} | ||
|
||
extension PackagePlugin.PluginContext { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the below was moved to Shared.swift
@Option(help: "The desired output location of a file a SafeDI representation of this module. Only include this option when running on a project‘s non-root module. Must have a `.safedi` suffix") var moduleInfoOutput: String? | ||
@Option(help: "A path to a CSV file comprising the names of modules to import in the generated dependency tree. This list is in addition to the import statements found in files that declare @Instantiable types.") var additionalImportedModulesFilePath: String? | ||
|
||
@Option(help: "The desired output location of a file a SafeDI representation of this module. Only include this option when running on a project‘s non-root module. Must have a `.safedi` suffix.") var moduleInfoOutput: String? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just added a trailing .
to the comment here
Going to merge this and release a 1.1.0-beta-1 release so it can be tested. @pouyayarandi can you adopt and lmk if it works well for you before I release the version to all? |
1f882cd
to
192a418
Compare
I just tested both include.csv and additionalImportedModules.csv, everything's work smoothly 🔥 |
Incredible! Will release the 1.1.0 within a few days then. Thank you for the feedback that led to this improvement, and for testing SafeDI 😄 |
Also enable the Xcode package plugin to run a Release binary.
Addresses #147