rust-lang / compiler-team

A home for compiler team planning documents, meeting minutes, and other such things.
https://rust-lang.github.io/compiler-team/
Apache License 2.0
388 stars 69 forks source link

Support response files generated by ninja with `@ninja:path` syntax #684

Closed djkoloski closed 11 months ago

djkoloski commented 1 year ago

Background

A response file (also called an argument file) is a special command-line argument that instructs a program to read additional arguments from a file. Response files are prefixed with @, and so invocations usually take the form rustc @args.txt. Response files exist to work around maximum line length limitations on various operating systems.

Motivation

This proposal is intended to provide a solution for this issue and allow rustc to integrate with ninja more easily. It also sets a convention for supporting response files with different formats in rustc.

Today in rustc, response files are parsed as newline-delimited files where each argument exists on a single line. Any spaces on a line are treated as part of that argument, and surrounding quotes are passed through verbatim. This makes it difficult to integrate rustc with ninja, which generates response files with space-delimited (possibly-)quoted arguments. This functionality is built into ninja itself with the rspfile option. Rustc's response file format is incompatible with ninja's generated format, and the maintainer of ninja is unlikely to make a special case for us.

Proposal

We propose changing the behavior of rustc and cargo to accept response file arguments of the form@ninja:path. Those arguments are treated as response files generated by ninja which follow its conventions for argument delimitation, quoting, and escaping.

Mentors or Reviewers

@tmandry is on the Fuchsia team and holds some stake in this MCP.

@Nilstrieb has expressed interest in reviewing an MCP for this behavior change.

@jsgf added response file handling to rustc in #63175 roughly four years ago. Argument quoting and character escaping was discussed but not implemented.

Process

The main points of the Major Change Process are as follows:

You can read more about Major Change Proposals on forge.

Comments

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

rustbot commented 1 year ago

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

cc @rust-lang/compiler @rust-lang/compiler-contributors

djkoloski commented 1 year ago

cc @fangism originally raised this in rust-lang/rust#116068. cc @p1n3appl3 who provided an implementation in rust-lang/rust#116954.

jsgf commented 1 year ago

(moved to Zulip)

tmandry commented 1 year ago

This MCP has had several revisions in response to feedback, and at this point only bikeshed details are left.

@rust-lang/compiler would someone please kick off an FCP?

tmandry commented 1 year ago

Actually, after some discussion with the compiler team it sounds like the right process is to start with an unstable-gated flag, in which case this does not need a full team sign-off and a second is all that is necessary.

@rustbot second

apiraino commented 11 months ago

@rustbot label -final-comment-period +major-change-accepted