Skip to content

Make swift driver friendlier to relative paths #73

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 1 commit into from
Mar 6, 2020

Conversation

zienag
Copy link
Contributor

@zienag zienag commented Feb 14, 2020

Add handling for relative paths in output file map path,
resonse file paths, recorded input modification dates.

Copy link
Contributor

@owenv owenv left a comment

Choose a reason for hiding this comment

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

LGTM, this looks like it should make path management a lot more consistent!

@owenv
Copy link
Contributor

owenv commented Mar 3, 2020

@swift-ci test

@owenv
Copy link
Contributor

owenv commented Mar 3, 2020

@zienag sorry for taking so long to come back to this. It looks one of the response file tests is failing on CI

@zienag
Copy link
Contributor Author

zienag commented Mar 4, 2020

@zienag sorry for taking so long to come back to this. It looks one of the response file tests is failing on CI

Hmm, that is unfortunately non trivial bug: given arguments

swiftc -Xlinker @SOMETHING

expandResponseFiles would think that @SOMETHING is response file passed to driver. Before my changes it tried to make absolute path from SOMETHING, failed and skipped that argument. Now it makes relative path from SOMETHING and tries to read its content. This behaviour is totally wrong, and likely deserves separate pr.

Do you know any code/place that works with -Xblabla arg arguments to ignore or filter them out in expandResponseFiles?

@zienag
Copy link
Contributor Author

zienag commented Mar 4, 2020

Meanwhile, I'll remove changes associated with response file from this pr, as extensions I've added are needed by other prs

@zienag zienag force-pushed the relative_paths_handling branch from dae1841 to eef0bfd Compare March 4, 2020 14:29
Copy link
Contributor

@cltnschlosser cltnschlosser left a comment

Choose a reason for hiding this comment

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

Unfortunate that you had to undo some of the work for response files, but this looks good!

@owenv
Copy link
Contributor

owenv commented Mar 4, 2020

@zienag unfortunately I don't have a good idea about how to handle -X... arguments with response files. This might cause problems when we eventually migrate to swift-argument-parser too. In the meantime, the rest of this change will still be really helpful though!

@owenv
Copy link
Contributor

owenv commented Mar 4, 2020

@swift-ci test

1 similar comment
@owenv
Copy link
Contributor

owenv commented Mar 4, 2020

@swift-ci test

@zienag zienag mentioned this pull request Mar 6, 2020
@owenv
Copy link
Contributor

owenv commented Mar 6, 2020

Looks like the checks are all passing now. Thanks @zienag !

@owenv owenv merged commit 0aa2178 into swiftlang:master Mar 6, 2020
@zienag zienag deleted the relative_paths_handling branch March 6, 2020 16:53
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