pantsbuild / pants

The Pants Build System
https://www.pantsbuild.org
Apache License 2.0
3.32k stars 636 forks source link

scalafix fails with ArgumentListTooLong #20966

Open somdoron opened 5 months ago

somdoron commented 5 months ago

Describe the bug I'm trying to upgrade pants from 2.14.2 to 2.21.0.rc0 and to use the new scalafix plugin. However, scalafix fails with the following error:

Error launching process: Os { code: 7, kind: ArgumentListTooLong, message: "Argument list too long" }

Pants version 2.21.0.rc0 OS Linux

Additional info From the stacktrace it seems the number of arguments to scalafix is 54 out of which 19 are scala-options arguments and 31 files arguments. I think both should be easy to merge into one argument separated by comma?

somdoron commented 5 months ago

I checked scalafix code, apparently it doesn't support comma separated values. I think we can either build a cli instead of provided one to bypass this (by parsing a single comma separated value), just send one file at a time or maybe to provide file as the configuration to scalafix?

alonsodomin commented 5 months ago

yeah, the way that the CLI parsers arguments is quite limiting. The number of files is workaround-able by sprinkling the repo with different .scalafix.conf files at various locations, that makes the size of the partitions used by the linting rules much smaller.

Obviously that is not a proper solution, but in terms of the usage of the current CLI, I believe we can drastically reduce the number of --files arguments if instead of passing the file names, we pass the directory names. The sources here make me believe that Scalafix's CLI would happily take directories and this shouldn't be too big of a change since the rule that runs Scalafix takes a Snapshot as input, so directory names are as available as directory names are.

Long term there is still the possibility of building a too large of the command line, specially if we add more customization options in the future. In such a case it does look like a sensible approach would be to dump the whole command line into a file and reading it from a Scala shim.

somdoron commented 5 months ago

if we are building our own shim we can skip the file and just provide all files in one argument like we do for the classpath. It seems like the most reasonable approach.