[lldb][test] Toolchain detection rewrite in Python #102185

dzhidzhoev · 2024-08-06T17:11:54Z

This fix is based on a problem with cxx_compiler and cxx_linker macros
on Windows.
There was an issue with compiler detection in paths containing "icc". In
such case, Makefile.rules thought it was provided with icc compiler.

To solve that, utilities detection has been rewritten in Python.
The last element of compiler's path is separated, taking into account
the platform path delimiter, and compiler type is extracted, with regard
of possible cross-toolchain prefix.

llvmbot · 2024-08-06T17:12:14Z

@llvm/pr-subscribers-lldb

Author: Vladislav Dzhidzhoev (dzhidzhoev)

Changes

This fix is based on a problem with cxx_compiler and cxx_linker macros on Windows.
There was an issue with compiler detection in paths containing "icc". In such case, Makefile.rules thought it was provided with ICC compiler.

To solve that, quotes from paths are removed, which may be added when arguments are passed from Python to make, the last element of the compiler's path is separated, taking into account platform path delimiter, and compiler type is extracted, taking into account possible cross-toolchain prefix. Paths for additional tools, like OBJCOPY, are initialized with tools built with LLVM if USE_LLVM_TOOLS is on, to achieve better portability for Windows.

Full diff: https://github.com/llvm/llvm-project/pull/102185.diff

1 Files Affected:

(modified) lldb/packages/Python/lldbsuite/test/make/Makefile.rules (+115-53)

diff --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
index 629ccee32e8404..8676531a8a5d93 100644
--- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -55,7 +55,10 @@ LLDB_BASE_DIR := $(THIS_FILE_DIR)/../../../../../
 # Also reset BUILDDIR value because "pwd" returns cygwin or msys path
 # which needs to be converted to windows path.
 #----------------------------------------------------------------------
+path_wrapper = $(1)
 ifeq "$(HOST_OS)" "Windows_NT"
+	path_wrapper = $(subst \,/,$(1))
+
 	# MinGW make gets $(windir) variable if launched from cmd.exe
 	# and $(WINDIR) if launched from MSYS2.
 	SHELL := $(or $(windir),$(WINDIR),C:\WINDOWS)\system32\cmd.exe
@@ -111,6 +114,110 @@ ifeq "$(CC)" ""
 $(error "C compiler is not specified. Please run tests through lldb-dotest or lit")
 endif
 
+# Remove all " and ' characters from the path. Also remove surrounding space chars.
+cstrip = $(strip $(subst ",,$(subst ',,$(1))))
+
+# We can get CC compiler string in the following formats:
+#  [<tool>] <compiler>    - such as 'xrun clang', 'xrun /usr/bin/clang' & etc
+#
+# Where <compiler> could contain the following parts:
+#   <simple-name>[.<exe-ext>]                           - sucn as 'clang', 'clang.exe' ('clamg-cl.exe'?)
+#   <target-triple>-<simple-name>[.<exe-ext>]           - such as 'armv7-linux-gnueabi-gcc'
+#   <path>/<simple-name>[.<exe-ext>]                    - such as '/usr/bin/clang', 'c:\path\to\compiler\clang,exe'
+#   <path>/<target-triple>-<simple-name>[.<exe-ext>]    - such as '/usr/bin/clang', 'c:\path\to\compiler\clang,exe'
+
+# We need CC without anything around.
+override CC := $(call path_wrapper,$(call cstrip,$(CC)))
+
+CC_EXT := $(suffix $(CC))
+# Compiler name without extension
+CC_NAME := $(basename $(lastword $(subst /, ,$(CC))))
+# A kind of compiler (canonical name): clang, gcc, cc & etc.
+ifeq (,$(filter $(CC_NAME), clang-cl llvm-gcc))
+	CCC := $(or $(lastword $(subst -, ,$(CC_NAME))), $(CC_NAME))
+	# A triple prefix of compiler name: <armv7-none-linux-gnu->gcc
+	CC_PREFIX := $(if $(findstring -,$(CC_NAME)),$(subst -$(CCC),,$(CC_NAME)),)
+else
+	CCC := $(CC_NAME)
+	CC_PREFIX :=
+endif
+
+# Any trace of path within CC?
+# $(dir ) function returns './' for the empty strings, but we need to keep path empty.
+# Use this function on CC only if a path was specified with CC.
+ifneq (,$(findstring /,$(CC)))
+	CC_PATH := $(dir $(lastword $(CC)))
+	replace_cc_with = $(patsubst %/$(CC_NAME)$(CC_EXT),%/$(1)$(CC_EXT),$(CC))
+else
+	CC_PATH :=
+	replace_cc_with = $(patsubst $(CC_NAME)$(CC_EXT),$(1)$(CC_EXT),$(CC))
+endif
+
+# Function returns the tool/compiler name including the triple (or whatnever) prefix
+# if it is present in the original CC.
+cname_with_prefix = $(if $(CC_PREFIX),$(CC_PREFIX)-$(1),$(1))
+
+# These functions return the fully qualified tool name (compiler or whatnevet) based on the previously parsed CC,
+# given a simple tool (clang, gcc, objcopy & etc) name as arg.
+# The first arg is the simplyfied tool name
+# The second arg is a path to the tool (CC_PATH otherwise)
+toolpath_base = $(join $(or $(2),$(CC_PATH)),$(addsuffix $(EXE_EXT),$(1)))
+
+# A kind of C++ compiler. Get the counterpart C++ compiler based on CC/CCC.
+CXXC := $(strip $(if $(filter $(CCC), icc),icpc,\
+                $(if $(filter $(CCC), llvm-gcc),llvm-g++,\
+                $(if $(filter $(CCC), gcc),g++,\
+                $(if $(filter $(CCC), cc),c++,\
+                $(if $(filter $(CCC), clang),clang++,\
+                $(CCC)))))))
+# A name of C++ compiler including the prefix.
+CXX_NAME := $(call cname_with_prefix,$(CXXC))
+
+ifneq "$(LLVM_TOOLS_DIR)" ""
+    override LLVM_TOOLS_DIR := $(call path_wrapper,$(call cstrip,$(LLVM_TOOLS_DIR)))/
+endif
+
+ifeq "$(HOST_OS)" "Windows_NT"
+	wrap_quotes = $(QUOTE)$(1)$(QUOTE)
+	# This function enframes the full path with the platform specific quotes. This is necessary to run the c++ executable
+	# properly under 'sh' on Windows host (prevent the path breakage because of Windows style path separators).
+	cxx_compiler = $(call wrap_quotes,$(call replace_cc_with,$(CXX_NAME)))
+else
+	cxx_compiler = $(call replace_cc_with,$(CXX_NAME))
+endif
+
+# Always override the linker. Assign already normalized CC.
+LD = $(CC)
+# A kind of linker. It always gets retrieved from CC.
+LDC := $(CCC)
+
+# Function that returns the C++ linker, given kind of compiler (CCC or CXXC) as arg.
+cxx_linker = $(call cxx_compiler)
+
+# Note: LLVM_AR is currently required by API TestBSDArchives.py tests.
+# Assembly a full path to llvm-ar for give  n LLVM_TOOLS_DIR;
+# otherwise assume we have llvm-ar at the same place where is CC specified.
+LLVM_AR ?= $(call toolpath_base,llvm-ar,$(LLVM_TOOLS_DIR))
+
+ifneq "$(OS)" "Darwin"
+	# Use the llvm project tool instead of the system defaults.
+	# Note: do not override explicity specified tool from the cmd line.
+	ifdef USE_LLVM_TOOLS
+		override OBJCOPY := $(call toolpath_base,llvm-objcopy,$(LLVM_TOOLS_DIR))
+		override STRIP := $(call toolpath_base,llvm-strip,$(LLVM_TOOLS_DIR))
+		override ARCHIVER := $(call toolpath_base,llvm-ar,$(LLVM_TOOLS_DIR))
+		override AR = $(ARCHIVER)
+		override LLVM_AR = $(ARCHIVER)
+	endif
+	# Assembly a toolchain side tool cmd based on passed CC properties we parsed early.
+	ifneq (,$(filter $(CCC), clang gcc cc))
+		OBJCOPY ?= $(call replace_cc_with,$(call cname_with_prefix,objcopy))
+		STRIP ?= $(call replace_cc_with,$(call cname_with_prefix,strip))
+		ARCHIVER ?= $(call replace_cc_with,$(call cname_with_prefix,ar))
+		override AR = $(ARCHIVER)
+	endif
+endif
+
 #----------------------------------------------------------------------
 # Handle SDKROOT for the cross platform builds.
 #----------------------------------------------------------------------
@@ -273,7 +380,6 @@ endif
 
 CFLAGS += $(CFLAGS_EXTRAS)
 CXXFLAGS += -std=c++11 $(CFLAGS) $(ARCH_CXXFLAGS)
-LD = $(CC)
 # Copy common options to the linker flags (dwarf, arch. & etc).
 # Note: we get some 'garbage' options for linker here (such as -I, --isystem & etc).
 LDFLAGS += $(CFLAGS)
@@ -306,50 +412,6 @@ ifneq "$(DYLIB_NAME)" ""
 	endif
 endif
 
-# Function that returns the counterpart C++ compiler, given $(CC) as arg.
-cxx_compiler_notdir = $(if $(findstring icc,$(1)), \
-			$(subst icc,icpc,$(1)), \
-			$(if $(findstring llvm-gcc,$(1)), \
-				$(subst llvm-gcc,llvm-g++,$(1)), \
-				$(if $(findstring gcc,$(1)), \
-					$(subst gcc,g++,$(1)), \
-					$(subst cc,c++,$(1)))))
-cxx_compiler = $(if $(findstring /,$(1)),$(join $(dir $(1)), $(call cxx_compiler_notdir,$(notdir $(1)))),$(call cxx_compiler_notdir,$(1)))
-
-# Function that returns the C++ linker, given $(CC) as arg.
-cxx_linker_notdir = $(if $(findstring icc,$(1)), \
-			$(subst icc,icpc,$(1)), \
-			$(if $(findstring llvm-gcc,$(1)), \
-				$(subst llvm-gcc,llvm-g++,$(1)), \
-				$(if $(findstring gcc,$(1)), \
-					$(subst gcc,g++,$(1)), \
-					$(subst cc,c++,$(1)))))
-cxx_linker = $(if $(findstring /,$(1)),$(join $(dir $(1)), $(call cxx_linker_notdir,$(notdir $(1)))),$(call cxx_linker_notdir,$(1)))
-
-ifneq "$(OS)" "Darwin"
-	CLANG_OR_GCC := $(strip $(if $(findstring clang,$(CC)), \
-				$(findstring clang,$(CC)), \
-				$(if $(findstring gcc,$(CC)), \
-					$(findstring gcc,$(CC)), \
-					cc)))
-
-	CC_LASTWORD := $(strip $(lastword $(subst -, ,$(CC))))
-
-	replace_with = $(strip $(if $(findstring $(3),$(CC_LASTWORD)), \
-			$(subst $(3),$(1),$(2)), \
-			$(subst $(3),$(1),$(subst -$(CC_LASTWORD),,$(2)))))
-
-	ifeq "$(notdir $(CC))" "$(CC)"
-		replace_cc_with = $(call replace_with,$(1),$(CC),$(CLANG_OR_GCC))
-	else
-		replace_cc_with = $(join $(dir $(CC)),$(call replace_with,$(1),$(notdir $(CC)),$(CLANG_OR_GCC)))
-	endif
-
-	OBJCOPY ?= $(call replace_cc_with,objcopy)
-	ARCHIVER ?= $(call replace_cc_with,ar)
-	override AR = $(ARCHIVER)
-endif
-
 ifdef PIE
 	LDFLAGS += -pie
 endif
@@ -468,8 +530,8 @@ DYLIB_OBJECTS +=$(strip $(DYLIB_C_SOURCES:.c=.o))
 DYLIB_OBJECTS +=$(strip $(DYLIB_OBJC_SOURCES:.m=.o))
 ifneq "$(strip $(DYLIB_CXX_SOURCES))" ""
 	DYLIB_OBJECTS +=$(strip $(patsubst %.mm, %.o, $(DYLIB_CXX_SOURCES:.cpp=.o)))
-	CXX = $(call cxx_compiler,$(CC))
-	LD = $(call cxx_linker,$(CC))
+	CXX = $(call cxx_compiler)
+	gD = $(call cxx_linker)
 endif
 
 #----------------------------------------------------------------------
@@ -492,8 +554,8 @@ endif
 #----------------------------------------------------------------------
 ifneq "$(strip $(CXX_SOURCES))" ""
 	OBJECTS +=$(strip $(CXX_SOURCES:.cpp=.o))
-	CXX = $(call cxx_compiler,$(CC))
-	LD = $(call cxx_linker,$(CC))
+	CXX = $(call cxx_compiler)
+	LD = $(call cxx_linker)
 endif
 
 #----------------------------------------------------------------------
@@ -509,19 +571,19 @@ endif
 #----------------------------------------------------------------------
 ifneq "$(strip $(OBJCXX_SOURCES))" ""
 	OBJECTS +=$(strip $(OBJCXX_SOURCES:.mm=.o))
-	CXX = $(call cxx_compiler,$(CC))
-	LD = $(call cxx_linker,$(CC))
+	CXX = $(call cxx_compiler)
+	LD = $(call cxx_linker)
 	ifeq "$(findstring lobjc,$(LDFLAGS))" ""
 		LDFLAGS +=-lobjc
 	endif
 endif
 
-ifeq ($(findstring clang, $(CXX)), clang)
+ifeq ($(CCC), clang)
 	CXXFLAGS += --driver-mode=g++
 endif
 
 ifneq "$(CXX)" ""
-	ifeq ($(findstring clang, $(LD)), clang)
+	ifeq ($(LDC), clang)
 		LDFLAGS += --driver-mode=g++
 	endif
 endif

labath · 2024-08-07T06:45:33Z

Umm... could we move this logic to python, and then just pass the final tool paths (or whatever) into the makefile, like we did with the other patches (for (HOST_)OS, etc.) ?

labath · 2024-08-07T06:47:04Z

(that said, I would be very surprised if anyone was running this test suite with icc, so I think ripping out icc support is also an option)

dzhidzhoev · 2024-08-15T19:41:47Z

Umm... could we move this logic to python, and then just pass the final tool paths (or whatever) into the makefile, like we did with the other patches (for (HOST_)OS, etc.) ?

Moved logic to Python, updated commit description.

dzhidzhoev · 2024-08-16T21:04:28Z

Updated added dwp tool handling, to adopt changes from e77ac42 and resolve merge conflict.

dzhidzhoev · 2024-08-20T16:57:15Z

Now this is ready for review.

lldb/packages/Python/lldbsuite/test/builders/builder.py

labath · 2024-08-26T15:09:45Z

lldb/packages/Python/lldbsuite/test/builders/builder.py

+
+            cc = cc.strip()
+            cc_path = pathlib.Path(cc)
+            cc = cc_path.as_posix()


What's up with the posix thing? Could we stick to native paths?

The idea in Makefile.rules was to unify path separators to process them. Indeed, we don't need that here.

labath · 2024-08-26T15:10:15Z

lldb/packages/Python/lldbsuite/test/builders/builder.py

+
+            def getLlvmUtil(util_name):
+                llvm_tools_dir = os.getenv("LLVM_TOOLS_DIR", cc_dir.as_posix())
+                return llvm_tools_dir + "/" + util_name + exe_ext


Suggested change

return llvm_tools_dir + "/" + util_name + exe_ext

return os.path.join(llvm_tools_dir, util_name + exe_ext)

labath · 2024-08-26T15:13:59Z

lldb/packages/Python/lldbsuite/test/make/Makefile.rules

+# Remove all " and ' characters from the path. Also remove surrounding space chars.
+cstrip = $(strip $(subst ",,$(subst ',,$(1))))
+override CC := $(call cstrip,$(CC))
+override CCC := $(call cstrip,$(CCC))
+override CXX := $(call cstrip,$(CXX))
+# Always override the linker. Assign already normalized CC.
+override LD := $(call cstrip,$(CC))
+# A kind of linker. It always gets retrieved from CC.
+override LDC := $(call cstrip,$(CCC))
+override OBJCOPY := $(call cstrip,$(OBJCOPY))
+override STRIP := $(call cstrip,$(STRIP))
+override ARCHIVER := $(call cstrip,$(ARCHIVER))
+override AR := $(call cstrip,$(AR))
+override DWP := $(call cstrip,$(DWP))
+override LLVM_AR := $(call cstrip,$(LLVM_AR))


Is this really necessary? Could we get rid of it by not passing the extra quotes in the python cmd?

Yes, you're right, fixed it.

labath · 2024-08-26T15:14:29Z

lldb/packages/Python/lldbsuite/test/builders/builder.py

+                'CC="%s"' % cc,
+                'CCC="%s"' % cc_type,
+                'CXX="%s"' % cxx,


I.e., by replacing this (and others) with

Suggested change

'CC="%s"' % cc,

'CCC="%s"' % cc_type,

'CXX="%s"' % cxx,

'CC=%s' % cc,

'CCC=%s' % cc_type,

'CXX=%s' % cxx,

labath · 2024-08-26T15:16:03Z

lldb/packages/Python/lldbsuite/test/builders/builder.py

+                utils.extend(['LLVM_AR="%s"' % llvm_ar])
+
+            if not lldbplatformutil.platformIsDarwin():
+                if os.getenv("USE_LLVM_TOOLS"):


I don't see any reference to this var in the llvm tree. Who sets this? (and why)

I'm still puzzled by this..

I got rid of environment variable usage and used dotest.py param instead.

The flag --use-llvm-tools flag is added to force usage of LLVM tools passed with --llvm-tools-dir instead of compiler-provided tools.
We use it while running API tests on Windows x86-host/Ubuntu AArch64-target combination with cl.exe. This flag enables switching to the "local" tools usage without relying on the PATH/compiler path for utilities lookup, so as not to rely on an external toolchain while having all necessary tools built together with lldb.

Ok, I see now. This is new functionality added in this patch. Could we put that in a separate PR and keep this one as a NFC-ish refactoring/cleanup patch? I'd like to discuss this concept further, but I think it would be easier to do it in isolation.

Sorry for the delay, I had to take a leave.
Removed that from this PR.

Thanks. Before you go on the next PR, let me just elaborate on what I'm thinking.

Basically, the (API) test suite can run in two modes. The first one (the default) is where it uses all of the tools from the build tree that it can. This includes the compiler and the other tools like llvm-objcopy, etc. This is done for two reasons:

it minimizes the amount of external dependencies

it maximizes the reproducibility of the build

This mode doesn't (shouldn't?) require any special flags or configuration. The second mode is where you can run the tests against a toolchain (mostly: compiler) of your choosing, and there it's expected that you may need to pass many arguments in order to make it work.

The thing which bothers me about this flag is that it's a flag that forces a switch back to the default mode (for some tools), which is makes reasoning about it more complicated. I think it would be better to just make the behavior you want to be the default, and then (if necessary) have a separate flag to force the usage of a specific tool (e.g. objcopy). I think that would be similar to how things work with e.g. dsymutil, where by default we pick the in-tree version, but we have a cmake/dotest/etc. flag to let the user choose a different one.

This would mean that users that run tests against gcc (and which want to continue using the gcc's version of objcopy) could no longer rely on the compiler-based detection and would have to pass (and add it to dotest first) an additional flag explicitly, but I think that's fine, because:

These tests mostly don't care about the objcopy version. Their main purpose is to capture the differences in DWARF emitted by different compilers. We have better ways of testing the differences in object file (elf) output.

I'm not sure if we have any such users at the moment.

The thing which bothers me about this flag is that it's a flag that forces a switch back to the default mode (for some tools), which is makes reasoning about it more complicated. I think it would be better to just make the behavior you want to be the default, and then (if necessary) have a separate flag to force the usage of a specific tool (e.g. objcopy). I think that would be similar to how things work with e.g. dsymutil, where by default we pick the in-tree version, but we have a cmake/dotest/etc. flag to let the user choose a different one.

It looks good for me. I'll do it this way.

labath · 2024-08-26T15:16:58Z

lldb/packages/Python/lldbsuite/test/make/Makefile.rules

@@ -213,7 +229,7 @@ endif
 LIMIT_DEBUG_INFO_FLAGS =
 NO_LIMIT_DEBUG_INFO_FLAGS =
 MODULE_DEBUG_INFO_FLAGS =
-ifneq (,$(findstring clang,$(CC)))
+ifeq ($(CCC), clang)


What does CCC stand for?

Compiler type

So the last C stands for "type" ? :)

Can we call it CC_TYPE/KIND (or similar) instead? It's not like we're using it that often...

github-actions · 2024-08-28T18:53:29Z

✅ With the latest revision this PR passed the Python code formatter.

labath

Looks pretty good now, but I still have some questions.

labath · 2024-08-29T15:35:41Z

lldb/packages/Python/lldbsuite/test/make/Makefile.rules

@@ -213,7 +229,7 @@ endif
 LIMIT_DEBUG_INFO_FLAGS =
 NO_LIMIT_DEBUG_INFO_FLAGS =
 MODULE_DEBUG_INFO_FLAGS =
-ifneq (,$(findstring clang,$(CC)))
+ifeq ($(CCC), clang)


So the last C stands for "type" ? :)

Can we call it CC_TYPE/KIND (or similar) instead? It's not like we're using it that often...

labath · 2024-08-29T15:37:29Z

lldb/packages/Python/lldbsuite/test/builders/builder.py

        used for the make system.
        """
        cc = compiler if compiler else None
        if not cc and configuration.compiler:
            cc = configuration.compiler
+
        if cc:


Suggested change

if cc:

if not cc:

return []

.. and then unindent the rest of the code

labath · 2024-08-29T15:40:46Z

lldb/packages/Python/lldbsuite/test/builders/builder.py

+
+            util_names = {
+                "OBJCOPY": "objcopy",
+                "STRIP": "objcopy",


Can we get rid of this variable? I don't think it's very helpful. Even though objcopy supports (I think) all of the functionality of strip, their args are different, so I think this could be pretty confusing.

Sorry, it was just a copy-paste typo.

labath · 2024-08-29T15:41:19Z

lldb/packages/Python/lldbsuite/test/builders/builder.py

+            utils = []
+
+            # Note: LLVM_AR is currently required by API TestBSDArchives.py tests.
+            # Assembly a full path to llvm-ar for given LLVM_TOOLS_DIR;


Suggested change

# Assembly a full path to llvm-ar for given LLVM_TOOLS_DIR;

# Assemble a full path to llvm-ar for given LLVM_TOOLS_DIR;

labath · 2024-08-29T15:42:02Z

lldb/packages/Python/lldbsuite/test/builders/builder.py

+                utils.extend(['LLVM_AR="%s"' % llvm_ar])
+
+            if not lldbplatformutil.platformIsDarwin():
+                if os.getenv("USE_LLVM_TOOLS"):


I'm still puzzled by this..

This fix is based on a problem with cxx_compiler and cxx_linker macros on Windows. There was an issue with compiler detection in paths containing "icc". In such case, Makefile.rules thought it was provided with icc compiler. To solve that, utilities detection has been rewritten in Python. The last element of compiler's path is separated, taking into account platform path delimiter, and compiler type is extracted, with regard of possible cross-toolchain prefix. Paths for additional tools, like OBJCOPY, are initialized with tools built with LLVM if USE_LLVM_TOOLS is on, to achieve better portability for Windows. Quotes from paths are removed in Makefile.rules, that may be added when arguments are passed from Python to make.

Co-authored-by: Pavel Labath <[email protected]>

This reverts commit 55b981cb0fcb4c2d3b62d8412b507afedd8801da.

labath

Looks good now, just remove the dead code.

labath · 2024-09-10T11:04:40Z

lldb/packages/Python/lldbsuite/test/builders/builder.py

+        exe_ext = ""
+        if lldbplatformutil.getHostPlatform() == "windows":
+            exe_ext = ".exe"
+


It looks like this isn't used now.

In llvm#102185, toolchain detection for API tests has been rewritten to Python. However, tools paths for tests are determined from compiler path. Here tools are taken from `--llvm-tools-dir` dotest.py argument, which by default refers to the LLVM build directory, unless they are explicitly redefined in environment variables. It helps to minimize external dependencies and to maximize the reproducibility of the build.

In #102185, toolchain detection for API tests has been rewritten in Python. Tools paths for tests there are determined from compiler path. Here tools are taken from `--llvm-tools-dir` dotest.py argument, which by default refers to the LLVM build directory, unless they are explicitly redefined in environment variables. It helps to minimize external dependencies and to maximize the reproducibility of the build.

dzhidzhoev added the lldb label Aug 6, 2024

dzhidzhoev requested review from splhack, DavidSpickett, labath, clayborg, jasonmolenda and omjavaid August 6, 2024 17:11

dzhidzhoev self-assigned this Aug 6, 2024

dzhidzhoev requested a review from JDevlieghere as a code owner August 6, 2024 17:11

jasonmolenda requested a review from adrian-prantl August 6, 2024 20:34

dzhidzhoev force-pushed the lldb/cc-detection branch 2 times, most recently from b21f63e to 5130b2a Compare August 15, 2024 19:39

dzhidzhoev force-pushed the lldb/cc-detection branch from 5130b2a to e40ca68 Compare August 16, 2024 21:02

labath reviewed Aug 26, 2024

View reviewed changes

labath reviewed Aug 29, 2024

View reviewed changes

dzhidzhoev and others added 8 commits September 3, 2024 23:08

Fix comment

ef9aef9

Typo fix

7a8a1c7

Co-authored-by: Pavel Labath <[email protected]>

Don't quote paths

52b420b

Join paths with os.path.join

94c71b7

Use native paths

e41da92

Use cmd arguments instead of env variables in builder

9ea46fe

Rename CCC to CC_TYPE

5b2661d

dzhidzhoev added 6 commits September 3, 2024 23:08

Fix typo

09a197b

Fix wrong utility for STRIP

7152e2e

Reformat condition

0cd8970

Formatting fixed

0086b9b

Revert "Use cmd arguments instead of env variables in builder"

646e41f

This reverts commit 55b981cb0fcb4c2d3b62d8412b507afedd8801da.

Remove USE_LLVM_TOOLS usage

ae96df6

dzhidzhoev force-pushed the lldb/cc-detection branch from 3d422ec to ae96df6 Compare September 9, 2024 15:52

dzhidzhoev changed the title ~~[lldb][test] Improve toolchain detection in Makefile.rules~~ [lldb][test] Rewrite toolchain detection in Python Sep 9, 2024

dzhidzhoev changed the title ~~[lldb][test] Rewrite toolchain detection in Python~~ [lldb][test] Toolchain detection rewrite in Python Sep 9, 2024

labath approved these changes Sep 10, 2024

View reviewed changes

Removed unused exe_ext variable

f1e4f5f

dzhidzhoev merged commit 44fc987 into llvm:main Sep 11, 2024
5 of 6 checks passed

dzhidzhoev mentioned this pull request Sep 25, 2024

[lldb][test] Use tools from llvm instead of compiler tools #109961

Merged

	return llvm_tools_dir + "/" + util_name + exe_ext
	return os.path.join(llvm_tools_dir, util_name + exe_ext)

	# Assembly a full path to llvm-ar for given LLVM_TOOLS_DIR;
	# Assemble a full path to llvm-ar for given LLVM_TOOLS_DIR;

[lldb][test] Toolchain detection rewrite in Python #102185

[lldb][test] Toolchain detection rewrite in Python #102185

Uh oh!

Conversation

dzhidzhoev commented Aug 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Aug 6, 2024

Uh oh!

labath commented Aug 7, 2024

Uh oh!

labath commented Aug 7, 2024

Uh oh!

dzhidzhoev commented Aug 15, 2024

Uh oh!

dzhidzhoev commented Aug 16, 2024

Uh oh!

dzhidzhoev commented Aug 20, 2024

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Aug 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

labath left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

labath left a comment

Choose a reason for hiding this comment

dzhidzhoev commented Aug 6, 2024 •

edited

Loading

github-actions bot commented Aug 28, 2024 •

edited

Loading