This is an archive of the discontinued LLVM Phabricator instance.

Resolve response file names relative to including file
ClosedPublic

Authored by sepavloff on Sep 26 2016, 8:00 AM.

Details

Summary

If a response file included by construct @file itself includes a response file
and that file is specified by relative file name, current behavior is to resolve
the name relative to the current working directory. The change adds additional
flag to ExpandResponseFiles that may be used to resolve nested response file
names relative to including file. With the new mode a set of related response
files may be kept together and reference each other with short position
independent names.
The new mode make work with configuration files more convenient.

Diff Detail

Event Timeline

sepavloff updated this revision to Diff 72487.Sep 26 2016, 8:00 AM
sepavloff retitled this revision from to Resolve response file names relative to including file.
sepavloff updated this object.
sepavloff added a reviewer: rnk.
sepavloff added a subscriber: cfe-commits.
sepavloff edited subscribers, added: llvm-commits; removed: cfe-commits.Sep 26 2016, 8:09 AM
rnk edited edge metadata.Sep 27 2016, 11:04 AM

Can you elaborate on the compatibility concerns with MSVC and libiberty?

According to MSDN, nested response files are not allowed, so no compatibility problems appear https://msdn.microsoft.com/en-us/library/3te4xt0y.aspx:

It is not possible to specify the @ option from within a response file. That is, a response file cannot embed another response file.

As for libiberty, it allows nested response files and interprets text right after @ as file name. So files are searched relative to the cwd of compiler. It is not a problem if absolute file paths are used. It also is not a problem if including file is in the cwd. But if included file is specified by relative name and including file is not in cwd, the behavior would change.

It seems that this situation should not occur in practice because it means that response file must be written with particular cwd of compiler in mind, but some risk of compatibility issues is present.

sepavloff updated this revision to Diff 72952.Sep 28 2016, 11:56 PM
sepavloff edited edge metadata.

Updated patch

Instead of changing resolution of all nested response file names, introduce
new mode of ExpandResponseFiles, in which relative names are resolved in the
new way. Implementation of config files may use the new mode while other
code continue using previous implementation. Such solution must not cause
compatibility issues.

sepavloff updated this object.Sep 28 2016, 11:58 PM
rnk added inline comments.Oct 4 2016, 11:01 AM
lib/Support/CommandLine.cpp
947

Can we do the relativization logic here rather than doing two complete scans of the response file tokens? Presumably they will be large.

sepavloff added inline comments.Oct 5 2016, 11:35 PM
lib/Support/CommandLine.cpp
947

Although it may not be obvious initially, there is no full rescan here. Two functions ExpandResponseFiles and ExpandResponseFiles make recursive response file expansion without actual recursion. ExpandResponseFiles operates on command line already split into arguments by OS command processor. It scans only first symbols of arguments looking for '@'. For each response file it calls ExpandResponseFile, which replaces @file by its split content. It may cause shift arguments that follow @file down but all these arguments are unexpanded. Then ExpandResponseFiles continues the scan from the first argument obtained from the file expansion. So, ExpandResponseFiles is called only once for a response file and ExpandResponseFiles does not scan each argument more than once.

rnk accepted this revision.Oct 31 2016, 1:16 PM
rnk edited edge metadata.

lgtm

unittests/Support/CommandLineTest.cpp
540

Delete TestDir

This revision is now accepted and ready to land.Oct 31 2016, 1:16 PM
This revision was automatically updated to reflect the committed changes.