-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[LLDB][test] Update Makefile.rules to support Windows host+Linux target #99266
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-lldb Author: Vladislav Dzhidzhoev (dzhidzhoev) ChangesThese changes aim to support cross-compilation build on Windows host for Linux target for API tests execution. They're not final: changes will follow for refactoring and adjustments to make all tests pass. Chocolatey make is recommended since it is maintained better than GnuWin32 mentioned here https://lldb.llvm.org/resources/build.html#codesigning (latest GnuWin32 release dated by 2010) and helps to avoid problems with building tests (for example, GnuWin32 doesn't support long paths and there are some other failures with building for Linux with it). This commit contains the following changes:
Full diff: https://github.com/llvm/llvm-project/pull/99266.diff 18 Files Affected:
diff --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
index d1a2de8b2478a..1474714ac4f5c 100644
--- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -47,7 +47,8 @@ LLDB_BASE_DIR := $(THIS_FILE_DIR)/../../../../../
.DEFAULT_GOAL := all
#----------------------------------------------------------------------
-# If OS is not defined, use 'uname -s' to determine the OS name.
+# If OS or/and HOST_OS are not defined, use 'uname -s' to determine
+# the OS name.
#
# GNUWin32 uname gives "windows32" or "server version windows32" while
# some versions of MSYS uname return "MSYS_NT*", but most environments
@@ -56,15 +57,12 @@ LLDB_BASE_DIR := $(THIS_FILE_DIR)/../../../../../
# inherited all the way down to the process spawned for make.
#----------------------------------------------------------------------
ifeq "$(HOST_OS)" ""
- HOST_OS := $(shell uname -s)
-endif
-
-ifneq (,$(findstring windows32,$(HOST_OS)))
- HOST_OS := Windows_NT
-endif
-
-ifneq (,$(findstring MSYS_NT,$(HOST_OS)))
- HOST_OS := Windows_NT
+ HOST_OS := $(shell uname -s)
+ ifneq (,$(or \
+ $(findstring windows32,$(HOST_OS)),\
+ $(findstring MSYS_NT,$(HOST_OS))))
+ HOST_OS := Windows_NT
+ endif
endif
ifeq "$(OS)" ""
@@ -80,9 +78,21 @@ endif
# Also reset BUILDDIR value because "pwd" returns cygwin or msys path
# which needs to be converted to windows path.
#----------------------------------------------------------------------
-ifeq "$(OS)" "Windows_NT"
- SHELL = $(WINDIR)\system32\cmd.exe
+path_wrapper = $(1)
+ifeq "$(HOST_OS)" "Windows_NT"
+ # Windows 10 and later has the lower-case 'windir' env variable.
+ SHELL := $(or $(windir),$(WINDIR),C:\WINDOWS)\system32\cmd.exe
BUILDDIR := $(shell echo %cd%)
+
+ ifneq (,$(filter $(OS), Linux Android))
+ path_wrapper = $(subst \,/,$(1))
+ # Normalize base paths at the same time.
+ override SRCDIR := $(call path_wrapper,$(SRCDIR))
+ override BUILDDIR := $(call path_wrapper,$(BUILDDIR))
+ override MAKEFILE_RULES := $(call path_wrapper,$(MAKEFILE_RULES))
+ override THIS_FILE_DIR := $(call path_wrapper,$(THIS_FILE_DIR))
+ override LLDB_BASE_DIR := $(call path_wrapper,$(LLDB_BASE_DIR))
+ endif
endif
#----------------------------------------------------------------------
@@ -135,7 +145,7 @@ $(error "C compiler is not specified. Please run tests through lldb-dotest or li
endif
#----------------------------------------------------------------------
-# Handle SDKROOT on Darwin
+# Handle SDKROOT for the cross platform builds.
#----------------------------------------------------------------------
ifeq "$(OS)" "Darwin"
@@ -143,6 +153,18 @@ ifeq "$(OS)" "Darwin"
# We haven't otherwise set the SDKROOT, so set it now to macosx
SDKROOT := $(shell xcrun --sdk macosx --show-sdk-path)
endif
+ SYSROOT_FLAGS := -isysroot "$(SDKROOT)"
+ GCC_TOOLCHAIN_FLAGS :=
+else
+ ifneq "$(SDKROOT)" ""
+ SYSROOT_FLAGS := --sysroot "$(call path_wrapper,$(SDKROOT))"
+ GCC_TOOLCHAIN_FLAGS := --gcc-toolchain="$(call path_wrapper,$(SDKROOT)/usr)"
+ else
+ # Do not set up these options if SDKROOT was not specified.
+ # This is a regular build in that case (or Android).
+ SYSROOT_FLAGS :=
+ GCC_TOOLCHAIN_FLAGS :=
+ endif
endif
#----------------------------------------------------------------------
@@ -239,20 +261,15 @@ endif
DEBUG_INFO_FLAG ?= -g
CFLAGS ?= $(DEBUG_INFO_FLAG) -O0
-
-ifeq "$(OS)" "Darwin"
- ifneq "$(SDKROOT)" ""
- CFLAGS += -isysroot "$(SDKROOT)"
- endif
-endif
+CFLAGS += $(SYSROOT_FLAGS)
ifeq "$(OS)" "Darwin"
CFLAGS += $(ARCHFLAG) $(ARCH) $(FRAMEWORK_INCLUDES)
else
CFLAGS += $(ARCHFLAG)$(ARCH)
endif
-CFLAGS += -I$(LLDB_BASE_DIR)include -I$(LLDB_OBJ_ROOT)/include
+CFLAGS += -I$(LLDB_BASE_DIR)/include -I$(LLDB_OBJ_ROOT)/include
CFLAGS += -I$(SRCDIR) -I$(THIS_FILE_DIR)
ifndef NO_TEST_COMMON_H
@@ -263,9 +280,9 @@ CFLAGS += $(NO_LIMIT_DEBUG_INFO_FLAGS) $(ARCH_CFLAGS)
# Use this one if you want to build one part of the result without debug information:
ifeq "$(OS)" "Darwin"
- CFLAGS_NO_DEBUG = -O0 $(ARCHFLAG) $(ARCH) $(FRAMEWORK_INCLUDES) $(ARCH_CFLAGS) $(CFLAGS_EXTRAS) -isysroot "$(SDKROOT)"
+ CFLAGS_NO_DEBUG = -O0 $(ARCHFLAG) $(ARCH) $(FRAMEWORK_INCLUDES) $(ARCH_CFLAGS) $(CFLAGS_EXTRAS) $(SYSROOT_FLAGS)
else
- CFLAGS_NO_DEBUG = -O0 $(ARCHFLAG)$(ARCH) $(FRAMEWORK_INCLUDES) $(ARCH_CFLAGS) $(CFLAGS_EXTRAS)
+ CFLAGS_NO_DEBUG = -O0 $(ARCHFLAG)$(ARCH) $(FRAMEWORK_INCLUDES) $(ARCH_CFLAGS) $(CFLAGS_EXTRAS) $(SYSROOT_FLAGS)
endif
ifeq "$(MAKE_DWO)" "YES"
@@ -275,7 +292,7 @@ endif
ifeq "$(USE_PRIVATE_MODULE_CACHE)" "YES"
THE_CLANG_MODULE_CACHE_DIR := $(BUILDDIR)/private-module-cache
else
-THE_CLANG_MODULE_CACHE_DIR := $(CLANG_MODULE_CACHE_DIR)
+THE_CLANG_MODULE_CACHE_DIR := $(call path_wrapper,$(CLANG_MODULE_CACHE_DIR))
endif
MODULE_BASE_FLAGS := -fmodules -gmodules -fmodules-cache-path=$(THE_CLANG_MODULE_CACHE_DIR)
@@ -296,11 +313,13 @@ endif
CFLAGS += $(CFLAGS_EXTRAS)
CXXFLAGS += -std=c++11 $(CFLAGS) $(ARCH_CXXFLAGS)
LD = $(CC)
-LDFLAGS ?= $(CFLAGS)
+# 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)
LDFLAGS += $(LD_EXTRAS) $(ARCH_LDFLAGS)
ifeq (,$(filter $(OS), Windows_NT Android Darwin))
ifneq (,$(filter YES,$(ENABLE_THREADS)))
- LDFLAGS += -pthread
+ LDFLAGS += -lpthread
endif
endif
OBJECTS =
@@ -418,11 +437,28 @@ ifeq (1, $(USE_SYSTEM_STDLIB))
endif
endif
+# No C++ library has been specifieed. Use libstdc++ by default.
+ifeq (,$(filter 1, $(USE_LIBSTDCPP) $(USE_LIBCPP) $(USE_SYSTEM_STDLIB)))
+ # If no explicit request was made, but we have paths to a custom libcxx, use
+ # them.
+ ifneq ($(and $(LIBCPP_INCLUDE_DIR), $(LIBCPP_LIBRARY_DIR)),)
+ CXXFLAGS += -nostdlib++ -nostdinc++ -cxx-isystem $(LIBCPP_INCLUDE_DIR)
+ ifneq "$(LIBCPP_INCLUDE_TARGET_DIR)" ""
+ CXXFLAGS += -cxx-isystem $(LIBCPP_INCLUDE_TARGET_DIR)
+ endif
+ LDFLAGS += -L$(LIBCPP_LIBRARY_DIR) -Wl,-rpath,$(LIBCPP_LIBRARY_DIR) -lc++ -lc++abi
+ # Otherwise no C++ library has been specified. Use stdc++ by default.
+ else
+ USE_LIBSTDCPP := 1
+ endif
+endif
+
ifeq (1,$(USE_LIBSTDCPP))
# Clang requires an extra flag: -stdlib=libstdc++
ifneq (,$(findstring clang,$(CC)))
- CXXFLAGS += -stdlib=libstdc++
- LDFLAGS += -stdlib=libstdc++
+ # Force clang looking for the gcc's headers at specific rootfs folder.
+ CXXFLAGS += -stdlib=libstdc++ $(GCC_TOOLCHAIN_FLAGS)
+ LDFLAGS += -stdlib=libstdc++ $(GCC_TOOLCHAIN_FLAGS)
endif
endif
@@ -432,18 +468,18 @@ ifeq (1,$(USE_LIBCPP))
ifneq "$(LIBCPP_INCLUDE_TARGET_DIR)" ""
CXXFLAGS += -cxx-isystem $(LIBCPP_INCLUDE_TARGET_DIR)
endif
- LDFLAGS += -L$(LIBCPP_LIBRARY_DIR) -Wl,-rpath,$(LIBCPP_LIBRARY_DIR) -lc++
+ LDFLAGS += -L$(LIBCPP_LIBRARY_DIR) -Wl,-rpath,$(LIBCPP_LIBRARY_DIR) -lc++ -lc++abi
else
ifeq "$(OS)" "Android"
# Nothing to do, this is already handled in
# Android.rules.
else
CXXFLAGS += -stdlib=libc++
- LDFLAGS += -stdlib=libc++
+ LDFLAGS += -stdlib=libc++ -lc++abi
endif
ifneq (,$(filter $(OS), FreeBSD Linux NetBSD))
ifneq (,$(LLVM_LIBS_DIR))
- LDFLAGS += -Wl,-rpath,$(LLVM_LIBS_DIR)
+ LDFLAGS += -Wl,-rpath,$(LLVM_LIBS_DIR)
endif
endif
endif
@@ -456,21 +492,15 @@ ifeq (1, $(USE_SYSTEM_STDLIB))
endif
CXXFLAGS += -nostdlib++ -nostdinc++ -cxx-isystem $(SDKROOT)/usr/include/c++/v1
LDFLAGS += -L$(SDKROOT)/usr/lib -Wl,-rpath,$(SDKROOT)/usr/lib -lc++
+ else
+ ifneq (,$(findstring clang,$(CC)))
+ # Force clang looking for the gcc's headers at specific rootfs folder.
+ CXXFLAGS += -stdlib=libstdc++ $(GCC_TOOLCHAIN_FLAGS)
+ LDFLAGS += -stdlib=libstdc++ $(GCC_TOOLCHAIN_FLAGS)
+ endif
endif
endif
-# If no explicit request was made, but we have paths to a custom libcxx, use
-# them.
-ifeq ($(or $(USE_LIBSTDCPP), $(USE_LIBCPP), $(USE_SYSTEM_STDLIB)),)
- ifneq ($(and $(LIBCPP_INCLUDE_DIR), $(LIBCPP_LIBRARY_DIR)),)
- CXXFLAGS += -nostdlib++ -nostdinc++ -cxx-isystem $(LIBCPP_INCLUDE_DIR)
- ifneq "$(LIBCPP_INCLUDE_TARGET_DIR)" ""
- CXXFLAGS += -cxx-isystem $(LIBCPP_INCLUDE_TARGET_DIR)
- endif
- LDFLAGS += -L$(LIBCPP_LIBRARY_DIR) -Wl,-rpath,$(LIBCPP_LIBRARY_DIR) -lc++
- endif
-endif
-
#----------------------------------------------------------------------
# Additional system libraries
#----------------------------------------------------------------------
@@ -659,20 +689,20 @@ endif
ifneq "$(PCH_OUTPUT)" ""
$(PCH_OUTPUT) : $(PCH_CXX_SOURCE)
- $(CXX) $(CXXFLAGS) -x c++-header -o $@ $<
+ $(CXX) $(CXXFLAGS) -x c++-header -o $@ $(call path_wrapper,$<)
endif
%.o: %.c %.d
- $(CC) $(CFLAGS) -MT $@ -MD -MP -MF $*.d -c -o $@ $<
+ $(CC) $(CFLAGS) -MT $@ -MD -MP -MF $*.d -c -o $@ $(call path_wrapper,$<)
%.o: %.cpp %.d $(PCH_OUTPUT)
- $(CXX) $(PCHFLAGS) $(CXXFLAGS) -MT $@ -MD -MP -MF $*.d -c -o $@ $<
+ $(CXX) $(PCHFLAGS) $(CXXFLAGS) -MT $@ -MD -MP -MF $*.d -c -o $@ $(call path_wrapper,$<)
%.o: %.m %.d
- $(CC) $(CFLAGS) -MT $@ -MD -MP -MF $*.d -c -o $@ $<
+ $(CC) $(CFLAGS) -MT $@ -MD -MP -MF $*.d -c -o $@ $(call path_wrapper,$<)
%.o: %.mm %.d
- $(CXX) $(CXXFLAGS) -MT $@ -MD -MP -MF $*.d -c -o $@ $<
+ $(CXX) $(CXXFLAGS) -MT $@ -MD -MP -MF $*.d -c -o $@ $(call path_wrapper,$<)
#----------------------------------------------------------------------
# Automatic variables based on items already entered. Below we create
diff --git a/lldb/test/API/commands/settings/use_source_cache/Makefile b/lldb/test/API/commands/settings/use_source_cache/Makefile
index 791cb7d868d87..2fc18665303cb 100644
--- a/lldb/test/API/commands/settings/use_source_cache/Makefile
+++ b/lldb/test/API/commands/settings/use_source_cache/Makefile
@@ -5,4 +5,4 @@ include Makefile.rules
# Copy file into the build folder to enable the test to modify it.
main-copy.cpp: main.cpp
- cp -f $< $@
+ cp -f $(call path_wrapper,$<) $@
diff --git a/lldb/test/API/functionalities/breakpoint/comp_dir_symlink/Makefile b/lldb/test/API/functionalities/breakpoint/comp_dir_symlink/Makefile
index 1c42301c7b587..ff090a7fd0ea2 100644
--- a/lldb/test/API/functionalities/breakpoint/comp_dir_symlink/Makefile
+++ b/lldb/test/API/functionalities/breakpoint/comp_dir_symlink/Makefile
@@ -6,4 +6,4 @@ include Makefile.rules
# Force relative filenames by copying it into the build directory.
relative.cpp: main.cpp
- cp -f $< $@
+ cp -f $(call path_wrapper,$<) $@
diff --git a/lldb/test/API/functionalities/inline-sourcefile/Makefile b/lldb/test/API/functionalities/inline-sourcefile/Makefile
index adb29d3a88e26..76a41fa4ec4b4 100644
--- a/lldb/test/API/functionalities/inline-sourcefile/Makefile
+++ b/lldb/test/API/functionalities/inline-sourcefile/Makefile
@@ -8,4 +8,4 @@ OBJECTS += inline.o
$(EXE): main.c inline.o
%.o: %.ll
- $(CC) $< -c -o $@
+ $(CC) $(call path_wrapper,$<) -c -o $@
diff --git a/lldb/test/API/functionalities/multiple-slides/Makefile b/lldb/test/API/functionalities/multiple-slides/Makefile
index 5f83deaa24d77..822af701e7794 100644
--- a/lldb/test/API/functionalities/multiple-slides/Makefile
+++ b/lldb/test/API/functionalities/multiple-slides/Makefile
@@ -8,5 +8,5 @@ include Makefile.rules
# sliding the binary, the address of `first` and
# `second` are not slid for some reason on Darwin.
main.o: main.c
- $(CC) $(CFLAGS_NO_DEBUG) -c $< -o $@
+ $(CC) $(CFLAGS_NO_DEBUG) -c $(call path_wrapper,$<) -o $@
diff --git a/lldb/test/API/functionalities/postmortem/netbsd-core/GNUmakefile b/lldb/test/API/functionalities/postmortem/netbsd-core/GNUmakefile
index 62c719d3d2ff0..f72ddf4234828 100644
--- a/lldb/test/API/functionalities/postmortem/netbsd-core/GNUmakefile
+++ b/lldb/test/API/functionalities/postmortem/netbsd-core/GNUmakefile
@@ -8,8 +8,8 @@ clean:
rm -f $(CORES) $(EXECS)
%.core: %
- sysctl -w proc.$$$$.corename=$@; ulimit -s 16; ! ./$<
+ sysctl -w proc.$$$$.corename=$@; ulimit -s 16; ! ./$(call path_wrapper,$<)
%.$(ARCH): %.c
- $(CC) -o $@ -g $<
+ $(CC) -o $@ -g $(call path_wrapper,$<)
.PHONY: all clean
diff --git a/lldb/test/API/functionalities/step-avoids-no-debug/Makefile b/lldb/test/API/functionalities/step-avoids-no-debug/Makefile
index 374e58b89a81c..1ea6acec8960c 100644
--- a/lldb/test/API/functionalities/step-avoids-no-debug/Makefile
+++ b/lldb/test/API/functionalities/step-avoids-no-debug/Makefile
@@ -3,4 +3,4 @@ C_SOURCES := with-debug.c without-debug.c
include Makefile.rules
without-debug.o: without-debug.c
- $(CC) $(CFLAGS_NO_DEBUG) -c $<
+ $(CC) $(CFLAGS_NO_DEBUG) -c $(call path_wrapper,$<)
diff --git a/lldb/test/API/functionalities/valobj_errors/Makefile b/lldb/test/API/functionalities/valobj_errors/Makefile
index d2c966a71411b..caf380fb87460 100644
--- a/lldb/test/API/functionalities/valobj_errors/Makefile
+++ b/lldb/test/API/functionalities/valobj_errors/Makefile
@@ -4,6 +4,6 @@ LD_EXTRAS = hidden.o
a.out: hidden.o
hidden.o: hidden.c
- $(CC) -g0 -c -o $@ $<
+ $(CC) -g0 -c -o $@ $(call path_wrapper,$<)
include Makefile.rules
diff --git a/lldb/test/API/lang/cpp/operator-overload/Makefile b/lldb/test/API/lang/cpp/operator-overload/Makefile
index 80b3ee02c3e93..7c4e5f0e2bd8b 100644
--- a/lldb/test/API/lang/cpp/operator-overload/Makefile
+++ b/lldb/test/API/lang/cpp/operator-overload/Makefile
@@ -3,4 +3,4 @@ CXX_SOURCES = a.cpp b.cpp
include Makefile.rules
a.o: a.cpp
- $(CC) $(CFLAGS_NO_DEBUG) -c $< -o $@
+ $(CC) $(CFLAGS_NO_DEBUG) -c $(call path_wrapper,$<) -o $@
diff --git a/lldb/test/API/lang/objcxx/class-name-clash/Makefile b/lldb/test/API/lang/objcxx/class-name-clash/Makefile
index b2c98c223e724..df2c787e9a622 100644
--- a/lldb/test/API/lang/objcxx/class-name-clash/Makefile
+++ b/lldb/test/API/lang/objcxx/class-name-clash/Makefile
@@ -3,4 +3,4 @@ include Makefile.rules
# myobject.o needs to be built without debug info
myobject.o: myobject.mm
- $(CXX) $(CFLAGS_NO_DEBUG) -c -o $@ $<
+ $(CXX) $(CFLAGS_NO_DEBUG) -c -o $@ $(call path_wrapper,$<)
diff --git a/lldb/test/API/linux/add-symbols/Makefile b/lldb/test/API/linux/add-symbols/Makefile
index b10a03899e225..8d65741c66c28 100644
--- a/lldb/test/API/linux/add-symbols/Makefile
+++ b/lldb/test/API/linux/add-symbols/Makefile
@@ -4,6 +4,6 @@ LD_EXTRAS := -Wl,--build-id=none
all: stripped.out
stripped.out : a.out
- $(OBJCOPY) --remove-section=.note.gnu.build-id --remove-section=.gnu_debuglink --strip-debug $< $@
+ $(OBJCOPY) --remove-section=.note.gnu.build-id --remove-section=.gnu_debuglink --strip-debug $(call path_wrapper,$<) $@
include Makefile.rules
diff --git a/lldb/test/API/linux/sepdebugsymlink/Makefile b/lldb/test/API/linux/sepdebugsymlink/Makefile
index a1b84c1843e96..6673918b72fdf 100644
--- a/lldb/test/API/linux/sepdebugsymlink/Makefile
+++ b/lldb/test/API/linux/sepdebugsymlink/Makefile
@@ -5,7 +5,7 @@ all: dirsymlink
dirreal: a.out
$(RM) -r $@
mkdir $@
- $(OBJCOPY) --only-keep-debug $< $@/stripped.debug
+ $(OBJCOPY) --only-keep-debug $(call path_wrapper,$<) $@/stripped.debug
$(OBJCOPY) --strip-all --add-gnu-debuglink=$@/stripped.debug $< $@/stripped.out
dirsymlink: dirreal
diff --git a/lldb/test/API/macosx/function-starts/Makefile b/lldb/test/API/macosx/function-starts/Makefile
index 0d6f517293930..e379b8a6bee8e 100644
--- a/lldb/test/API/macosx/function-starts/Makefile
+++ b/lldb/test/API/macosx/function-starts/Makefile
@@ -5,4 +5,4 @@ MAKE_DSYM := NO
include Makefile.rules
main.o: main.cpp
- $(CC) $(CFLAGS_NO_DEBUG) -c $< -o $@
+ $(CC) $(CFLAGS_NO_DEBUG) -c $(call path_wrapper,$<) -o $@
diff --git a/lldb/test/API/macosx/posix_spawn/Makefile b/lldb/test/API/macosx/posix_spawn/Makefile
index 7ae46ca95828d..2119e15dda84f 100644
--- a/lldb/test/API/macosx/posix_spawn/Makefile
+++ b/lldb/test/API/macosx/posix_spawn/Makefile
@@ -6,13 +6,13 @@ include Makefile.rules
all: fat.out
x86_64.out: x86_64.c
- $(CC) -isysroot $(SDKROOT) -target x86_64-apple-macosx10.9 -o x86_64.out $<
+ $(CC) -isysroot $(SDKROOT) -target x86_64-apple-macosx10.9 -o x86_64.out $(call path_wrapper,$<)
x86_64h.out: x86_64h.c
- $(CC) -isysroot $(SDKROOT) -target x86_64h-apple-macosx10.9 -o x86_64h.out $<
+ $(CC) -isysroot $(SDKROOT) -target x86_64h-apple-macosx10.9 -o x86_64h.out $(call path_wrapper,$<)
arm64.out: arm64.c
- $(CC) -isysroot $(SDKROOT) -target arm64-apple-macosx10.9 -o arm64.out $<
+ $(CC) -isysroot $(SDKROOT) -target arm64-apple-macosx10.9 -o arm64.out $(call path_wrapper,$<)
fat.out: x86_64.out x86_64h.out arm64.out
$(LIPO) -o fat.out -create $^
diff --git a/lldb/test/API/macosx/universal/Makefile b/lldb/test/API/macosx/universal/Makefile
index 7d4762f240874..13fce3bdc0249 100644
--- a/lldb/test/API/macosx/universal/Makefile
+++ b/lldb/test/API/macosx/universal/Makefile
@@ -8,13 +8,13 @@ testit: testit.x86_64h testit.x86_64
lipo -create -o testit $^
testit.x86_64h: testit.x86_64h.o
- $(CC) -isysroot $(SDKROOT) -target x86_64h-apple-macosx10.9 -o testit.x86_64h $<
+ $(CC) -isysroot $(SDKROOT) -target x86_64h-apple-macosx10.9 -o testit.x86_64h $(call path_wrapper,$<)
testit.x86_64: testit.x86_64.o
- $(CC) -isysroot $(SDKROOT) -target x86_64-apple-macosx10.9 -o testit.x86_64 $<
+ $(CC) -isysroot $(SDKROOT) -target x86_64-apple-macosx10.9 -o testit.x86_64 $(call path_wrapper,$<)
testit.x86_64h.o: main.c
- $(CC) -isysroot $(SDKROOT) -g -O0 -target x86_64h-apple-macosx10.9 -c -o testit.x86_64h.o $<
+ $(CC) -isysroot $(SDKROOT) -g -O0 -target x86_64h-apple-macosx10.9 -c -o testit.x86_64h.o $(call path_wrapper,$<)
testit.x86_64.o: main.c
- $(CC) -isysroot $(SDKROOT) -g -O0 -target x86_64-apple-macosx10.9 -c -o testit.x86_64.o $<
+ $(CC) -isysroot $(SDKROOT) -g -O0 -target x86_64-apple-macosx10.9 -c -o testit.x86_64.o $(call path_wrapper,$<)
diff --git a/lldb/test/API/macosx/universal64/Makefile b/lldb/test/API/macosx/universal64/Makefile
index bfbfc3d67c06a..1e3445dfc5466 100644
--- a/lldb/test/API/macosx/universal64/Makefile
+++ b/lldb/test/API/macosx/universal64/Makefile
@@ -12,13 +12,13 @@ fat.out: fat.x86_64h.out fat.x86_64.out
lipo -fat64 -create -o $@ $^
fat.x86_64.out: fat.x86_64.o
- $(CC) -isysroot $(SDKROOT) -target x86_64-apple-macosx10.9 -o $@ $<
+ $(CC) -isysroot $(SDKROOT) -target x86_64-apple-macosx10.9 -o $@ $(call path_wrapper,$<)
fat.x86_64h.out: fat.x86_64h.o
- $(CC) -isysroot $(SDKROOT) -target x86_64h-apple-macosx10.9 -o $@ $<
+ $(CC) -isysroot $(SDKROOT) -target x86_64h-apple-macosx10.9 -o $@ $(call path_wrapper,$<)
fat.x86_64.o: main.c
- $(CC) -isysroot $(SDKROOT) -g -O0 -target x86_64-apple-macosx10.9 -c -o $@ $<
+ $(CC) -isysroot $(SDKROOT) -g -O0 -target x86_64-apple-macosx10.9 -c -o $@ $(call path_wrapper,$<)
fat.x86_64h.o: main.c
- $(CC) -isysroot $(SDKROOT) -g -O0 -target x86_64h-apple-macosx10.9 -c -o $@ $<
+ $(CC) -isysroot $(SDKROOT) -g -O0 -target x86_64h-apple-macosx10.9 -c -o $@ $(call path_wrapper,$<)
diff --git a/lldb/test/API/source-manager/Makefile b/lldb/test/API/source-manager/Makefile
index 422ecd0c39908..47436df4ae1f4 100644
--- a/lldb/test/API/source-manager/Makefile
+++ b/lldb/test/API/source-manager/Makefile
@@ -4,4 +4,4 @@ include Makefile.rules
# Copy file into the build folder to enable the test to modify it.
main-copy.c: main.c
- cp -f $< $@
+ cp -f $(call path_wrapper,$<) $@
diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/Makefile b/lldb/test/API/tools/lldb-dap/breakpoint/Makefile
index 30a6400184933..ef81c2b25de0e 100644
--- a/lldb/test/API/tools/lldb-dap/breakpoint/Makefile
+++ b/lldb/test/API/tools/lldb-dap/breakpoint/Makefile
@@ -8,10 +8,10 @@ include Makefile.rules
# We copy the source files to move them to test source mapping
other-copy.c: other.c
- cp -f $< $@
+ cp -f $(call path_wrapper,$<) $@
main-copy.cpp: main.cpp
- cp -f $< $@
+ cp -f $(call path_wrapper,$<) $@
# The following shared library will be used to test breakpoints under dynamic loading
libother: other-copy.c
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be better to split this into smaller patches, given the fragility of this code. (Basically, one patch for each of your bullet points).
Also, can you elaborate on this part?
Paths are turned into POSIX-style since some tests and Unix utilities use them for manipulating files. It helps to avoid compiler/linker errors because of broken paths.
What exactly does it help with? Given that you're canonicalizing to a forward slash, does that mean that some of the tools you use don't accept backslashes (perhaps because they come from cygwin or the like)?
ifneq (,$(or \ | ||
$(findstring windows32,$(HOST_OS)),\ | ||
$(findstring MSYS_NT,$(HOST_OS)))) | ||
HOST_OS := Windows_NT | ||
endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to do this if we're already setting HOST_OS in python (7021e44)? If the python code is not sufficient, could we fix it (and delete this code) to keep everything in one place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BUILDDIR := $(shell echo %cd%) | ||
|
||
ifneq (,$(filter $(OS), Linux Android)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this based on the target OS? I would expect that something like this would be keyed off of host os being windows, not target os not being linux.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed this from the PR, together with path_wrapper thing.
@@ -296,11 +313,13 @@ endif | |||
CFLAGS += $(CFLAGS_EXTRAS) | |||
CXXFLAGS += -std=c++11 $(CFLAGS) $(ARCH_CXXFLAGS) | |||
LD = $(CC) | |||
LDFLAGS ?= $(CFLAGS) | |||
# Copy common options to the linker flags (dwarf, arch. & etc). | |||
#Note: we get some 'garbage' options for linker here (such as -I, --isystem & etc). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add space after '#'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
LDFLAGS += $(LD_EXTRAS) $(ARCH_LDFLAGS) | ||
ifeq (,$(filter $(OS), Windows_NT Android Darwin)) | ||
ifneq (,$(filter YES,$(ENABLE_THREADS))) | ||
LDFLAGS += -pthread | ||
LDFLAGS += -lpthread |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding was that -pthread
is the recommended flag here. Why change it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
else | ||
ifeq "$(OS)" "Android" | ||
# Nothing to do, this is already handled in | ||
# Android.rules. | ||
else | ||
CXXFLAGS += -stdlib=libc++ | ||
LDFLAGS += -stdlib=libc++ | ||
LDFLAGS += -stdlib=libc++ -lc++abi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this necessary. Is this a static/dynamic linking thing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Moved to #99589 for clarity.
LDFLAGS += -L$(LIBCPP_LIBRARY_DIR) -Wl,-rpath,$(LIBCPP_LIBRARY_DIR) -lc++ -lc++abi | ||
# Otherwise no C++ library has been specified. Use stdc++ by default. | ||
else | ||
USE_LIBSTDCPP := 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this default doesn't make sense for darwin and windows builds. Could we default to the system library instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed it to USE_SYSTEM_STDLIB
else | ||
ifneq (,$(findstring clang,$(CC))) | ||
# Force clang looking for the gcc's headers at specific rootfs folder. | ||
CXXFLAGS += -stdlib=libstdc++ $(GCC_TOOLCHAIN_FLAGS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
..and then change this to not pass any flags by default? (I don't know what -stdlib=libstdc++
will do for windows, but I don't expect it to go down well)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This still forces the usage of libstdc++ on windows, right? Could we just pass nothing here and let clang use its default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This still forces the usage of libstdc++ on windows, right? Could we just pass nothing here and let clang use its default?
Actually, it's ignored on Windows target. We have some problems without that line during cross-compiling, but I think it's due to missing $(GCC_TOOLCHAIN_FLAGS) content, not stdlib selection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This still forces the usage of libstdc++ on windows, right? Could we just pass nothing here and let clang use its default?
Sorry, that was too soon.
We still need this flag. It is ignored for Windows target, but clang tries to pass -lc++ to linker in case of Linux target if -lstdc++ is not specified, and ld.lld fails to link since libc++abi is linked separately from libc++ in our config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, but that is a clang configuration thing (CLANG_DEFAULT_CXX_STDLIB
cmake option), right? Can't you set it to the actual c++ library that you intend to use instead of overriding it here? Or use a clang wrapper or a configuration file to override it externally? (FWIW, that's what we do internally to adapt the test suite to our (strange) internal infrastructure)
It's nice (maybe) that -lstdc++
is ignored on windows, but I still think this is the wrong thing to do. libstdc++
is definitely not the system default everywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed -stdlib=libstdc++
. The problem was that I was trying to link with in-tree libcxx not linked statically to libcxxabi, but clang driver couldn't use it without the explicit -lc++abi flag (it doesn't distinguish libcxx compilation types according to https://releases.llvm.org/6.0.0/projects/libcxx/docs/BuildingLibcxx.html#using-libcxxrt-on-linux).
Adding LIBCXX_ENABLE_STATIC_ABI_LIBRARY helps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense. The fix sounds good to me, but what does that mean for #99589 ? Is that compatible with this? Should we drop it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seemingly we don't need it anymore. I launched the testing on our CI. As soon as it shows that it makes no difference without -lc++abi, I'll close #99589.
As far as I understand, MinGW make, on which Chocolatey build is based, prefers \ or / slashes. I get errors like this, that don't show up with path_wrapper change.
|
I was thinking about that, but for me it looked like path_wrapper thing and cross-compilation/sdkroot fixes should have been sent in the same commit since each of them is insufficient for setting this configuration up on Windows. |
Moved SHELL variable detection change to a separate commit #99532. |
I understand. In terms of raw LOC, this patch isn't really big, but it has some fragile and potentially controversial parts, so I think it'd be easier to handle it in parts. Thanks for splitting it up. |
I don't understand this sentence. Are you saying it prefers forward slashes or backward slashes (or uniform slashes)?
This is interesting. I'm surprised that errors are coming from llvm tools, as the llvm path libraries treat forward and backward slashes (on windows) as equivalent at a very low level. Could you try to reduce this to to figure out which specific slash in the command is causing the error? I want to make sure we're not working around a bug here... |
7e79d7c
to
a54680b
Compare
When we were setting up cross-platform build and testing config, we had a multi-screen list of errors we had to solve. Also, we were unsure about the environment in which we would run this. Therefore we decided to bulk normalize all paths and some flags. |
a54680b
to
1b20182
Compare
Got it. Thanks for clarifying. This makes the patch much easier to review. I think the main thing left to figure out here is the "default to libstdc++" part. |
1b20182
to
1d074ba
Compare
ifeq (1,$(USE_LIBSTDCPP)) | ||
# Clang requires an extra flag: -stdlib=libstdc++ | ||
ifneq (,$(findstring clang,$(CC))) | ||
CXXFLAGS += -stdlib=libstdc++ | ||
LDFLAGS += -stdlib=libstdc++ | ||
# Force clang looking for the gcc's headers at specific rootfs folder. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there's some tab/space problem here. Could you make sure you use whatever formatting the original file uses? If make
is fine with it, I'm also open to reformatting the whole file to e.g. remove tabs (as a separate patch).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
@@ -403,7 +429,7 @@ ifeq (1,$(USE_LIBCPP)) | |||
endif | |||
ifneq (,$(filter $(OS), FreeBSD Linux NetBSD)) | |||
ifneq (,$(LLVM_LIBS_DIR)) | |||
LDFLAGS += -Wl,-rpath,$(LLVM_LIBS_DIR) | |||
LDFLAGS += -Wl,-rpath,$(LLVM_LIBS_DIR) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
else | ||
ifneq (,$(findstring clang,$(CC))) | ||
# Force clang looking for the gcc's headers at specific rootfs folder. | ||
CXXFLAGS += -stdlib=libstdc++ $(GCC_TOOLCHAIN_FLAGS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This still forces the usage of libstdc++ on windows, right? Could we just pass nothing here and let clang use its default?
2756efd
to
4699a3b
Compare
These changes are aimed to support cross compilation build on Windows host for Linux target for API tests execution. They're not final: changes will follow for refactoring and adjustments to make all tests passing. Chocolatey make is recommended to use, since it is maintained better than GnuWin32 recommended here https://lldb.llvm.org/resources/build.html#codesigning (it was updated last time in 2010) and helps to avoid problems with building tests (for example, GnuWin32 doesn't support long paths and there are some other failures with building for Linux with it). Co-authored-by: Pavel Labath <[email protected]>
4699a3b
to
4e8dec0
Compare
@labath are there any comments from you that haven't been resolved yet? I feel that I may be missing something, but I don't see exactly what. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks fine. Thanks for the reminder.
Thank you! |
These changes aim to support cross-compilation build on Windows host for Linux target for API tests execution. They're not final: changes will follow for refactoring and adjustments to make all tests pass.
Chocolatey make is recommended to be used since it is maintained better than GnuWin32 mentioned here https://lldb.llvm.org/resources/build.html#windows (latest GnuWin32 release is dated by 2010) and helps to avoid problems with building tests (for example, GnuWin32 make doesn't support long paths and there are some other failures with building for Linux with it).