[PATCH] D77818: [lit] Print substitutions with --show-suites

[Julian Lettner via Phabricator via llvm-commits]

yln accepted this revision.
yln added a comment.
This revision is now accepted and ready to land.

I think --show-suites is already a debugging feature (to figure out what has been discovered) so a bit more verbosity shouldn't hurt.

LGTM, with nits.



================
Comment at: llvm/utils/lit/lit/main.py:140
             print('    Exec Root  : %s' % suite.exec_root)
             if suite.config.available_features:
                 features = ' '.join(sorted(suite.config.available_features))
----------------
Let's change this to always print `available_features` and `substitutions`.  Not having any is rare and that is information as well.


================
Comment at: llvm/utils/lit/lit/main.py:142
                 features = ' '.join(sorted(suite.config.available_features))
                 print('    Available Features : %s' % features)
+            if suite.config.substitutions:
----------------
Let's remove the space before the colon here.  For Source/Exec Root it aligns the output, but for features and substitutions I don't think that it improves legibility.


================
Comment at: llvm/utils/lit/lit/main.py:144
+            if suite.config.substitutions:
+                first = lambda (x, y): x
+                substitutions = sorted(suite.config.substitutions, key=first)
----------------
I think tuples already have a good natural sort.


================
Comment at: llvm/utils/lit/lit/main.py:147
+                substitutions = ('%s => %s' % (x, y) for (x, y) in substitutions)
+                substitutions = '\n                              '.join(substitutions)
+                print('    Available Substitutions : %s' % substitutions)
----------------
Maybe use `ljust` here?


================
Comment at: llvm/utils/lit/tests/Inputs/discovery/lit.cfg:17
+# Check that available_features are printed by --show-suites
+config.available_features = ['feature1', 'feature2']
+
----------------
Thanks for adding a test for an existing feature!  :)


================
Comment at: llvm/utils/lit/tests/Inputs/discovery/lit.cfg:20
+# Check that substitutions are printed by --show-suites
+config.substitutions = [('%key1', 'value1'), ('%key2', 'value2')]
----------------
Maybe provide unsorted entries here (also for features) so that the test proves that we sort it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77818