Fix argdumper build in cmake (OS X) after r228636

[ki.stfu]

Summary

This patch fixes the following tests on OS X:

FAIL: test_with_dsym (TestLaunchWithGlob.LaunchWithGlobTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/testuser/build/workspace/LLDB_master_release_OSX/llvm_master/tools/lldb/test/lldbtest.py", line 456, in wrapper
    return func(self, *args, **kwargs)
  File "/Users/testuser/build/workspace/LLDB_master_release_OSX/llvm_master/tools/lldb/test/functionalities/launch_with_glob/TestLaunchWithGlob.py", line 21, in test_with_dsym
    self.do_test ()
  File "/Users/testuser/build/workspace/LLDB_master_release_OSX/llvm_master/tools/lldb/test/functionalities/launch_with_glob/TestLaunchWithGlob.py", line 42, in do_test
    self.runCmd("process launch -G true -w %s -- fi*.tx?" % (os.getcwd()))
  File "/Users/testuser/build/workspace/LLDB_master_release_OSX/llvm_master/tools/lldb/test/lldbtest.py", line 1953, in runCmd
    msg if msg else CMD_MSG(cmd))
AssertionError: False is not True : Command 'process launch -G true -w /Users/testuser/build/workspace/LLDB_master_release_OSX/llvm_master/tools/lldb/test/functionalities/launch_with_glob -- fi*.tx?' returns successfully
Config=x86_64-clang
======================================================================
FAIL: test_with_dwarf (TestLaunchWithGlob.LaunchWithGlobTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/testuser/build/workspace/LLDB_master_release_OSX/llvm_master/tools/lldb/test/lldbtest.py", line 473, in wrapper
    return func(self, *args, **kwargs)
  File "/Users/testuser/build/workspace/LLDB_master_release_OSX/llvm_master/tools/lldb/test/functionalities/launch_with_glob/TestLaunchWithGlob.py", line 28, in test_with_dwarf
    self.do_test ()
  File "/Users/testuser/build/workspace/LLDB_master_release_OSX/llvm_master/tools/lldb/test/functionalities/launch_with_glob/TestLaunchWithGlob.py", line 42, in do_test
    self.runCmd("process launch -G true -w %s -- fi*.tx?" % (os.getcwd()))
  File "/Users/testuser/build/workspace/LLDB_master_release_OSX/llvm_master/tools/lldb/test/lldbtest.py", line 1953, in runCmd
    msg if msg else CMD_MSG(cmd))
AssertionError: False is not True : Command 'process launch -G true -w /Users/testuser/build/workspace/LLDB_master_release_OSX/llvm_master/tools/lldb/test/functionalities/launch_with_glob -- fi*.tx?' returns successfully

[zturner]

Sorry for the ignorance, but what is argdumper and why is it Mac only?

[ki.stfu]

As I understood it's for globbing. See here:

• http://llvm.org/viewvc/llvm-project?view=revision&revision=228636
• http://llvm.org/viewvc/llvm-project?view=revision&revision=228658
• http://llvm.org/viewvc/llvm-project?view=revision&revision=228659

[zturner]

+enrico

But why is it only for mac? We shouldn't be adding features that are only
available on specific platforms without good reason.

[ki.stfu]

Tests were marked as "Darwin only" (see test/functionalities/launch_with_glob/TestLaunchWithGlob.py), therefore I have fixed it only for os x.

[ki.stfu]

Should I port it on Windows? or non-windows platforms only?

[ki.stfu]

Hello Enrico,

RE: I assume that the lack of this build logic was causing the test case to fail on your machine. Correct?

Correct. But tests had failed on my side because argdumper not found. I use cmake to build lldb.

Log:

1: test_with_dsym (TestLaunchWithGlob.LaunchWithGlobTestCase) ...
os command: make clean ; make MAKE_DSYM=YES ARCH=x86_64 CC="clang"
with pid: 94168
stdout: rm -f "a.out"  main.o main.d main.d.tmp
rm -f -r "a.out.dSYM"
clang++ -std=c++11 -g -O0 -arch x86_64   -I/Users/IliaK/p/llvm/tools/lldb/test/make/../../include   -c -o main.o main.cpp
clang++  main.o -g -O0 -arch x86_64   -I/Users/IliaK/p/llvm/tools/lldb/test/make/../../include   -o "a.out"
"/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/dsymutil"  -o "a.out.dSYM" "a.out"

stderr:
retcode: 0


runCmd: file /Users/IliaK/p/llvm/tools/lldb/test/functionalities/launch_with_glob/a.out
output: Current executable set to '/Users/IliaK/p/llvm/tools/lldb/test/functionalities/launch_with_glob/a.out' (x86_64).


/Users/IliaK/p/llvm/build_ninja/lib/python2.7/site-packages/lldb
runCmd: process launch -G true -w /Users/IliaK/p/llvm/tools/lldb/test/functionalities/launch_with_glob -- fi*.tx?
runCmd failed!
error: could not find argdumper tool


Command 'process launch -G true -w /Users/IliaK/p/llvm/tools/lldb/test/functionalities/launch_with_glob -- fi*.tx?' failed!

/Users/IliaK/p/llvm/build_ninja/lib/python2.7/site-packages/lldb
runCmd: process launch -G true -w /Users/IliaK/p/llvm/tools/lldb/test/functionalities/launch_with_glob -- fi*.tx?
runCmd failed!
error: could not find argdumper tool


Command 'process launch -G true -w /Users/IliaK/p/llvm/tools/lldb/test/functionalities/launch_with_glob -- fi*.tx?' failed!

/Users/IliaK/p/llvm/build_ninja/lib/python2.7/site-packages/lldb
runCmd: process launch -G true -w /Users/IliaK/p/llvm/tools/lldb/test/functionalities/launch_with_glob -- fi*.tx?
runCmd failed!
error: could not find argdumper tool


Command 'process launch -G true -w /Users/IliaK/p/llvm/tools/lldb/test/functionalities/launch_with_glob -- fi*.tx?' failed!

FAILURE

[ki.stfu]

Fix argdumper build in cmake on all platforms

This was tested only on OS X. Could somebody test it on others?

[zturner]

I was objecting to both things actually:

1. Disabled in the CMake build that Ilia ported
2. Test disabled on non mac.

We should be enabling tests in all platforms by default, and if it breaks
someone should be fixing the tests.

Also, I know you guys use the Xcode build primarily, but i think for large
changes like this it would be helpful to at least try to fix the CMake
build. Otherwise there's a good chance nobody will notice the change go
through and the feature will not be added to other builds until someday
when it breaks a test.

[ki.stfu]

Enable dwarf test on non-mac platforms

[ki.stfu]

In D7550#122016, @zturner wrote:

I was objecting to both things actually:

1. Disabled in the CMake build that Ilia ported
2. Test disabled on non mac.

We should be enabling tests in all platforms by default, and if it breaks
someone should be fixing the tests.

Done. Now it's ready for submitting?

[zturner]

scripts/Python/finishSwigPythonLLDB.py

466–468

I think this won't work on Windows as written? The previous code was using make_symlink_windows() and make_symlink_other_platforms. make_symlink_windows makes a hardlink because os.symlink is not supported on Windows for some reason. So I think make_symlink_argdumper() should do the same. It would be nice if we wrapped this into an lldb_make_symlink(source, target) or something, so we didn't have to worry about this in the future.

[ki.stfu]

scripts/Python/finishSwigPythonLLDB.py

255

Is it an error? It should be "../../../liblldb%s"?

[ki.stfu]

Refactor scripts/Python/finishSwigPythonLLDB.py

[ki.stfu]

Note: I assumed that it was an error on line #255 (see D7550#122560) therefore I fixed it in Diff 4.

[zturner]

I will test this patch out on Windows.

[zturner]

Actually the original was correct. On Windows our directory structure looks like this:

ninja
--bin
----liblldb.dll
--lib
----site-packages
------lldb
--------lldb_d.pyd

So the lldb_d.pyd needs to be a link to the liblldb.dll. I will test with the correct relative path back in place and let you know.

[clayborg]

I will stay out of this one since it mostly affects the CMake and Make builds.

[zturner]

Greg: At some point I would like to switch the Xcode build over to using
the SWIG python scripts. There's no reason to maintain two parallel sets
of scripts that do the same thing (and we can't assume an environment that
will be capable of running shell scripts on Windows). Might not matter for
the purposes of this review, but just a note.

[zturner]

There's no function maked make_symlink_lldb anywhere, so it's failing.

[zturner]

Going to commandeer this revision because it's the only way I can attach a patch to it.

[zturner]

In the future, please try to test the python changes more thoroughly. I know we can't expect everyone to test everything on every platform, but I'm not sure this was tested at all. There were lots of places not in Windows specific codepaths that had syntax errors. Like calling a function that didn't exist, returning a local variable of the wrong name, spaces instead of tabs, and more.

I'm glad that there's more people digging into this stuff, but please make sure to test sufficiently. I'm uploading a new version of this which makes it work insofar as I can test. Please feel free to download this revision and make sure it works on other platforms.

[zturner]

BTW, you can commandeer this revision back to re-take ownership of it. Use the combo box at the bottom of the page on the Phabricator web interface.

[zturner]

I accidentally left in a stray debugging message in the CMakeLists.txt. This update removes that.

[ki.stfu]

In D7550#122769, @zturner wrote:

In the future, please try to test the python changes more thoroughly. I know we can't expect everyone to test everything on every platform, but I'm not sure this was tested at all. There were lots of places not in Windows specific codepaths that had syntax errors. Like calling a function that didn't exist, returning a local variable of the wrong name, spaces instead of tabs, and more.

I'm glad that there's more people digging into this stuff, but please make sure to test sufficiently. I'm uploading a new version of this which makes it work insofar as I can test. Please feel free to download this revision and make sure it works on other platforms.

Sorry. I want to see what you had changed but when I selected Diff 4 in red column and Diff 6 in green column, it looked like you had removed all my changes in python scripts (see here). Is it a phabricator bug?

[ki.stfu]

In D7550#122566, @ki.stfu wrote:

Is it an error? It should be "../../../liblldb%s"?

In D7550#122703, @zturner wrote:

Actually the original was correct. On Windows our directory structure looks like this:

ninja
--bin
----liblldb.dll
--lib
----site-packages
------lldb
--------lldb_d.pyd

So the lldb_d.pyd needs to be a link to the liblldb.dll. I will test with the correct relative path back in place and let you know.

You missed "python2.7" folder in "lib", which contains "site-packages" folder.

If so, then path should be "../../../../bin/liblldb%s".

[zturner]

Im seeing the same issue the diff. Try applying the patch and use git diff.
Does that work?

[zturner]

Hi Ilia, please see D7598 where I've attempted to rectify the messed up diff. Let me know if that patch works for on your platforms.

[ki.stfu]

In D7550#122832, @zturner wrote:

Im seeing the same issue the diff. Try applying the patch and use git diff.
Does that work?

Yes. It is shown correctly on my computer.

[ki.stfu]

In D7550#122998, @zturner wrote:

Hi Ilia, please see D7598 where I've attempted to rectify the messed up diff. Let me know if that patch works for on your platforms.

Thanks for created review. It was simpler to verify your changes. I have tested Diff 6 on OS X, it works. I think we can close D7598 and commit patch it from here. I'll do it today, if you don't mind.

[ki.stfu]

without any changes

[ki.stfu]

oops. sorry guys. it's my local files.

[ki.stfu]

restore Diff 6

[ki.stfu]

Restore to Diff 6

[ki.stfu]

restore tools/argdumper/CMakeLists.txt

[ovyalov]

test/functionalities/launch_with_glob/TestLaunchWithGlob.py

24 ↗

(On Diff #19922)

It seems this test is failing on Linux - could we mark it "requires Darwin" for now?

[zturner]

Hi Oleksiy, I spent a few minutes looking into this test. Can you try
checking what CanDebugProcess() returns for your Platform implementation?
If it returns false, change it to true and see if this test still fails for
you.

scripts/Python/finish-swig-Python-LLDB.sh

else
     fi
 else
     if [ $Debug -eq 1 ]
     then
         echo "${framework_python_dir}/_lldb.so already exists."
     fi
 fi
 
+# Make symlink for darwin-debug on Darwin
 if [ ${OS_NAME} = "Darwin" ] && [ $MakefileCalled -ne 0 ]
 then
     # We are being built by CMake on Darwin
 
     if [ ! -L "${framework_python_dir}/darwin-debug" ]
     then
         if [ $Debug -eq 1 ]
         then
             echo "Creating symlink for darwin-debug"
         fi
         cd "${framework_python_dir}"
         ln -s "../../../../bin/lldb-launcher" darwin-debug
     else
         if [ $Debug -eq 1 ]
         then
             echo "${framework_python_dir}/darwin-debug already exists."
         fi
     fi
 fi
 
+# Make symlink for argdumper on any platform
+if [ $MakefileCalled -ne 0 ]
+then
+    # We are being built by CMake
+
+    if [ ! -L "${framework_python_dir}/argdumper" ]
+    then
+        if [ $Debug -eq 1 ]
+        then
+            echo "Creating symlink for argdumper"
+        fi
+        cd "${framework_python_dir}"
+        ln -s "../../../../bin/argdumper" argdumper
+    else
+        if [ $Debug -eq 1 ]
+        then
+            echo "${framework_python_dir}/argdumper already exists."
+        fi
+    fi
+fi
+
 create_python_package () {
     package_dir="${framework_python_dir}$1"
     package_files="$2"
     package_name=`echo $1 | tr '/' '.'`
     package_name="lldb${package_name}"
 
     if [ ! -d "${package_dir}" ]
     then

scripts/Python/finishSwigPythonLLDB.py

def make_symlink_windows( vDictArgs, vstrFrameworkPythonDir, vstrDllName ):
   strSrc = "";
 
   os.chdir( vstrFrameworkPythonDir );
   bMakeFileCalled = vDictArgs.has_key( "-m" );
   if not bMakeFileCalled:
     strSrc = os.path.normcase( "../../../LLDB" );
   else:
     strLibFileExtn = ".dll";
     strSrc = os.path.normcase( "../../../bin/liblldb%s" % strLibFileExtn );
 
   if os.path.isfile( strTarget ):
     if bDbg:
       print strMsglldbsoExists % strTarget;
     return (bOk, strMsg);
 
   if bDbg:
     print strMsglldbsoMk % (os.path.abspath(strSrc), os.path.abspath(strTarget));
except OSError as e:
     strMsg += " Src:'%s' Target:'%s'" % (strSrc, strTarget);
   except:
     bOk = False;
     strMsg = strErrMsgUnexpected % sys.exec_info()[ 0 ];
   
   return (bOk, strMsg);
 
 #++---------------------------------------------------------------------------
+# Details:  Make the symbolic link to the argdumper. Code for all platforms
+#           apart from Windows.
+# Args:   vDictArgs       - (R) Program input parameters.
+#     vstrFrameworkPythonDir  - (R) Python framework directory.
+#     vstrArgdumperFileName - (R) File name for argdumper.
+# Returns:  Bool - True = function success, False = failure.
+#     Str - Error description on task failure.
+# Throws: None.
+#--
+def make_symlink_argdumper( vDictArgs, vstrFrameworkPythonDir, vstrArgdumperFileName ):
+  dbg = utilsDebug.CDebugFnVerbose( "Python script make_symlink_other_platforms()" );
+  bOk = True;
+  strMsg = "";
+  bDbg = vDictArgs.has_key( "-d" );
+  strTarget = vstrArgdumperFileName
+  strArgdumperPath = "%s/%s" % (vstrFrameworkPythonDir, strTarget);
+  strTarget = os.path.normcase( strArgdumperPath );
+  strSrc = "";
+      
+  os.chdir( vstrFrameworkPythonDir );
+  bMakeFileCalled = vDictArgs.has_key( "-m" );
+  if not bMakeFileCalled:
+      return (bOk, strMsg);
+  else:
+    strSrc = os.path.normcase( "../../../../bin/argdumper" );
+
+  if os.path.islink( strTarget ):
+    if bDbg:
+      print strMsglldbsoExists % strTarget;
+    return (bOk, strMsg);
+  
+  if bDbg:
+    print strMsglldbsoMk;
+    
+  try:
+    os.symlink( strSrc, strTarget );
+  except OSError as e:
+    bOk = False;
+    strMsg = "OSError( %d ): %s %s" % (e.errno, e.strerror, strErrMsgMakeSymlink);
+    strMsg += " Src:'%s' Target:'%s'" % (strSrc, strTarget);
+  except:
+    bOk = False;
+    strMsg = strErrMsgUnexpected % sys.exec_info()[ 0 ];
+  
+  return (bOk, strMsg);
+
+#++---------------------------------------------------------------------------
 # Details:  Make the symlink that the script bridge for Python will need in 
 #       the Python framework directory.
 # Args:   vDictArgs       - (R) Program input parameters.
 #       vstrFrameworkPythonDir  - (R) Python framework directory.
 # Returns:  Bool - True = function success, False = failure.
 #     strErrMsg - Error description on task failure.
 # Throws: None.
 #--
elif eOSType == utilsOsType.EnumOsType.Windows:
     bOk, strErrMsg = make_symlink_windows( vDictArgs, 
                          vstrFrameworkPythonDir,
                          strSoFileName );
   else:
     bOk, strErrMsg = make_symlink_other_platforms( vDictArgs, 
                              vstrFrameworkPythonDir,
                              strSoFileName );   
 
-    # Make symlink for darwin-debug
+    # Make symlink for darwin-debug on Darwin
   strDarwinDebugFileName = "darwin-debug"
   if bOk and eOSType == utilsOsType.EnumOsType.Darwin:
     bOk, strErrMsg = make_symlink_darwin_debug( vDictArgs,
                             vstrFrameworkPythonDir,
                             strDarwinDebugFileName );
 
+    # Make symlink for argdumper on any platform
+  strArgdumperFileName = "argdumper"
+  if bOk:
+    bOk, strErrMsg = make_symlink_argdumper( vDictArgs,
+                         vstrFrameworkPythonDir,
+                         strArgdumperFileName );
+
   return (bOk, strErrMsg);
 
 #++---------------------------------------------------------------------------
 # Details:  Look for the directory in which to put the Python files; if it 
 #     does not already exist, attempt to make it.
 # Args:   vDictArgs       - (R) Program input parameters.
 #     vstrFrameworkPythonDir  - (R) Python framework directory.
 # Returns:  Bool - True = function success, False = failure.

tools/CMakeLists.txt

 if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
   add_subdirectory(darwin-debug)
   add_subdirectory(debugserver)
 endif()
+  add_subdirectory(argdumper)
   add_subdirectory(driver)
 if (NOT __ANDROID_NDK__)
   add_subdirectory(lldb-mi)
 endif()
 if (CMAKE_SYSTEM_NAME MATCHES "FreeBSD" OR CMAKE_SYSTEM_NAME MATCHES "Linux")
   add_subdirectory(lldb-gdbserver)
 endif()
 if (NOT CMAKE_SYSTEM_NAME MATCHES "Windows")
   add_subdirectory(lldb-platform)
 endif()

tools/argdumper/CMakeLists.txt

+add_lldb_executable(argdumper
+  argdumper.cpp
+  )
+
+target_link_libraries(argdumper liblldb)
+
+install(TARGETS argdumper
+  RUNTIME DESTINATION bin)