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

[Joel E. Denny via Phabricator via llvm-commits]

jdenny added a comment.

In D64135#1704685 <https://reviews.llvm.org/D64135#1704685>, @yln wrote:

> Hi Joel @jdenny,


Hi Julian.

> 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 guess it depends on the intended use case: which is the default, and which is the override?

1. I added `LIT_OPTS` specifically in order to override the default options supplied by the build config.  That implies `LIT_OPTS` should be parsed last (assuming later options override earlier options when order matters).

2. I think the convention you mention is for the (probably much more common) use case where an environment variable is intended to hold a default that can be overridden on the command line.  Do we anticipate that people will actually use `LIT_OPTS` for that purpose?

To fully support the use case 1 without risking surprising behavior if someone attempts use case 2, we could try to move the `LIT_OPTS` implementation out of lit and into the build system.  That is, use case 2 would become impossible.

Then again, order doesn't seem to matter anyway for options that I normally care about: `-a` and `-vv` seem to take effect no matter where they appear in relation to the usual default of `-sv`.  Order does matter for `--filter`, and I assume it matters for `-D`.  I don't know if anyone would ever build with those as default options and then want to override them.  It might be surprising if you couldn't, given that 1 is the intended use case.

Without knowing which use cases beyond my own are real, I'm inclined to wait before selecting a path forward.  @probinson reviewed, so perhaps he has some opinion.

> 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?

Good point.

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

Good to hear!

Thanks.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64135