Windows: Add response file support

[davido]

Inspired by #3742.

On Windows, the command line size is restricted to 8191 chars. Add support for response file in bazel, so that bazel @command.rsp just works.

[laszlocsomor]

SGTM.

BTW the Bazel terminology is "parameter file" or "params file".

Edit: ...or "flagfile".

[laszlocsomor]

I think this feature request sounds good and I think we need it for Windows. I see two immediate caveats that make this FR non-trivial and call for a design doc:

• the Bazel client needs to parse the params file, and potentially write a different params file to propagate the args to the Bazel server
• the Bazel client would need a reliable temp directory to write this params file

Pinging @lfpino, @jmmv, @cvcal, @lberki: do you see any caveats with adding params file support to Bazel itself (e.g. bazel @commandline.txt)?

[jmmv]

I think this is a good idea, but any caveats will depend on the specific proposal to do this. E.g. if the parameters file is read very early on during client initialization, the client's parser should have no knowledge that this happened and thus everything should be "transparent".

The more tricky case is: how do you decide when to use a parameters file for the server? An option would be to compute the length of the server command line and only convert it to a parameters file if necessary. Another would be to simply force the use of a parameters file in all cases to avoid two flag parsing mechanisms on the server. The latter seems simpler, but I'm sure there are drawbacks ;-)

Lastly, regarding the temporary directory: the Bazel client is already writing files for the server to consume (e.g. the logging.properties file), which I think go into the output base. So the parameters file should go there as well; there is no need to come up with a separate directory.

[laszlocsomor]

The more tricky case is: how do you decide when to use a parameters file for the server?

There's the params file a user may pass to the client, and there's the potentially different params file the client may pass to the server. I think it may also happen that:

• the user passes a params file to the client but the client doesn't pass any to the server (because the commands can fit on the command line), or
• the user passes no params file (the command line is just below the limit) but the client needs to write a params file for the server (the server's complete command line exceeds the limit)

An option would be to compute the length of the server command line and only convert it to a parameters file if necessary.

SGTM.

Another would be to simply force the use of a parameters file in all cases to avoid two flag parsing mechanisms on the server. The latter seems simpler, but I'm sure there are drawbacks ;-)

I don't see any problems with that either: the Bazel client already writes the command.log file, which records the flags it passed to the server. Repurposing that file as the server's params file would incur no extra I/O cost.

[laszlocsomor]

Forgot to fully answer:

The more tricky case is: how do you decide when to use a parameters file for the server?

The client knows which platform it launches the server on, and what the limits on the platform are.

[jmmv]

An option would be to compute the length of the server command line and only convert it to a parameters file if necessary.

SGTM.

Yes, but that adds complexity. If we could unconditionally use a params file for the server on all platforms you'd just need to maintain a code path, and you would avoid having to decide whether to write one or not depending on command-line limits.

Food for thought: Linux has infinite command-lines but FreeBSD (and other Unixes that we might support in the future) do not. Will we need to special-case those too?

So yes, a design doc is warranted ;-P

[laszlocsomor]

Food for thought: Linux has infinite command-lines

I think it's limited to 64K or so (depending on your kernel of course).

[jmmv]

I think it's limited to 64K or so (depending on your kernel of course).

Linux has essentially-unlimited command lines thanks to a very old contribution from Google. See the execve(2) manual page for details, section Limits on size of arguments and environment. (The system I'm looking at right now has 8MB of stack which means that the command line can grow to 1MB... but I could bump that stack limit.)

[laszlocsomor]

@cvcal pinged me with some concerns about this FR, she'll update the thread later today with details.

For now I think this FR is not time-critical: we can work around the problem by creating test_suite rules for the CI.

[lberki]

By "server command line" you mean the command line used to start up the server? That's usually short-ish, except in batch mode, but batch mode as we know is moribund anyway (the plan is to replace it with single-shot gRPC servers, although I can't seem to find who's pursuing that -- @ericfelly knows maybe?)

Given this, I think the most reasonable approach is to implement this within the client. Then one needs to implement it only once. Sure, batch mode wouldn't work, but that will fix itself soon anyway.

/cc @lfpino

[laszlocsomor]

By "server command line" you mean the command line used to start up the server?

No, I meant the Bazel command's (build, test, etc.) command line. Bazel test runs on CI often list many targets, which can push the command length over the platform's limit.

Sure, batch mode wouldn't work

Why not?

[lberki]

Because batch mode currently works by running a separate process. If we parsed params files in the client and then copied their contents into another command line, we'd have the exact same problem one step later.

[cvcal]

The command line looks like this (simplified slightly and standardized, but this is generally true):
bazel (startup flags)* command (command flags)* (targets)*

There are clearly 3 chunks that can get arbitrarily large, and we already have a solution to 2 of them! The bazelrc configuration file lets you customize the startup flags and command flags while keeping your command line minimal. Let's not add another solution that parallels this behavior with its own new flag syntax. Instead, could we limit this FR to making the target list params-file settable?

I say this, but I believe it is in vain - ParamsFilePreProcessor seems to already exist.

@lberki - I'm the one who wanted to replace batch with a short lived server. I still want to make this happen, but I haven't gotten around to it yet.

[laszlocsomor]

Thanks @cvcal !

could we limit this FR to making the target list params-file settable?

Yes, I think so.

I say this, but I believe it is in vain - ParamsFilePreProcessor seems to already exist.

I found this class: https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/common/options/ParamsFilePreProcessor.java

Bazel 0.5.4 however doesn't seem to support it for Bazel's own flags:

  $ bazel query 'tests(//src/test/...)' | wc -l
274

  $ bazel query 'tests(//src/test/...)' > /tmp/a.txt

  $ bazel query --output=label_kind @/tmp/a.txt
ERROR: Couldn't find package in target @/tmp/a.txt

[lberki]

@cvcal Erm, sorry -- neither my memory nor my Gmail-search-fu is good enough, apparently.

re: the argument that we already have a mechanism for setting both the startup arguments and the command arguments: it's true, but at the same time, the @ params file syntax is a widely understood command line convention, so I think it makes sense to support that. There is also the issue that allowing target specifications in config files allows for surprising behavior, which we may want to avoid.

In summary, I'm (hesitantly)in support of adding the @ syntax.

[laszlocsomor]

Thanks @lberki !

There is also the issue that allowing target specifications in config files allows for surprising behavior, which we may want to avoid.

Could you elaborate what surprising behavior you had in mind?

In summary, I'm (hesitantly)in support of adding the @ syntax.

Could you clarify which suggestion you're supporting:

• config file for targets only (would suffice for this bug)
• config file for targets and Bazel flags (overlaps functionality with bazelrc files)

[lberki]

For every command line option, as is customary with params files.

Unfortunately, I just realized that this is not as simple as I had hoped because the target-in-specified-repository syntax (@repo//pkg:target) clashes with the customary @filename syntax :(

[github-actions]

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 3 years. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

[github-actions]

This issue has been automatically closed due to inactivity. If you're still interested in pursuing this, please reach out to the triage team (@bazelbuild/triage). Thanks!