<div dir="ltr">As discussed on IRC, etc.<br><br>The split DWARF llvm-symbolizer tests are a bit finicky & had hardcoded in the data files, the relative directory between the working directory where llvm-symbolizer would be run from, and the location of the .dwo files that it needed to load. This was "Output" originally, but needs to change for the test directory layout changes you're proposing.<br><br>In theory, based on your results, it sounds like this should be "." now (the .dwo file placed in the %T directory, and the %T should be the current working directory of llvm-symbolizer). But that's not what I'm observing locally - maybe there's some platform difference? Perhaps you could provide the patch to llvm-symbolizer you made to test the working directory?<br><br>In any case, I made a review with all the split-dwarf fixes (and a bunch of missing source files to help produce some of the binaries) here: <a href="https://reviews.llvm.org/D34778" target="_blank">https://reviews.llvm.org/D34778</a> <br><br>You can verify that the relative directory these tests are using is 'DebugInfo' by running llvm-dwarfdump -debug-dump=info on the .o or executable files & see the DW_AT_compilation_dir attribute's value.<br><br>Also try running llvm-symbolizer from the various different directories yourself to see when it successfully finds the .dwo files and when it doesn't (general rule of thumb is that if it prints a simple function name ('f2') then it hasn't found the .dwo file, if it prints the mangled name ('_Z2f2v') then it has).<br><br>TBH there's still a few parts of that test that confuse me (well, the fact that they pass with your change confuses me... but at least there's something to start/discuss)<br><br><br>I'm worried there may be some non-portability if you were observing the working directory as test/Output/DebugInfo and I seem to be observing it as test/Output. If these tests work for you, that sort of points to the working directory (at some level, in some way - maybe there's some specific handling in llvm-symbolizer that's resolving the path not just relative to the current working directory, but something else? Maybe we can manipulate/fix that so it is test/Output/DebugInfo)  being test/Output. Give it a go & see?<br><br><br>eg (showing that it passes when run from the test/Output directory, not the test/Output/DebugInfo directory): <br><br><div><font face="monospace">$ /usr/local/google/home/blaikie/dev/llvm/build/default/./bin/llvm-symbolizer --functions=linkage --inlining --demangle=false     --default-arch=i386 < /usr/local/google/home/blaikie/dev/llvm/build/default/test/Output/DebugInfo/llvm-symbolizer.test.tmp.input | /usr/local/google/home/blaikie/dev/llvm/build/default/./bin/FileCheck --check-prefix=SPLIT --check-prefix=DWO --check-prefix=EXTRA /usr/local/google/home/blaikie/dev/llvm/src/test/DebugInfo/llvm-symbolizer.test</font></div><div><font face="monospace">/usr/local/google/home/blaikie/dev/llvm/src/test/DebugInfo/llvm-symbolizer.test:129:13: error: SPLIT-NEXT: is not on the line after the previous match</font></div><div><font face="monospace">SPLIT-NEXT: {{.*}}split-dwarf-test.cc</font></div><div><font face="monospace">            ^</font></div><div><font face="monospace"><stdin>:111:1: note: 'next' match was here</font></div><div><font face="monospace">Output/split-dwarf-test.cc:5:3</font></div><div><font face="monospace">^</font></div><div><font face="monospace"><stdin>:105:7: note: previous match ended here</font></div><div><font face="monospace">_Z2f2v</font></div><div><font face="monospace">      ^</font></div><div><font face="monospace"><stdin>:106:1: note: non-matching line after previous match is here</font></div><div><font face="monospace">Output/split-dwarf-dwp.cpp:3:3</font></div><div><font face="monospace">^</font></div><div><font face="monospace">blaikie@blaikie-linux2:~/dev/llvm/build/default$ cd test</font></div><div><font face="monospace">blaikie@blaikie-linux2:~/dev/llvm/build/default/test$ /usr/local/google/home/blaikie/dev/llvm/build/default/./bin/llvm-symbolizer --functions=linkage --inlining --demangle=false     --default-arch=i386 < /usr/local/google/home/blaikie/dev/llvm/build/default/test/Output/DebugInfo/llvm-symbolizer.test.tmp.input | /usr/local/google/home/blaikie/dev/llvm/build/default/./bin/FileCheck --check-prefix=SPLIT --check-prefix=DWO --check-prefix=EXTRA /usr/local/google/home/blaikie/dev/llvm/src/test/DebugInfo/llvm-symbolizer.test</font></div><div><font face="monospace">/usr/local/google/home/blaikie/dev/llvm/src/test/DebugInfo/llvm-symbolizer.test:129:13: error: SPLIT-NEXT: is not on the line after the previous match</font></div><div><font face="monospace">SPLIT-NEXT: {{.*}}split-dwarf-test.cc</font></div><div><font face="monospace">            ^</font></div><div><font face="monospace"><stdin>:111:1: note: 'next' match was here</font></div><div><font face="monospace">Output/split-dwarf-test.cc:5:3</font></div><div><font face="monospace">^</font></div><div><font face="monospace"><stdin>:105:7: note: previous match ended here</font></div><div><font face="monospace">_Z2f2v</font></div><div><font face="monospace">      ^</font></div><div><font face="monospace"><stdin>:106:1: note: non-matching line after previous match is here</font></div><div><font face="monospace">Output/split-dwarf-dwp.cpp:3:3</font></div><div><font face="monospace">^</font></div><div><font face="monospace">blaikie@blaikie-linux2:~/dev/llvm/build/default/test$ cd Output/</font></div><div><font face="monospace">blaikie@blaikie-linux2:~/dev/llvm/build/default/test/Output$ /usr/local/google/home/blaikie/dev/llvm/build/default/./bin/llvm-symbolizer --functions=linkage --inlining --demangle=false     --default-arch=i386 < /usr/local/google/home/blaikie/dev/llvm/build/default/test/Output/DebugInfo/llvm-symbolizer.test.tmp.input | /usr/local/google/home/blaikie/dev/llvm/build/default/./bin/FileCheck --check-prefix=SPLIT --check-prefix=DWO --check-prefix=EXTRA /usr/local/google/home/blaikie/dev/llvm/src/test/DebugInfo/llvm-symbolizer.test</font></div><div><font face="monospace">blaikie@blaikie-linux2:~/dev/llvm/build/default/test/Output$ cd DebugInfo/</font></div><div><font face="monospace">blaikie@blaikie-linux2:~/dev/llvm/build/default/test/Output/DebugInfo$ /usr/local/google/home/blaikie/dev/llvm/build/default/./bin/llvm-symbolizer --functions=linkage --inlining --demangle=false     --default-arch=i386 < /usr/local/google/home/blaikie/dev/llvm/build/default/test/Output/DebugInfo/llvm-symbolizer.test.tmp.input | /usr/local/google/home/blaikie/dev/llvm/build/default/./bin/FileCheck --check-prefix=SPLIT --check-prefix=DWO --check-prefix=EXTRA /usr/local/google/home/blaikie/dev/llvm/src/test/DebugInfo/llvm-symbolizer.test</font></div><div><font face="monospace">/usr/local/google/home/blaikie/dev/llvm/src/test/DebugInfo/llvm-symbolizer.test:129:13: error: SPLIT-NEXT: is not on the line after the previous match</font></div><div><font face="monospace">SPLIT-NEXT: {{.*}}split-dwarf-test.cc</font></div><div><font face="monospace">            ^</font></div><div><font face="monospace"><stdin>:111:1: note: 'next' match was here</font></div><div><font face="monospace">Output/split-dwarf-test.cc:5:3</font></div><div><font face="monospace">^</font></div><div><font face="monospace"><stdin>:105:7: note: previous match ended here</font></div><div><font face="monospace">_Z2f2v</font></div><div><font face="monospace">      ^</font></div><div><font face="monospace"><stdin>:106:1: note: non-matching line after previous match is here</font></div><div><font face="monospace">Output/split-dwarf-dwp.cpp:3:3</font></div><div><font face="monospace">^</font></div><div><br></div><br><div class="gmail_quote"><div dir="ltr">On Wed, Jun 28, 2017 at 10:16 AM Reid Kleckner via Phabricator <<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">rnk added inline comments.<br>
<br>
<br>
================<br>
Comment at: llvm/utils/lit/lit/run.py:254<br>
+        # runs' output.<br>
+        import time<br>
+        start = time.time()<br>
----------------<br>
The time module is already available at global scope<br>
<br>
<br>
================<br>
Comment at: llvm/utils/lit/lit/run.py:260<br>
+        clean_paths = list(clean_paths)<br>
+        clean_paths.sort(key=lambda x: len(x.split(os.sep)))<br>
+        for base in clean_paths:<br>
----------------<br>
Can you add a comment, something like "Sort the list by number of path components so that parent directories come before their children."<br>
<br>
<br>
================<br>
Comment at: llvm/utils/lit/lit/run.py:264<br>
+                if not os.path.islink(base) and os.path.isdir(base):<br>
+                    from shutil import rmtree<br>
+                    rmtree(base, True)<br>
----------------<br>
Let's just import shutil at global scope and do shutil.rmtree.<br>
<br>
<br>
================<br>
Comment at: llvm/utils/lit/lit/run.py:271<br>
+        end = time.time()<br>
+        print("Cleanup took {} seconds".format(end - start))<br>
+<br>
----------------<br>
Should we print this at all? If we decide we want it, should we print with litConfig.note()?<br>
<br>
<br>
<a href="https://reviews.llvm.org/D34732" rel="noreferrer" target="_blank">https://reviews.llvm.org/D34732</a><br>
<br>
<br>
<br>
</blockquote></div></div>