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

Build: quote the path string for bundle resources #2972

Merged
merged 1 commit into from
Oct 10, 2020

Conversation

compnerd
Copy link
Member

@compnerd compnerd commented Oct 9, 2020

We need to ensure that we escape \ for paths in strings. This enables
using bundles for tests on Windows.

@compnerd
Copy link
Member Author

compnerd commented Oct 9, 2020

@swift-ci please smoke test

@neonichu
Copy link
Contributor

neonichu commented Oct 9, 2020

What's the impact on non-Windows? The forward slash is a valid character in filenames on macOS, for example.

@compnerd
Copy link
Member Author

compnerd commented Oct 9, 2020

Forward slashes remain as is - this simply escapes backslashes in the path so that it can be embedded into a string.

Sources/Build/BuildPlan.swift Outdated Show resolved Hide resolved
We need to ensure that we escape `\` for paths in strings.  This enables
using bundles for tests on Windows.
@compnerd
Copy link
Member Author

compnerd commented Oct 9, 2020

@swift-ci please smoke test

@abertelrud
Copy link
Contributor

Thanks for fixing this! I agree with Jakes comment and the revised fix looks good.

@abertelrud
Copy link
Contributor

Jake, do you still feel that changes are requested here?

Copy link
Contributor

@jakepetroules jakepetroules left a comment

Choose a reason for hiding this comment

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

Looks great, thanks Saleem!

@compnerd compnerd merged commit 9fe4642 into swiftlang:main Oct 10, 2020
@compnerd compnerd deleted the quoted branch October 10, 2020 15:04
stream <<< """
import class Foundation.Bundle

extension Foundation.Bundle {
static var module: Bundle = {
let mainPath = Bundle.main.bundlePath + "/" + "\(bundlePath.basename)"
Copy link
Contributor

@ffried ffried Apr 30, 2021

Choose a reason for hiding this comment

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

This change actually broke the lookup for resource bundles. Since Bundle.main is now executed inside the swift build context, mainPath is now /usr/bin on linux instead of the location of the output binary.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've moved the call to Bundle.main back to the generated code in #3463

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.

5 participants