Skip to content

Fix response file quote bug, add flag for testing, add tests. #19321

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

Conversation

davidungar
Copy link
Contributor

@davidungar davidungar commented Sep 14, 2018

Explanation: Fixes response file bug when spaces are in file paths.
Scope of issue: Build breakage when paths are too long and contain spaces.
Origination: Typo when written, as near as I can tell.
Risk: Minimal. I can't see how getting the quotes right could introduce a bug.
Reviewed by: Jordan Rose
Testing: Normal regression tests.
Radar: rdar://44142091

Details:

Recently, response files were added to the compiler. If the project is located in a path that is long enough, so that the arguments to a tool (frontend, linker) might be too long, the driver passes them in a file instead, called a "response file". In order for the tool to accurately reparse the reponse file contents into a sequence of arguments, each argument in the response file must be enclosed in quotes in case that argument contains a space character.

However, when the response file mechanism was added, extra quotes were mistakenly used. For instance, the argument gets written to the response file as <""part1 part2.swift"">.
The effect is to cause projects using response files to break.

This PR:

  • fixes the bug,
  • adds a flag to make it possible to test the fix: -driver-force-response-files,
  • adds tests for the flag, and the bug fix.

It combines PRs from master:
#19316
#19314
#19313

@davidungar
Copy link
Contributor Author

@swift-ci please test

@davidungar
Copy link
Contributor Author

@swift-ci please nominate

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - d5528a0

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - d5528a0

@jrose-apple
Copy link
Contributor

Must be an absent #include on the 4.2 branch.

@jrose-apple
Copy link
Contributor

LGTM besides the #include issue, ready for Ted's approval.

@davidungar
Copy link
Contributor Author

@swift-ci please test

@davidungar
Copy link
Contributor Author

Thanks, @jrose-apple .

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - d5528a0

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - d5528a0

@davidungar davidungar merged commit eeb6230 into swiftlang:swift-4.2-branch Sep 14, 2018
davidungar pushed a commit to davidungar/swift that referenced this pull request Sep 14, 2018
…spaces-in-files-in-resp-files-swift-4.2

Fix response file quote bug, add flag for testing, add tests.
davidungar pushed a commit to davidungar/swift that referenced this pull request Sep 20, 2018
…spaces-in-files-in-resp-files-swift-4.2

Fix response file quote bug, add flag for testing, add tests.
davidungar pushed a commit that referenced this pull request Sep 20, 2018
…s-with-spaces-Swift-5

Merge pull request #19321 from davidungar/rdar-44458502-fix-spaces-in-long-paths
@davidungar davidungar deleted the rdar-44458502-fix-spaces-in-files-in-resp-files-swift-4.2 branch September 23, 2019 16:19
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