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

Support MSVC response files #192

Closed
wants to merge 7 commits into from

Conversation

jasonwhite
Copy link

Adds support for @-files appearing on the MSVC command line.

The response file is expanded in-place on the command line, but not recursively like in the gcc implementation in order to conform to the spec. I also implemented a Windows command line parser for this instead of relying on CommandLineToArgvW just in case people somehow use the MSVC compiler on non-Windows platforms.

This was one of the blockers for MSBuild support as mentioned in #107 (comment). After this is merged, I'll look into adding support for handling multiple source files on one command line.

Please rip it apart in a code review. :feelsgood:

@jasonwhite
Copy link
Author

jasonwhite commented Nov 3, 2017

I fixed a few problems in my implementation:

  1. While MSBuild encodes response files as UTF-16LE, users may write them in other encodings such as latin1 or UTF-8. To handle this, I decided to add a new dependency on the encoding crate. This will decode the BOM at the beginning of the response file (if any) and decide how to decode it based on that.
  2. MSBuild passes preprocessor defines as separate arguments (e.g., /D DEBUG instead of /DDEBUG). A simple change in the argument parser fixes it.
  3. Added a unit test to actually test everything.

I'm more confident that this implementation is solid now, but a code review would be great.

@jasonwhite
Copy link
Author

@luser Could you review this when you have a chance? I'm not in a rush, but it'd be nice to get this in before it rots for too long.

I'll follow up with another pull request to add support for multiple inputs per command line. It'll be a while before that part is done though.

@StevenEBarbaro
Copy link

Hi all!

Any chance the above PR will be accepted/rejected sometime soon?

I've been attempting to leverage sccache to build Chromium on Windows with mixed results.
These changes relieve some of the problems. I believe more will be resolved by #107 (comment)

MSBuild passes them to the response file this way.
If a directory is specified with `-Fo`, it should be interpretted as the
base directory for object files. The path to the object file is then the
directory + the basename of the source file with an `.obj` extension.

https://docs.microsoft.com/en-us/cpp/build/reference/fo-object-file-name
@jasonwhite
Copy link
Author

I rebased to resolve the merge conflict. This is good to go.

@StevenEBarbaro
Copy link

Hi again,

Is this one projected to merge sometime soon?

@luser
Copy link
Contributor

luser commented Mar 27, 2018

Sorry, this is just large enough that I need to clear headspace to review it and I haven't managed.


Instead, you must use `/Z7` where the debug information is embedded in the
object file itself. The main trade off is that you cannot use Minimal Rebuild
(`/Gm`) or Edit and Continue.
Copy link

Choose a reason for hiding this comment

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

Also consider the /Fd option to name your PDB file. When you compile a single input and write to an exclusiv PDB file, explicitly named with /Fd compilation, hashing and caching is safe.

@froydnj
Copy link
Contributor

froydnj commented Aug 31, 2018

@jasonwhite, do you have space/time to rebase this?

@jasonwhite
Copy link
Author

Closing this as I currently don't use sccache. I may come back to this in the future, but someone else is free to pick this back up where I left off.

@jasonwhite jasonwhite closed this Mar 5, 2019
sylvestre pushed a commit that referenced this pull request Mar 9, 2023
Adds an iteration layer between the command-line argument iterator and the `ArgIter` used to compare arguments against the supported flags/options. This new layer determines if an option is a response-file directive (`@file`), and if it is, reads the options from the file before continuing to iterate over the command-line args. This requires an additional file-parsing iterating (`SplitArgs`) to split the file contents into arguments in a way which is consistent with the file format.

The `encoding` crate is used to read utf-8 (default encoding in rust) & utf-16 (big and little endian) encodings. The latter is used by `MSBuild` when generating response files.

Resources:
- [MSDN](https://docs.microsoft.com/en-us/cpp/build/reference/at-specify-a-compiler-response-file)
- [MSBuild](https://learn.microsoft.com/en-us/visualstudio/msbuild/msbuild-response-files?view=vs-2019)

Contributes to #107
Based off of #192
Closes #1082
Closes #1183
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.

None yet

5 participants