Only throw -fPIC when building a shared library

[asmith]

Summary

Update makefiles to specify -fPIC in Makefile.rules and only throw -fPIC when building a shared library. This change is necessary to allow building the lldb tests on Windows where -fPIC is not a valid option.

Update a few places to Python 3.x syntax

[zturner]

In the future when you upload diffs can you include context? (i.e. git diff -U999999). It's nice to be able to see the surrounding code when I'm looking at a diff.

Is there ever a case where you would want to build a shared library without -fPIC? I'm wondering if we should just update the common Makefile.rules and if DYLIB_NAME is set (or something else indicating that this is a shared library), then we put the logic about -fPIC in that common file. Then people wouldn't have to remember to get this right in the future.

[labath]

In D42994#999891, @zturner wrote:

In the future when you upload diffs can you include context? (i.e. git diff -U999999). It's nice to be able to see the surrounding code when I'm looking at a diff.

Is there ever a case where you would want to build a shared library without -fPIC? I'm wondering if we should just update the common Makefile.rules and if DYLIB_NAME is set (or something else indicating that this is a shared library), then we put the logic about -fPIC in that common file. Then people wouldn't have to remember to get this right in the future.

Adding -fPIC by default sounds like a great idea. Building a shared library without it will probably break compilation, but if some test still wants to try it, it can be turned off with -fno-PIC.

[labath]

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

550 ↗

(On Diff #133362)

I think Zachary meant only when building the shared library, but you're doing it unconditionally. This would be a nice place to set the flag for shared library only.

[zturner]

This lgtm. If this causes some tests that were previously skipped or xfailed to start passing, you can unskip / unxfail them at the same time.

packages/Python/lldbsuite/test/functionalities/process_attach/TestProcessAttach.py

49 ↗

(On Diff #133467)

Weird, does that comma syntax even do anything or was that just a bug?

[stella.stamenova]

packages/Python/lldbsuite/test/functionalities/process_attach/TestProcessAttach.py

49 ↗

(On Diff #133467)

In python 2.5 or earlier it meant the same as "as" but "as" was not supported. Then for a while (until 3.x) both the comma and "as" meant the same thing and then since 3.x "as" is the only option allowed.

packages/Python/lldbsuite/test/functionalities/breakpoint/global_constructor/Makefile

 DYLIB_NAME := foo
 DYLIB_CXX_SOURCES := foo.cpp
 CXX_SOURCES := main.cpp
-CFLAGS_EXTRAS := -fPIC
+
+ifneq "$(OS)" "Windows_NT"
+  CFLAGS_EXTRAS := -fPIC
+endif
 
 include $(LEVEL)/Makefile.rules

packages/Python/lldbsuite/test/functionalities/breakpoint/move_nearest/Makefile

 DYLIB_NAME := foo
 DYLIB_CXX_SOURCES := foo.cpp
 CXX_SOURCES := main.cpp
-CFLAGS_EXTRAS += -fPIC
+
+ifneq "$(OS)" "Windows_NT"
+  CFLAGS_EXTRAS += -fPIC
+endif
 
 include $(LEVEL)/Makefile.rules

packages/Python/lldbsuite/test/lang/c/conflicting-symbol/One.mk

 
 include $(LEVEL)/Makefile.rules
 
-CFLAGS_EXTRAS += -fPIC
+ifneq "$(OS)" "Windows_NT"
+  CFLAGS_EXTRAS += -fPIC
+endif
 
 OneConstant.o: OneConstant.c
   $(CC) $(CFLAGS_NO_DEBUG) -c $< -o $@

packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Two.mk

 
 include $(LEVEL)/Makefile.rules
 
-CFLAGS_EXTRAS += -fPIC
+ifneq "$(OS)" "Windows_NT"
+  CFLAGS_EXTRAS += -fPIC
+endif
 
 TwoConstant.o: TwoConstant.c
   $(CC) $(CFLAGS_NO_DEBUG) -c $< -o $@

packages/Python/lldbsuite/test/lang/c/shared_lib/Makefile

 DYLIB_NAME := foo
 DYLIB_C_SOURCES := foo.c
 C_SOURCES := main.c
-CFLAGS_EXTRAS += -fPIC
+
+ifneq "$(OS)" "Windows_NT"
+  CFLAGS_EXTRAS += -fPIC
+endif
 
 include $(LEVEL)/Makefile.rules

packages/Python/lldbsuite/test/lang/c/shared_lib_stripped_symbols/Makefile

 DYLIB_NAME := foo
 DYLIB_C_SOURCES := foo.c
 C_SOURCES := main.c
-CFLAGS_EXTRAS += -fPIC
+
+ifneq "$(OS)" "Windows_NT"
+  CFLAGS_EXTRAS += -fPIC
+endif
 
 SPLIT_DEBUG_SYMBOLS = YES
 

packages/Python/lldbsuite/test/lang/cpp/namespace_definitions/a.mk

 DYLIB_CXX_SOURCES := a.cpp
 DYLIB_ONLY := YES
 
-CXXFLAGS += -fPIC
+ifneq "$(OS)" "Windows_NT"
+  CXXFLAGS += -fPIC
+endif
 
 include $(LEVEL)/Makefile.rules

packages/Python/lldbsuite/test/lang/cpp/namespace_definitions/b.mk

 DYLIB_CXX_SOURCES := b.cpp
 DYLIB_ONLY := YES
 
-CXXFLAGS += -fPIC
+ifneq "$(OS)" "Windows_NT"
+  CXXFLAGS += -fPIC
+endif
 
 include $(LEVEL)/Makefile.rules

packages/Python/lldbsuite/test/lldbtest.py

             d = {
                 'DYLIB_CXX_SOURCES': sources,
                 'DYLIB_NAME': lib_name,
-                'CFLAGS_EXTRAS': "%s -I%s -fPIC" % (stdflag,
-                                                    os.path.join(
-                                                        os.environ["LLDB_SRC"],
-                                                        "include")),
+                'CFLAGS_EXTRAS': "%s -I%s " % (stdflag,
                 'LD_EXTRAS': "-shared -l%s\liblldb.lib" % self.os.environ["LLDB_IMPLIB_DIR"]}
         if self.TraceOn():
             print(