A few build/portability fixes

Hello,

I think these should be harmless enough.

- Remove a bunch of trailing commas at the end of enumerated lists.
Fixes compilation with -fpedantic (and is more valid c++ anyway)

- Factor some harcoded python paths/flags into make variables. This allows users
to override these when building in other "environments". The correct
way, of course,
is to have the buildsystem extract these from python-config, but I'm not sure if
that'll need to be in llvm's configure script or in lldb.

resolve_storeinst.patch (659 Bytes)

python_override.patch (1.98 KB)

trailing_commas_in_enums.patch (2.61 KB)

Hi Jai,

Jai Menon <jmenon86@gmail.com> writes:

Hello,

I think these should be harmless enough.

- Remove a bunch of trailing commas at the end of enumerated lists.
Fixes compilation with -fpedantic (and is more valid c++ anyway)

- Factor some harcoded python paths/flags into make variables. This
allows users to override these when building in other
"environments". The correct way, of course, is to have the buildsystem
extract these from python-config, but I'm not sure if that'll need to
be in llvm's configure script or in lldb.

I think using python-config from within the lldb makefiles should be OK
(perhaps one day LLDB will grow its own configure stage so we can
test for the existence of python-config).

Perhaps something like the below -- could you test on Darwin?

All the other patches look good to me.

diff --git a/Makefile b/Makefile
index 528c36c..9afcad2 100644
--- a/Makefile
+++ b/Makefile
@@ -33,10 +33,11 @@ CPP.Flags += -I$(PROJ_SRC_DIR)/$(LLDB_LEVEL)/include
CPP.Flags += -I$(PROJ_OBJ_DIR)/$(LLDB_LEVEL)/include
CPP.Flags += -I$(PROJ_SRC_DIR)/$(LLDB_LEVEL)/../clang/include
CPP.Flags += -I$(PROJ_OBJ_DIR)/$(LLDB_LEVEL)/../clang/include
-CPP.Flags += -I/usr/include/python2.6
CPP.Flags += -I$(PROJ_SRC_DIR)/$(LLDB_LEVEL)/source
CPP.Flags += -I$(PROJ_SRC_DIR)/$(LLDB_LEVEL)/source/Utility
CPP.Flags += -I$(PROJ_SRC_DIR)/$(LLDB_LEVEL)/source/Plugins/Process/Utility
+CPP.Flags += $(shell python-config --includes)

Hi Stephen,

Hi Jai,

Jai Menon <jmenon86@gmail.com> writes:

Hello,

I think these should be harmless enough.

- Remove a bunch of trailing commas at the end of enumerated lists.
Fixes compilation with -fpedantic (and is more valid c++ anyway)

- Factor some harcoded python paths/flags into make variables. This
allows users to override these when building in other
"environments". The correct way, of course, is to have the buildsystem
extract these from python-config, but I'm not sure if that'll need to
be in llvm's configure script or in lldb.

I think using python-config from within the lldb makefiles should be OK
(perhaps one day LLDB will grow its own configure stage so we can
test for the existence of python-config).

Indeed, an autotools based system would be a great addition. I'm not
sure what the general opinion of this list is on that topic though.

Using python-config is a step up in terms of portability, but this
approach still has the minor issue of not working out-of-the-box on
systems where python-config corresponds to python3.x and
python2[.x]-config corresponds to what we need (an example of this is
archlinux). But I guess that can be fixed when lldb gets a
configuration mechanism.

Perhaps something like the below -- could you test on Darwin?

Ah, I don't have access to any darwin boxes :expressionless:
Perhaps one of the developers might want to test it out on their machines?
FWIW, the change looks good to me.

All the other patches look good to me.

Great, I'll see if I run into more serious bugs later :slight_smile:

diff --git a/Makefile b/Makefile
index 528c36c..9afcad2 100644
--- a/Makefile
+++ b/Makefile
@@ -33,10 +33,11 @@ CPP.Flags += -I$(PROJ_SRC_DIR)/$(LLDB_LEVEL)/include
CPP.Flags += -I$(PROJ_OBJ_DIR)/$(LLDB_LEVEL)/include
CPP.Flags += -I$(PROJ_SRC_DIR)/$(LLDB_LEVEL)/../clang/include
CPP.Flags += -I$(PROJ_OBJ_DIR)/$(LLDB_LEVEL)/../clang/include
-CPP.Flags += -I/usr/include/python2.6
CPP.Flags += -I$(PROJ_SRC_DIR)/$(LLDB_LEVEL)/source
CPP.Flags += -I$(PROJ_SRC_DIR)/$(LLDB_LEVEL)/source/Utility
CPP.Flags += -I$(PROJ_SRC_DIR)/$(LLDB_LEVEL)/source/Plugins/Process/Utility
+CPP.Flags += $(shell python-config --includes)
+
ifeq ($(HOST_OS),Darwin)
CPP.Flags += -F/System/Library/Frameworks -F/System/Library/PrivateFrameworks
endif
diff --git a/lib/Makefile b/lib/Makefile
index fffcab6..3440663 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -84,8 +84,9 @@ ifeq ($(HOST_OS),Darwin)
LLVMLibsOptions += -avoid-version
LLVMLibsOptions += -F/System/Library/Frameworks -F/System/Library/PrivateFrameworks
LLVMLibsOptions += -framework Foundation -framework CoreFoundation
- LLVMLibsOptions += -framework DebugSymbols -lpython2.6 -lobjc
+ LLVMLibsOptions += -framework DebugSymbols -lobjc
LLVMLibsOptions += -Wl,-exported_symbols_list -Wl,"$(PROJ_SRC_DIR)/$(LLDB_LEVEL)/resources/lldb-framework-exports"
+ LLVMLibsOptions += $(shell python-config --ldflags)
# Mac OS X 10.4 and earlier tools do not allow a second -install_name on command line
DARWIN_VERS := $(shell echo $(TARGET_TRIPLE) | sed 's/.*darwin\([0-9]*\).*/\1/')
ifneq ($(DARWIN_VERS),8)
@@ -101,5 +102,5 @@ ifeq ($(HOST_OS), Linux)
# Don't allow unresolved symbols.
LLVMLibsOptions += -Wl,--no-undefined
# Link in python
- LD.Flags += -lpython2.6
+ LD.Flags += $(shell python-config --ldflags)
endif

Minor nit : maybe `python-config --libs` is more consistent with the
previous hunk, but that's not very important.

Thanks.

Any comments on this? Is there anything else I need to do to get these applied?

Jai ++

i was about to submit the same thing you called to resolve_storinst.patch , and i think the person i’ve been working with will probably apply it.

i had turned on -pedantic as well, but got so many errors in my effort to get it building that i turned it back off for the time being. but your trailing_commas_in_enums.patch looks fine to me, and perhaps it can get committed as is.

i see there was already a comment on your python patch … no further comment from me either way.

++ kirk

Sorry for the delay, All these have been checked in!