Skip to content

fix: absolute URL for correct path resolution in Runner #650

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

Merged
merged 3 commits into from
May 13, 2025
Merged

fix: absolute URL for correct path resolution in Runner #650

merged 3 commits into from
May 13, 2025

Conversation

Fab1n
Copy link
Contributor

@Fab1n Fab1n commented May 6, 2025

Problem

The logic when using the --cwd parameter has been altered with https://github.com/danger/swift/releases/tag/3.21.0.

In one of the projects where I am participating we use the Danger Swift in a nested subdir called scripts/Scripts.
Thus we need to make use of the cwd param with --cwd ../../.

This is the error we get since upgrading to a version >= 3.21.0:

The operation couldn’t be completed. (SwiftLintCore.Issue error 13.)
Could not read configuration: file Configuration.swift, line 274

/bin/sh: line 1: 89322 Abort trap: 6           mise x -- swiftlint lint --reporter json --quiet --config ".swiftlint-danger.yml" > /var/folders/5j/lqqdx5hd6qj66nzrg6bhr_kw0000gq/T/swiftlintReport.json

How to reproduce

To execute the script then we follow your advice here:

Note that to do this, you must run danger-swift from the directory where the Package.swift is located

So we literally do this:

cd scripts/Scripts
swift run danger-swift ci --dangerfile Dangerfile-Preflight.swift --id "Danger Preflight" --failOnErrors --cwd ../../

Notice the ../../ at the end!

Why does it happen?

The PR chunk that introduces the breaking change is: https://github.com/danger/swift/pull/634/files#diff-c744a5c86fb867044f963d726549b0df11e03443f10c9d4c21c06103d8f0d534

Here is the PR ref: #634

We switched from:

if let cwdOptionIndex = CommandLine.arguments.firstIndex(of: DangeSwiftRunnerOption.cwd.rawValue),
(cwdOptionIndex + 1) < CommandLine.arguments.count,
let directoryURL = URL(string: CommandLine.arguments[cwdOptionIndex + 1])
{
proc.currentDirectoryPath = directoryURL.absoluteString
}

To:

if let cwdOptionIndex = CommandLine.arguments.firstIndex(of: DangeSwiftRunnerOption.cwd.rawValue),
(cwdOptionIndex + 1) < CommandLine.arguments.count
{
let directoryURL = URL(fileURLWithPath: CommandLine.arguments[cwdOptionIndex + 1])
proc.currentDirectoryURL = directoryURL
}

... effectively removing the absolute URL resolution, which especially has an effect if you pass a relative path like ../../.

The fix 🔧

The fix simply is to add the absoluteURL var call: proc.currentDirectoryURL = directoryURL.absoluteURL

Testing

Apply the fix locally via swift package edit swift in the Package folder, where the Danger Swift dependency has been added by replacing the dependency with:
.package(url: "https://github.com/Fab1n/danger-swift.git", branch: "runner-fix-absolute-url"),

This pulls in this PR's change.

Run:

swift run danger-swift pr https://github.com/danger/swift/pull/146 --dangerfile Some-Dangerfile.swift --cwd ../../

🎉

Copy link
Member

@f-meloni f-meloni left a comment

Choose a reason for hiding this comment

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

Wow! Thank you for the really detailed PR! Can you please also update the changelog? Thank you!

@Fab1n
Copy link
Contributor Author

Fab1n commented May 13, 2025

Wow! Thank you for the really detailed PR! Can you please also update the changelog? Thank you!

@f-meloni done

@f-meloni f-meloni merged commit 667d5c9 into danger:master May 13, 2025
14 checks passed
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.

3 participants