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

Enable Xcode 16 plugin to use versioned release build #109

Merged
merged 13 commits into from
Sep 3, 2024

Conversation

dfed
Copy link
Owner

@dfed dfed commented Sep 2, 2024

Xcode 16 allows for plugins to determine their own display version, which enables us to:

  1. Write a package command plugin that can download the current release build of the SafeDITool from Github
  2. Use that release version if it exists in our package build plugin

This PR does both of these things. Note that we do not touch the Swift 5.10 & Xcode 15 code-path, which still looks for an unversioned 😭 release installed via Homebrew.

@dfed dfed self-assigned this Sep 2, 2024
Comment on lines +1 to +19
// Distributed under the MIT License
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to deal
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in all
// copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
// SOFTWARE.
Copy link
Owner Author

Choose a reason for hiding this comment

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

oops. this header should always have been here

Copy link

codecov bot commented Sep 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.66%. Comparing base (78e9d87) to head (5c88555).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #109   +/-   ##
=======================================
  Coverage   99.66%   99.66%           
=======================================
  Files          33       33           
  Lines        2710     2710           
=======================================
  Hits         2701     2701           
  Misses          9        9           

Comment on lines 64 to 72
Diagnostics.error("""
\(context.safediFolder.path()) exists, but there is no SafeDITool binary for this release present.

To download the current release SafeDITool binary, 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 the `.safedi` directory by running:
\trm -rf \(context.safediFolder.path())
""")
Copy link
Owner Author

Choose a reason for hiding this comment

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

what the error looks like in practice:
IMG_9199

Copy link
Owner Author

Choose a reason for hiding this comment

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

If there is a .safedi/ directory at the package root, I'm taking that to mean that the user intended to be using a release version of SafeDITool. Users will likely enter this state after installing SafeDITool, then bumping the SafeDI version they are using.

Comment on lines 74 to 79
Diagnostics.warning("""
Using a debug SafeDITool binary, which is 15x slower than a release SafeDITool binary.

To download the current release SafeDITool binary, run:
\tswift package --package-path \(context.package.directoryURL.path()) --allow-network-connections all --allow-writing-to-package-directory safedi-release-install
""")
Copy link
Owner Author

Choose a reason for hiding this comment

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

Warning shows up nicely too:
IMG_6124

}
switch safeDIOrigin {
case let .repository(url, displayVersion, _):
// As of Xcode 16.0 Beta 6, the display version is of the form "Optional(version)".
Copy link
Owner Author

Choose a reason for hiding this comment

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

Apple... why...

Using displayVersion is clearly brittle, but it does work.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Watch them add a space

Copy link
Owner Author

Choose a reason for hiding this comment

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

Seemed like an oversight. We'll see swiftlang/swift-package-manager#7934

Copy link
Owner Author

Choose a reason for hiding this comment

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

If this ever fully breaks we can do what Apollo does and hardcode the current release version. I'd prefer to trust SPM rather than have to keep this in-sync myself, so we're starting out with this approach.

@dfed dfed marked this pull request as ready for review September 3, 2024 00:17
// 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
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why checking .1 as well as .2?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ditto in the copied code in the other plugin

Copy link
Owner Author

Choose a reason for hiding this comment

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

I'm falling back to a version with no surrounding Optional(...) in case this gets fixed. .1 gets us the optional match. .2 gets us the match without the optional.

Copy link
Collaborator

@MrAdamBoyd MrAdamBoyd left a comment

Choose a reason for hiding this comment

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

Question but LGTM

@dfed dfed changed the title Enable Xcode 16 plugin to use correctly versioned, downloaded release build Enable Xcode 16 plugin to use versioned, downloaded release build Sep 3, 2024
@dfed dfed changed the title Enable Xcode 16 plugin to use versioned, downloaded release build Enable Xcode 16 plugin to use versioned release build Sep 3, 2024
@dfed dfed merged commit 96b65d5 into main Sep 3, 2024
13 checks passed
@dfed dfed deleted the dfed--install-cli-command branch September 3, 2024 14:57
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.

2 participants