[llvm-commits] [llvm] r122341 - in /llvm/trunk: docs/TestingGuide.html test/lit.cfg utils/lit/lit/TestRunner.py

Tue Dec 21 09:12:49 PST 2010

On 12/21/2010 11:55 AM, David Greene wrote:
> Author: greened
> Date: Tue Dec 21 10:55:53 2010
> New Revision: 122341
>
> URL: http://llvm.org/viewvc/llvm-project?rev=122341&view=rev
> Log:
>
> Fix PR 8199.  This patch prepends the build tool dir to LLVM programs
> being tested.  This ensures that we test the tools just built and not
> some random tools that might happen to be in the user's PATH.  This
> makes LLVM testing much more stable and predictable.

Hey David,

sorry for not having responded to your replies on the bug report. I 
forgot to add myself to the CC, so did not get the replies.

I just read your replies and have still an open question. You replied 
the patch provided by me, that prepends the llvm tools directory to the 
path will not solve the problem as it is too brittle. Can you explain this.

I know that the path can contain arbitray binaries and libraries, 
however they are always searched first in the leftmost directories of 
the path. So by prepending /our/new/tools/dir:<arbitraryDangerousPath> 
we will always get the right binaries. The problem of this bug was, that 
we created PATH=/system/path/with/llvm-gcc/:/our/new/tools/dir:...
So any old llvm tools that where installed next to llvm-gcc where 
preferred over the ones in /our/new/tools/dir. This can be solved by 
always putting /our/new/tools/dir leftmost.
Like:

PATH=/usr/new/tools/dir:/system/path/with/llvm-gcc:...

This seems for me to solve the issue without having to add a lot of 
subsitutions.

Why do you think this solution is brittle?

Cheers
Tobi

P.S.: Again, sorry for missing the bug report replies

> Modified:
>      llvm/trunk/docs/TestingGuide.html
>      llvm/trunk/test/lit.cfg
>      llvm/trunk/utils/lit/lit/TestRunner.py
>
> Modified: llvm/trunk/docs/TestingGuide.html
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/docs/TestingGuide.html?rev=122341&r1=122340&r2=122341&view=diff
> ==============================================================================
> --- llvm/trunk/docs/TestingGuide.html (original)
> +++ llvm/trunk/docs/TestingGuide.html Tue Dec 21 10:55:53 2010
> @@ -376,6 +376,11 @@
>     shell. Consequently the syntax differs from normal shell script syntax in a
>     few ways.  You can specify as many RUN lines as needed.</p>
>
> +<p>lit performs substitution on each RUN line to replace LLVM tool
> +  names with the full paths to the executable built for each tool (in
> +  $(LLVM_OBJ_ROOT)/$(BuildMode)/bin).  This ensures that lit does not
> +  invoke any stray LLVM tools in the user's path during testing.</p>
> +
>     <p>Each RUN line is executed on its own, distinct from other lines unless
>     its last character is<tt>\</tt>. This continuation character causes the RUN
>     line to be concatenated with the next one. In this way you can build up long
>
> Modified: llvm/trunk/test/lit.cfg
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/lit.cfg?rev=122341&r1=122340&r2=122341&view=diff
> ==============================================================================
> --- llvm/trunk/test/lit.cfg (original)
> +++ llvm/trunk/test/lit.cfg Tue Dec 21 10:55:53 2010
> @@ -4,6 +4,7 @@
>
>   import os
>   import sys
> +import re
>
>   # name: The name of this test suite.
>   config.name = 'LLVM'
> @@ -148,6 +149,44 @@
>       else:
>           config.substitutions.append(('%' + sub, site_exp[sub]))
>
> +# For each occurrence of an llvm tool name as its own word, replace it
> +# with the full path to the build directory holding that tool.  This
> +# ensures that we are testing the tools just built and not some random
> +# tools that might happen to be in the user's PATH.  Thus this list
> +# includes every tool placed in $(LLVM_OBJ_ROOT)/$(BuildMode)/bin
> +# (llvm_tools_dir in lit parlance).
> +                # Don't match 'bugpoint-'.
> +for pattern in [r"\bbugpoint\b(?!-)",   r"\bclang\b",
> +                r"\bedis\b",            r"\bgold\b",
> +                r"\bllc\b",             r"\blli\b",
> +                r"\bllvm-ar\b",         r"\bllvm-as\b",
> +                r"\bllvm-bcanalyzer\b", r"\bllvm-config\b",
> +                r"\bllvm-diff\b",       r"\bllvm-dis\b",
> +                r"\bllvm-extract\b",    r"\bllvm-ld\b",
> +                r"\bllvm-link\b",       r"\bllvm-mc\b",
> +                r"\bllvm-nm\b",         r"\bllvm-prof\b",
> +                r"\bllvm-ranlib\b",     r"\bllvm-shlib\b",
> +                r"\bllvm-stub\b",       r"\bllvm2cpp\b",
> +                # Don't match '-llvmc'.
> +                r"(?<!-)\bllvmc\b",     r"\blto\b",
> +                                        # Don't match '.opt', '-opt'
> +                                        # or '^opt'.
> +                r"\bmacho-dump\b",      r"(?<!\.|-|\^)\bopt\b",
> +                r"\btblgen\b",          r"\bFileCheck\b",
> +                r"\bFileUpdate\b",      r"\bc-index-test\b",
> +                r"\bfpcmp\b",           r"\bllvm-PerfectShuffle\b",
> +                # Handle these specially as they are strings searched
> +                # for during testing.
> +                r"\| \bcount\b",         r"\| \bnot\b"]:
> +    # Extract the tool name from the pattern.  This relies on the tool
> +    # name being surrounded by \b word match operators.  If the
> +    # pattern starts with "| ", include it in the string to be
> +    # substituted.
> +    substitution = re.sub(r"^(\\)?((\| )?)\W+b([0-9A-Za-z-_]+)\\b\W*$",
> +                          r"\2" + llvm_tools_dir + "/" + r"\4",
> +                          pattern)
> +    config.substitutions.append((pattern, substitution))
> +
>   excludes = []
>
>   # Provide target_triple for use in XFAIL and XTARGET.
>
> Modified: llvm/trunk/utils/lit/lit/TestRunner.py
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/lit/lit/TestRunner.py?rev=122341&r1=122340&r2=122341&view=diff
> ==============================================================================
> --- llvm/trunk/utils/lit/lit/TestRunner.py (original)
> +++ llvm/trunk/utils/lit/lit/TestRunner.py Tue Dec 21 10:55:53 2010
> @@ -8,6 +8,8 @@
>   import platform
>   import tempfile
>
> +import re
> +
>   class InternalShellError(Exception):
>       def __init__(self, command, message):
>           self.command = command
> @@ -444,11 +446,13 @@
>               if ln[ln.index('END.'):].strip() == 'END.':
>                   break
>
> -    # Apply substitutions to the script.
> +    # Apply substitutions to the script.  Allow full regular
> +    # expression syntax.  Replace each matching occurrence of regular
> +    # expression pattern a with substitution b in line ln.
>       def processLine(ln):
>           # Apply substitutions
>           for a,b in substitutions:
> -            ln = ln.replace(a,b)
> +            ln = re.sub(a, b, ln)
>
>           # Strip the trailing newline and any extra whitespace.
>           return ln.strip()
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits