[lit] Parse command-line options from LIT_OPTS

[jdenny]

Summary

Similar to FILECHECK_OPTS for FileCheck, LIT_OPTS makes it easy to
adjust lit behavior when running the test suite via ninja. For
example:

$ LIT_OPTS='--time-tests -vv --filter=threadprivate' \
  ninja check-clang-openmp

[rnk]

I think it would be preferable to standardize on llvm-lit as the way to re-run particular tests with some options.

[jdenny]

In D64135#1569174, @rnk wrote:

I think it would be preferable to standardize on llvm-lit as the way to re-run particular tests with some options.

Thanks for the comment. Why so?

For me, it seems quicker to use LIT_OPTS and ninja because I don't have to extract the lit command line from ninja -v. For ninja check-all, that's a lot to copy and paste. It also avoids forgetting to rebuild while I'm alternating between hacking the source and re-running failing tests.

[probinson]

lit has options?

If this works when running llvm-lit.py as well as ninja check-foo, I can see it being handy for those (rare at least for me) occasions when a lit option is what you want. And it looks pretty cheap.

Re tests, I don't see why you have 4, wouldn't 2 be sufficient? Given that 2, 3, and 4 are identical.

[jdenny]

In D64135#1569395, @probinson wrote:

lit has options?

:-)

If this works when running llvm-lit.py as well as ninja check-foo, I can see it being handy for those (rare at least for me) occasions when a lit option is what you want. And it looks pretty cheap.

Lately, I most often use -vv (so I can see the line number of a failed RUN command) and -a (so I can verify the passing tests are doing what I expect).

Re tests, I don't see why you have 4, wouldn't 2 be sufficient? Given that 2, 3, and 4 are identical.

Good point. Actually, 1 is probably enough for what I actually mean to test. I'll adjust soon.

Thanks.

[jdenny]

Simplify test as discussed with @probinson. Improve wording in docs slightly.

[probinson]

LGTM

[yln]

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

[jdenny]

In 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.

llvm/trunk/docs/CommandGuide/lit.rst

 :program:`lit` also includes a number of options for controlling how tests are
 executed (specific features may depend on the particular test format).  See
 :ref:`execution-options` for more information.
 
 Finally, :program:`lit` also supports additional options for only running a
 subset of the options specified on the command line, see
 :ref:`selection-options` for more information.
 
+:program:`lit` parses options from the environment variable ``LIT_OPTS`` after
+parsing options from the command line.  ``LIT_OPTS`` is primarily useful for
+supplementing or overriding the command-line options supplied to :program:`lit`
+by ``check`` targets defined by a project's build system.
+
 Users interested in the :program:`lit` architecture or designing a
 :program:`lit` testing implementation should see :ref:`lit-infrastructure`.
 
 GENERAL OPTIONS
 ---------------
 
 .. option:: -h, --help
 

llvm/trunk/utils/lit/lit/main.py

 #!/usr/bin/env python
 
 """
 lit - LLVM Integrated Tester.
 
 See lit.pod for more information.
 """
 
 from __future__ import absolute_import
 import os
 import platform
 import random
 import re
+import shlex
 import sys
 import time
 import argparse
 import tempfile
 import shutil
 from xml.sax.saxutils import quoteattr
 
 import lit.ProgressBar
debug_group.add_argument("--debug",
                       action="store_true", default=False)
     debug_group.add_argument("--show-suites", dest="showSuites",
                       help="Show discovered test suites",
                       action="store_true", default=False)
     debug_group.add_argument("--show-tests", dest="showTests",
                       help="Show all discovered tests",
                       action="store_true", default=False)
 
-    opts = parser.parse_args()
+    opts = parser.parse_args(sys.argv[1:] +
+                             shlex.split(os.environ.get("LIT_OPTS", "")))
     args = opts.test_paths
 
     if opts.show_version:
         print("lit %s" % (lit.__version__,))
         return
 
     if not args:
         parser.error('No inputs specified')

llvm/trunk/utils/lit/tests/Inputs/lit-opts/lit.cfg

+import lit.formats
+config.name = 'lit-opts'
+config.suffixes = ['.txt']
+config.test_format = lit.formats.ShTest()
+config.test_source_root = None
+config.test_exec_root = None
+config.substitutions.append(('%var', lit_config.params.get('var', '')))

llvm/trunk/utils/lit/tests/Inputs/lit-opts/test.txt

+# RUN: echo %var

llvm/trunk/utils/lit/tests/lit-opts.py

+# Check cases where LIT_OPTS has no effect.
+#
+# RUN:                 %{lit} -j 1 -s %{inputs}/lit-opts | FileCheck %s
+# RUN: env LIT_OPTS=   %{lit} -j 1 -s %{inputs}/lit-opts | FileCheck %s
+# RUN: env LIT_OPTS=-s %{lit} -j 1 -s %{inputs}/lit-opts | FileCheck %s
+
+# Check that LIT_OPTS can override command-line options.
+#
+# RUN: env LIT_OPTS=-a \
+# RUN: %{lit} -j 1 -s %{inputs}/lit-opts \
+# RUN: | FileCheck -check-prefix=SHOW-ALL -DVAR= %s
+
+# Check that LIT_OPTS understands multiple options with arbitrary spacing.
+#
+# RUN: env LIT_OPTS='-a -v  -Dvar=foobar' \
+# RUN: %{lit} -j 1 -s %{inputs}/lit-opts \
+# RUN: | FileCheck -check-prefix=SHOW-ALL -DVAR=foobar %s
+
+# Check that LIT_OPTS parses shell-like quotes and escapes.
+#
+# RUN: env LIT_OPTS='-a   -v -Dvar="foo bar"\ baz' \
+# RUN: %{lit} -j 1 -s %{inputs}/lit-opts \
+# RUN: | FileCheck -check-prefix=SHOW-ALL -DVAR="foo bar baz" %s
+
+# CHECK:      Testing: 1 tests
+# CHECK-NOT:  PASS
+# CHECK:      Expected Passes : 1
+
+# SHOW-ALL:     Testing: 1 tests
+# SHOW-ALL:     PASS: lit-opts :: test.txt (1 of 1)
+# SHOW-ALL:     {{^}}[[VAR]]
+# SHOW-ALL-NOT: PASS
+# SHOW-ALL:     Expected Passes : 1