[PATCH] D64135: [lit] Parse command-line options from LIT_OPTS

[Julian Lettner via Phabricator via llvm-commits]

yln added a comment.
Herald added a reviewer: jdoerfert.

Hi Joel @jdenny,
I would like to ask you to reconsider whether or not it is a good idea that the env var overrides the command line options. I understand the desire for this so one can reinvoke `env LIT_OPTS=... ninja check-...` and be certain that those options are going to be used.  My arguments against it are mostly for consistency and to avoid surprising non-standard behavior:

- It is not "standard". The default is that CL options override ENV vars [1].
- It is inconsistent with the other option configurable var env vars, e.g., `LIT_FILTER` and `LIT_*_SHARD`.
- Most of the time we want to supplement---not override---existing options.

One more ask independent of what we decide: the `lit-opts.py` test does not go red when we change the override order. Can you adapt the test to give a signal in this case?

  env_args = shlex.split(os.environ.get("LIT_OPTS", ""))
  args = sys.argv[1:] + env_args  # lit-opts.py does not go red when we flip the order of concatenation here. Is this expected?
  opts = parser.parse_args(args)

Also: thank you for adding and documenting this feature and even providing a test. I am already using it and find it useful!

[1] https://stackoverflow.com/a/11077282/271968


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64135/new/

https://reviews.llvm.org/D64135