Hi Wookey,

Unfortunately for some reason, two of the four patch files would not apply - to either tag v0.8 or the current v0.8.1 (should have worked with both as the makefile had no significant change between 0.8.0 and 0.8.1. However, I've had a look and comments inline,,,

On 9 March 2018 at 17:58, Mathieu Poirier mathieu.poirier@linaro.org wrote:

On 7 March 2018 at 22:18, Wookey wookey@wookware.org wrote:

...

I've been packaging libopenCSD for debian. That has resulted in various changes, largely to the makefiles. Most of those changes should go upstream, rather than exist only in the debian version.

I'll post my issues/changes here for discussion so we can decide what is upstreamable.

In the meantime anyone interested can try the debian packages here: http://wookware.org/software/repo/ either manually from: http://wookware.org/software/repo/pool/main/libo/libopencsd/

or with deb [ trusted=yes ] http://wookware.org/software/repo unstable main apt update; apt install libopencsd0

I was under the impression (per the docs links you sent) that the library naming convention was to have the version as libopencsd.so.0.8.0 (and variants) rather than libopencsd0.... Not sure what the 0 postfixed to the name is for - though I may be missing something here.

...

(and/or libopencsd-dev libopencsd-doc libopencsd-dbgsym)

I have test this and things install and work seamlessly.

...

(for debian unstable, amd64 only) (package is signed, but repo isn't. Sorry) Or get the source and rebuild for a different debian-based distro/release.

I've not separated all my changes into proper standalone patches yet, but some are so let's start with those.

1. The doxygen doc build doesn't find all the components because they moved.

This fixes that: Index: libopencsd-0.8.0/decoder/docs/doxygen_config.dox =================================================================== --- libopencsd-0.8.0.orig/decoder/docs/doxygen_config.dox +++ libopencsd-0.8.0/decoder/docs/doxygen_config.dox @@ -765,11 +765,11 @@ WARN_LOGFILE =

INPUT = ../include \ ../include/interfaces \

•                     ../include/etmv3 \
  

•                     ../include/etmv4 \
  

•                     ../include/ptm \
  

•                     ../include/c_api \
  

•                     ../include/stm \
  

•                     ../include/opencsd/etmv3 \
  

•                     ../include/opencsd/etmv4 \
  

•                     ../include/opencsd/ptm \
  

•                     ../include/opencsd/c_api \
  

•                     ../include/opencsd/stm \
                      ../include/mem_acc \
                      ../../README.md \
                      . \
  
  
  

The makefile provides no doc-build. The patch below adds that.

I didn't include an install-docs target, although if you added one with a variable to set the target dir then I'd use it.

No problem here - and this one did apply.

...

Index: libopencsd-0.8.0/decoder/build/linux/makefile

--- libopencsd-0.8.0.orig/decoder/build/linux/makefile +++ libopencsd-0.8.0/decoder/build/linux/makefile @@ -155,6 +155,13 @@ tests: libs cd $(OCSD_ROOT)/tests/build/linux/trc_pkt_lister && make cd $(OCSD_ROOT)/tests/build/linux/c_api_pkt_print_test && make

+# +# build docs +.PHONY: docs +docs:

•   (cd $(OCSD_ROOT/docs); doxygen doxygen_config.dox)
  

############################################################# # clean targets # @@ -176,3 +183,4 @@ clean_install: rm -f $(INSTALL_LIB_DIR)/lib$(LIB_BASE_NAME).so rm -f $(INSTALL_LIB_DIR)/lib$(LIB_CAPI_NAME).so rm -rf $(INSTALL_INCLUDE_DIR)/$(LIB_UAPI_INC_DIR)

•   rm -rf $(OCSD_ROOT)/docs/html
  
  

Happy with this too - for some reason the indexing was out compared to my system - did not apply.

...

3. The static libraries are built but not installed.

Now static libraries aren't much use to anyone these days, but if you build them then you might as well install them.

This patch does that: Index: libopencsd-0.8.0/decoder/build/linux/makefile =================================================================== --- libopencsd-0.8.0.orig/decoder/build/linux/makefile +++ libopencsd-0.8.0/decoder/build/linux/makefile @@ -136,6 +136,8 @@ mkdir -p $(INSTALL_LIB_DIR) $(INSTALL_INCLUDE_DIR) $(INSTALL) --mode=644 $(LIB_TARGET_DIR)/lib$(LIB_BASE_NAME).so $(INSTALL_LIB_DIR)/ $(INSTALL) --mode=644 $(LIB_TARGET_DIR)/lib$(LIB_CAPI_NAME).so $(INSTALL_LIB_DIR)/

•   $(INSTALL) --mode=644 $(LIB_TARGET_DIR)/lib$(LIB_BASE_NAME).a $(INSTALL_LIB_DIR)/
  

•   $(INSTALL) --mode=644 $(LIB_TARGET_DIR)/lib$(LIB_CAPI_NAME).a $(INSTALL_LIB_DIR)/
    cd $(OCSD_ROOT)/build/linux/rctdl_c_api_lib && make install_inc
  
  

################################ @@ -192,6 +194,6 @@ cd $(OCSD_ROOT)/tests/build/linux/c_api_pkt_print_test && make clean

clean_install:

•   rm -f $(INSTALL_LIB_DIR)/lib$(LIB_BASE_NAME).so
  

•   rm -f $(INSTALL_LIB_DIR)/lib$(LIB_CAPI_NAME).so
  

•   rm -f $(INSTALL_LIB_DIR)/lib$(LIB_BASE_NAME).{so,a}
  

•   rm -f $(INSTALL_LIB_DIR)/lib$(LIB_CAPI_NAME).{so,a}
    rm -rf $(INSTALL_INCLUDE_DIR)/$(LIB_UAPI_INC_DIR)
  
  
  

Fine with this, though again as above did not apply.

...

4. The makefile arbitrarily restricts the build to arm and x86

architectures. The only reason for this is in order to build into a particular named directory, and to set -m32/-m64 flags for x86 which should be defaulted correctly on any sensible toolchain or cross-toolchain anyway.

Not sure this actually restricts the build architecture - merely customises target directory names. If ARCH is set to anything other than those filtered then the output will be in an undecorated ./linux/ directory, I'd agree with the comments re "sensible" defaults, but I've never seen any reason not to be specific to avoid unnecessary support calls.

...

This code can build, and be used, on any arch and the makefile should allow that, so this should be fixed.

I don't see the need to build into a host-arch named PLAT_DIR directory. Is there one? It only does one build at a time, whether crossing or not, so I see no reason not to use a fixed name for this dir, such as 'builddir'. This has no effect on the ability to cross or not. If you really want to change the name of PLAT_DIR then find out the HOST triplet and use that. On debian this is dpkg-architecture -q DEB_HOST_GNU_TYPE, (which is also the same prefix as would be used to specify the toolchain prefix for crossing). But like I say, I think a fixed builddir is actually all you need unless I'm missing something.

The reason behind this is twofold. 1) having explicit dbg/rel directories in my development environment allows me to build clients and pick out precisely the library version I want. This means that a command sequence such as: make clean make make DEBUG=1 will work, without the explicit dbg/rel dirs, the last command simply re-links the release code - which is not what I want.

2) having explicit bit / arch suffixed directories again helps in development - both on linux and windows (yes - it is a cross platform project so consistency is important). At ARM we have a system where we batch build libraries and keep all the build variants in sub-directories for later client builds. This reflects that practice too, so I guess for me there is a certain amount of "force of habit" there.

I have no objection to either the packager building with ARCH=<something-else> to get a predictable directory name, or indeed adding in extra branch on the filter to create an additional standard name. e.g. .... else BIT_VARIANT=-std endif ... so the default build will build to .../linux-std/rel

so although this patch does apply - I'd really rather not!

Thanks for the input.

Mike

...

So this patch lets the build work on any arch: Index: libopencsd-0.8.0/decoder/build/linux/makefile =================================================================== --- libopencsd-0.8.0.orig/decoder/build/linux/makefile +++ libopencsd-0.8.0/decoder/build/linux/makefile @@ -92,27 +92,6 @@ BUILD_VARIANT=rel endif

-# platform bit size variant -ifeq ($(ARCH),x86)

• MFLAG:="-m32"
• BIT_VARIANT=32

-else ifeq ($(ARCH),x86_64)

• MFLAG:="-m64"
• BIT_VARIANT=64

-else ifeq ($(ARCH),arm)

• BIT_VARIANT=-arm

-else ifeq ($(ARCH),arm64)

• BIT_VARIANT=-arm64

-else ifeq ($(ARCH),aarch64)

• BIT_VARIANT=-arm64

-else ifeq ($(ARCH),aarch32)

• BIT_VARIANT=-arm

-endif

-MASTER_CC_FLAGS += $(MFLAG) -MASTER_CPP_FLAGS += $(MFLAG) -MASTER_LINKER_FLAGS += $(MFLAG)

# export build flags export MASTER_CC_FLAGS export MASTER_CPP_FLAGS @@ -120,7 +99,7 @@ export MASTER_LINKER_FLAGS export MASTER_LIB_FLAGS

# target directories -export PLAT_DIR=linux$(BIT_VARIANT)/$(BUILD_VARIANT) +export PLAT_DIR=builddir export LIB_TARGET_DIR=$(OCSD_LIB_ROOT)/$(PLAT_DIR) export LIB_TEST_TARGET_DIR=$(OCSD_TESTS)/lib/$(PLAT_DIR) export BIN_TEST_TARGET_DIR=$(OCSD_TESTS)/bin/$(PLAT_DIR)

I'm in agreement with the proposed changes - Mike please review on your side.

Thanks, Mathieu

...

All these patches included as files too.

Wookey

Principal hats: Linaro, Debian, Wookware, ARM http://wookware.org/

---

CoreSight mailing list CoreSight@lists.linaro.org https://lists.linaro.org/mailman/listinfo/coresight

---

CoreSight mailing list CoreSight@lists.linaro.org https://lists.linaro.org/mailman/listinfo/coresight

On 2018-03-13 16:19 +0000, Wookey wrote:

On 2018-03-08 05:18 +0000, Wookey wrote: (this is not working quite right, so I'll send this mail now, as relevant to the discussion or the first set of patches, and a finished patch later)

OK. Attached is the patch. As well as adding the SONAME setting, and installing, it sets out the actual dependencies for the links and libraries, using order-only dependencies for the directories. This is -j safe, unlike using mkdir -p, and avoids using a sentinel file for each dir (which would also work if you prefer that for some reason).

I've set it to use the release version, rather than 0.0.0, assuming that you want to go with that. (I guess I should move on to 0.8.1 before uploading this?)

No doubt this could be improved, but it is now at least reasonably correct and only builds things that need building.

I note that the changed sections for the two submakefiles in rctdl_c_api_lib and ref_trace_decode_lib only differ by one word (the rctdl_c_api_lib shared lib build has $(SO_LIB_DEPS) where ref_trace_decode_lib doesn't). I do wonder if there is room for some merging to only have one set of rules for all that?

Index: libopencsd-0.8.0/decoder/build/linux/makefile =================================================================== --- libopencsd-0.8.0.orig/decoder/build/linux/makefile +++ libopencsd-0.8.0/decoder/build/linux/makefile @@ -104,6 +104,13 @@ export LIB_TARGET_DIR=$(OCSD_LIB_ROOT)/$ export LIB_TEST_TARGET_DIR=$(OCSD_TESTS)/lib/$(PLAT_DIR) export BIN_TEST_TARGET_DIR=$(OCSD_TESTS)/bin/$(PLAT_DIR)

+# verion +export SO_MAJOR_VER = 0 +SO_MINOR_VER = 8 +SO_PATCH_VER = 0 +export SO_VER = $(SO_MAJOR_VER).$(SO_MINOR_VER).$(SO_PATCH_VER) + + ########################################################### # build targets

@@ -113,8 +120,12 @@ libs: $(LIB_BASE_NAME)_lib $(LIB_CAPI_N

install: libs mkdir -p $(INSTALL_LIB_DIR) $(INSTALL_INCLUDE_DIR) - $(INSTALL) --mode=644 $(LIB_TARGET_DIR)/lib$(LIB_BASE_NAME).so $(INSTALL_LIB_DIR)/ - $(INSTALL) --mode=644 $(LIB_TARGET_DIR)/lib$(LIB_CAPI_NAME).so $(INSTALL_LIB_DIR)/ + cp -d $(LIB_TARGET_DIR)/lib$(LIB_BASE_NAME).so $(INSTALL_LIB_DIR)/ + cp -d $(LIB_TARGET_DIR)/lib$(LIB_BASE_NAME).so.$(SO_MAJOR_VER) $(INSTALL_LIB_DIR)/ + $(INSTALL) --mode=644 $(LIB_TARGET_DIR)/lib$(LIB_BASE_NAME).so.$(SO_VER) $(INSTALL_LIB_DIR)/ + cp -d $(LIB_TARGET_DIR)/lib$(LIB_CAPI_NAME).so $(INSTALL_LIB_DIR)/ + cp -d $(LIB_TARGET_DIR)/lib$(LIB_CAPI_NAME).so.$(SO_MAJOR_VER) $(INSTALL_LIB_DIR)/ + $(INSTALL) --mode=644 $(LIB_TARGET_DIR)/lib$(LIB_CAPI_NAME).so.$(SO_VER) $(INSTALL_LIB_DIR)/ $(INSTALL) --mode=644 $(LIB_TARGET_DIR)/lib$(LIB_BASE_NAME).a $(INSTALL_LIB_DIR)/ $(INSTALL) --mode=644 $(LIB_TARGET_DIR)/lib$(LIB_CAPI_NAME).a $(INSTALL_LIB_DIR)/ cd $(OCSD_ROOT)/build/linux/rctdl_c_api_lib && make install_inc Index: libopencsd-0.8.0/decoder/build/linux/rctdl_c_api_lib/makefile =================================================================== --- libopencsd-0.8.0.orig/decoder/build/linux/rctdl_c_api_lib/makefile +++ libopencsd-0.8.0/decoder/build/linux/rctdl_c_api_lib/makefile @@ -55,13 +55,31 @@ OBJECTS=$(BUILD_DIR)/ocsd_c_api.o \ INST_INC_SRC=$(OCSD_INCLUDE)/$(LIB_UAPI_INC_DIR) INST_INC_DST=$(INSTALL_INCLUDE_DIR)/$(LIB_UAPI_INC_DIR)

-all: build_dir $(OBJECTS) - mkdir -p $(LIB_TARGET_DIR) +all: links + +links: $(LIB_TARGET_DIR)/$(LIB_NAME).so.$(SO_MAJOR_VER) $(LIB_TARGET_DIR)/$(LIB_NAME).so +.PHONY: links + +LIBS:= $(LIB_TARGET_DIR)/$(LIB_NAME).a $(LIB_TARGET_DIR)/$(LIB_NAME).so.$(SO_VER) + +$(LIB_TARGET_DIR): + mkdir $(LIB_TARGET_DIR) + +$(BUILD_DIR): + mkdir $(BUILD_DIR) + +$(LIB_TARGET_DIR)/$(LIB_NAME).a: $(OBJECTS) | $(BUILD_DIR) $(LIB_TARGET_DIR) $(LIB) $(LIB_FLAGS) $(LIB_TARGET_DIR)/$(LIB_NAME).a $(OBJECTS) - $(LINKER) $(LINKER_FLAGS) -o $(LIB_TARGET_DIR)/$(LIB_NAME).so -Wl,-soname,$(LIB_NAME).so $(OBJECTS) $(SO_LIB_DEPS)

-build_dir: - mkdir -p $(BUILD_DIR) +$(LIB_TARGET_DIR)/$(LIB_NAME).so.$(SO_VER): $(OBJECTS) | $(BUILD_DIR) $(LIB_TARGET_DIR) + $(LINKER) $(LINKER_FLAGS) -o $(LIB_TARGET_DIR)/$(LIB_NAME).so.$(SO_VER) -Wl,-soname,$(LIB_NAME).so.$(SO_MAJOR_VER) $(OBJECTS) $(SO_LIB_DEPS) + +$(LIB_TARGET_DIR)/$(LIB_NAME).so.$(SO_MAJOR_VER): $(LIBS) | $(LIB_TARGET_DIR) + ( cd $(LIB_TARGET_DIR); ln -s $(LIB_NAME).so.$(SO_VER) $(LIB_NAME).so.$(SO_MAJOR_VER) ) + +$(LIB_TARGET_DIR)/$(LIB_NAME).so: $(LIB_TARGET_DIR)/$(LIB_NAME).so.$(SO_MAJOR_VER) | $(LIB_TARGET_DIR) + ( cd $(LIB_TARGET_DIR); ln -s $(LIB_NAME).so.$(SO_MAJOR_VER) $(LIB_NAME).so ) +

##### build rules

@@ -81,7 +99,7 @@ clean: rm -f $(OBJECTS) rm -f $(DEPS) rm -f $(LIB_TARGET_DIR)/$(LIB_NAME).a - rm -f $(LIB_TARGET_DIR)/$(LIB_NAME).so + rm -f $(LIB_TARGET_DIR)/$(LIB_NAME).so*

#### install the necessary include files for the c-api library on linux install_inc: Index: libopencsd-0.8.0/decoder/build/linux/ref_trace_decode_lib/makefile =================================================================== --- libopencsd-0.8.0.orig/decoder/build/linux/ref_trace_decode_lib/makefile +++ libopencsd-0.8.0/decoder/build/linux/ref_trace_decode_lib/makefile @@ -114,13 +114,31 @@ OBJECTS=$(BUILD_DIR)/ocsd_code_follower. $(PTMOBJ) \ $(PKTPRNTOBJ)

-all: build_dir $(OBJECTS) - mkdir -p $(LIB_TARGET_DIR) +all: links + +links: $(LIB_TARGET_DIR)/$(LIB_NAME).so.$(SO_MAJOR_VER) $(LIB_TARGET_DIR)/$(LIB_NAME).so +.PHONY: links + +LIBS:= $(LIB_TARGET_DIR)/$(LIB_NAME).a $(LIB_TARGET_DIR)/$(LIB_NAME).so.$(SO_VER) + +$(LIB_TARGET_DIR): + mkdir $(LIB_TARGET_DIR) + +$(BUILD_DIR): + mkdir $(BUILD_DIR) + +$(LIB_TARGET_DIR)/$(LIB_NAME).a: $(OBJECTS) | $(BUILD_DIR) $(LIB_TARGET_DIR) $(LIB) $(LIB_FLAGS) $(LIB_TARGET_DIR)/$(LIB_NAME).a $(OBJECTS) - $(LINKER) $(LINKER_FLAGS) -o $(LIB_TARGET_DIR)/$(LIB_NAME).so -Wl,-soname,$(LIB_NAME).so $(OBJECTS)

-build_dir: - mkdir -p $(BUILD_DIR) +$(LIB_TARGET_DIR)/$(LIB_NAME).so.$(SO_VER): $(OBJECTS) | $(BUILD_DIR) $(LIB_TARGET_DIR) + $(LINKER) $(LINKER_FLAGS) -o $(LIB_TARGET_DIR)/$(LIB_NAME).so.$(SO_VER) -Wl,-soname,$(LIB_NAME).so.$(SO_MAJOR_VER) $(OBJECTS) + +$(LIB_TARGET_DIR)/$(LIB_NAME).so.$(SO_MAJOR_VER): $(LIBS) | $(LIB_TARGET_DIR) + ( cd $(LIB_TARGET_DIR); ln -s $(LIB_NAME).so.$(SO_VER) $(LIB_NAME).so.$(SO_MAJOR_VER) ) + +$(LIB_TARGET_DIR)/$(LIB_NAME).so: $(LIB_TARGET_DIR)/$(LIB_NAME).so.$(SO_MAJOR_VER) | $(LIB_TARGET_DIR) + ( cd $(LIB_TARGET_DIR); ln -s $(LIB_NAME).so.$(SO_MAJOR_VER) $(LIB_NAME).so ) +

##### build rules

@@ -140,4 +158,4 @@ clean: rm -f $(OBJECTS) rm -f $(DEPS) rm -f $(LIB_TARGET_DIR)/$(LIB_NAME).a - rm -f $(LIB_TARGET_DIR)/$(LIB_NAME).so + rm -f $(LIB_TARGET_DIR)/$(LIB_NAME).so*

I'll look at the flags next.

Wookey

Hi,

On 15 March 2018 at 14:36, Mathieu Poirier mathieu.poirier@linaro.org wrote:

On 14 March 2018 at 23:03, Wookey wookey@wookware.org wrote:

...

On 2018-03-13 16:19 +0000, Wookey wrote:

...

On 2018-03-08 05:18 +0000, Wookey wrote: (this is not working quite right, so I'll send this mail now, as relevant to the discussion or the first set of patches, and a finished patch later)

OK. Attached is the patch. As well as adding the SONAME setting, and installing, it sets out the actual dependencies for the links and libraries, using order-only dependencies for the directories. This is -j safe, unlike using mkdir -p, and avoids using a sentinel file for each dir (which would also work if you prefer that for some reason).

I've set it to use the release version, rather than 0.0.0, assuming that you want to go with that. (I guess I should move on to 0.8.1 before uploading this?)

What would be the rationale for moving to 0.8.1? Or perhaps I missed something...

Strictly speaking, the current version of OpenCSD is 0.8.1 (as of 26/01/2018), so when any patches for adjusting makefiles for packaging purposes are added in, then the release number will increment to 0.8.2. If any file changes in the project then that is at a minimum a patch release, across both supported platforms. For that I will have to increment the source and docs files that quote the latest version number too, before creating a release version.

Wookey: I'll be at connect next week so if we need to thrash out any final details then that may be a chance.

Mike.

...

No doubt this could be improved, but it is now at least reasonably correct and only builds things that need building.

I note that the changed sections for the two submakefiles in rctdl_c_api_lib and ref_trace_decode_lib only differ by one word (the rctdl_c_api_lib shared lib build has $(SO_LIB_DEPS) where ref_trace_decode_lib doesn't). I do wonder if there is room for some merging to only have one set of rules for all that?

Index: libopencsd-0.8.0/decoder/build/linux/makefile

--- libopencsd-0.8.0.orig/decoder/build/linux/makefile +++ libopencsd-0.8.0/decoder/build/linux/makefile @@ -104,6 +104,13 @@ export LIB_TARGET_DIR=$(OCSD_LIB_ROOT)/$ export LIB_TEST_TARGET_DIR=$(OCSD_TESTS)/lib/$(PLAT_DIR) export BIN_TEST_TARGET_DIR=$(OCSD_TESTS)/bin/$(PLAT_DIR)

+# verion +export SO_MAJOR_VER = 0 +SO_MINOR_VER = 8 +SO_PATCH_VER = 0 +export SO_VER = $(SO_MAJOR_VER).$(SO_MINOR_VER).$(SO_PATCH_VER)

########################################################### # build targets

@@ -113,8 +120,12 @@ libs: $(LIB_BASE_NAME)_lib $(LIB_CAPI_N

install: libs mkdir -p $(INSTALL_LIB_DIR) $(INSTALL_INCLUDE_DIR)

•   $(INSTALL) --mode=644 $(LIB_TARGET_DIR)/lib$(LIB_BASE_NAME).so $(INSTALL_LIB_DIR)/
  

•   $(INSTALL) --mode=644 $(LIB_TARGET_DIR)/lib$(LIB_CAPI_NAME).so $(INSTALL_LIB_DIR)/
  

•   cp -d $(LIB_TARGET_DIR)/lib$(LIB_BASE_NAME).so $(INSTALL_LIB_DIR)/
  

•   cp -d $(LIB_TARGET_DIR)/lib$(LIB_BASE_NAME).so.$(SO_MAJOR_VER) $(INSTALL_LIB_DIR)/
  

•   $(INSTALL) --mode=644 $(LIB_TARGET_DIR)/lib$(LIB_BASE_NAME).so.$(SO_VER) $(INSTALL_LIB_DIR)/
  

•   cp -d $(LIB_TARGET_DIR)/lib$(LIB_CAPI_NAME).so $(INSTALL_LIB_DIR)/
  

•   cp -d $(LIB_TARGET_DIR)/lib$(LIB_CAPI_NAME).so.$(SO_MAJOR_VER) $(INSTALL_LIB_DIR)/
  

•   $(INSTALL) --mode=644 $(LIB_TARGET_DIR)/lib$(LIB_CAPI_NAME).so.$(SO_VER) $(INSTALL_LIB_DIR)/
    $(INSTALL) --mode=644 $(LIB_TARGET_DIR)/lib$(LIB_BASE_NAME).a $(INSTALL_LIB_DIR)/
    $(INSTALL) --mode=644 $(LIB_TARGET_DIR)/lib$(LIB_CAPI_NAME).a $(INSTALL_LIB_DIR)/
    cd $(OCSD_ROOT)/build/linux/rctdl_c_api_lib && make install_inc
  

Index: libopencsd-0.8.0/decoder/build/linux/rctdl_c_api_lib/makefile

--- libopencsd-0.8.0.orig/decoder/build/linux/rctdl_c_api_lib/makefile +++ libopencsd-0.8.0/decoder/build/linux/rctdl_c_api_lib/makefile @@ -55,13 +55,31 @@ OBJECTS=$(BUILD_DIR)/ocsd_c_api.o \ INST_INC_SRC=$(OCSD_INCLUDE)/$(LIB_UAPI_INC_DIR) INST_INC_DST=$(INSTALL_INCLUDE_DIR)/$(LIB_UAPI_INC_DIR)

-all: build_dir $(OBJECTS)

•   mkdir -p $(LIB_TARGET_DIR)
  

+all: links

+links: $(LIB_TARGET_DIR)/$(LIB_NAME).so.$(SO_MAJOR_VER) $(LIB_TARGET_DIR)/$(LIB_NAME).so +.PHONY: links

+LIBS:= $(LIB_TARGET_DIR)/$(LIB_NAME).a $(LIB_TARGET_DIR)/$(LIB_NAME).so.$(SO_VER)

+$(LIB_TARGET_DIR):

•   mkdir $(LIB_TARGET_DIR)
  

+$(BUILD_DIR):

•   mkdir $(BUILD_DIR)
  

+$(LIB_TARGET_DIR)/$(LIB_NAME).a: $(OBJECTS) | $(BUILD_DIR) $(LIB_TARGET_DIR) $(LIB) $(LIB_FLAGS) $(LIB_TARGET_DIR)/$(LIB_NAME).a $(OBJECTS)

•   $(LINKER) $(LINKER_FLAGS) -o $(LIB_TARGET_DIR)/$(LIB_NAME).so -Wl,-soname,$(LIB_NAME).so $(OBJECTS) $(SO_LIB_DEPS)
  
  

-build_dir:

•   mkdir -p $(BUILD_DIR)
  

+$(LIB_TARGET_DIR)/$(LIB_NAME).so.$(SO_VER): $(OBJECTS) | $(BUILD_DIR) $(LIB_TARGET_DIR)

•   $(LINKER) $(LINKER_FLAGS) -o $(LIB_TARGET_DIR)/$(LIB_NAME).so.$(SO_VER) -Wl,-soname,$(LIB_NAME).so.$(SO_MAJOR_VER) $(OBJECTS) $(SO_LIB_DEPS)
  

+$(LIB_TARGET_DIR)/$(LIB_NAME).so.$(SO_MAJOR_VER): $(LIBS) | $(LIB_TARGET_DIR)

•   ( cd $(LIB_TARGET_DIR); ln -s $(LIB_NAME).so.$(SO_VER) $(LIB_NAME).so.$(SO_MAJOR_VER) )
  

+$(LIB_TARGET_DIR)/$(LIB_NAME).so: $(LIB_TARGET_DIR)/$(LIB_NAME).so.$(SO_MAJOR_VER) | $(LIB_TARGET_DIR)

•   ( cd $(LIB_TARGET_DIR); ln -s $(LIB_NAME).so.$(SO_MAJOR_VER) $(LIB_NAME).so )
  

##### build rules

@@ -81,7 +99,7 @@ clean: rm -f $(OBJECTS) rm -f $(DEPS) rm -f $(LIB_TARGET_DIR)/$(LIB_NAME).a

•   rm -f $(LIB_TARGET_DIR)/$(LIB_NAME).so
  

•   rm -f $(LIB_TARGET_DIR)/$(LIB_NAME).so*
  
  

#### install the necessary include files for the c-api library on linux install_inc: Index: libopencsd-0.8.0/decoder/build/linux/ref_trace_decode_lib/makefile =================================================================== --- libopencsd-0.8.0.orig/decoder/build/linux/ref_trace_decode_lib/makefile +++ libopencsd-0.8.0/decoder/build/linux/ref_trace_decode_lib/makefile @@ -114,13 +114,31 @@ OBJECTS=$(BUILD_DIR)/ocsd_code_follower. $(PTMOBJ) \ $(PKTPRNTOBJ)

-all: build_dir $(OBJECTS)

•   mkdir -p $(LIB_TARGET_DIR)
  

+all: links

+links: $(LIB_TARGET_DIR)/$(LIB_NAME).so.$(SO_MAJOR_VER) $(LIB_TARGET_DIR)/$(LIB_NAME).so +.PHONY: links

+LIBS:= $(LIB_TARGET_DIR)/$(LIB_NAME).a $(LIB_TARGET_DIR)/$(LIB_NAME).so.$(SO_VER)

+$(LIB_TARGET_DIR):

•   mkdir $(LIB_TARGET_DIR)
  

+$(BUILD_DIR):

•   mkdir $(BUILD_DIR)
  

+$(LIB_TARGET_DIR)/$(LIB_NAME).a: $(OBJECTS) | $(BUILD_DIR) $(LIB_TARGET_DIR) $(LIB) $(LIB_FLAGS) $(LIB_TARGET_DIR)/$(LIB_NAME).a $(OBJECTS)

•   $(LINKER) $(LINKER_FLAGS) -o $(LIB_TARGET_DIR)/$(LIB_NAME).so -Wl,-soname,$(LIB_NAME).so $(OBJECTS)
  
  

-build_dir:

•   mkdir -p $(BUILD_DIR)
  

+$(LIB_TARGET_DIR)/$(LIB_NAME).so.$(SO_VER): $(OBJECTS) | $(BUILD_DIR) $(LIB_TARGET_DIR)

•   $(LINKER) $(LINKER_FLAGS) -o $(LIB_TARGET_DIR)/$(LIB_NAME).so.$(SO_VER) -Wl,-soname,$(LIB_NAME).so.$(SO_MAJOR_VER) $(OBJECTS)
  

+$(LIB_TARGET_DIR)/$(LIB_NAME).so.$(SO_MAJOR_VER): $(LIBS) | $(LIB_TARGET_DIR)

•   ( cd $(LIB_TARGET_DIR); ln -s $(LIB_NAME).so.$(SO_VER) $(LIB_NAME).so.$(SO_MAJOR_VER) )
  

+$(LIB_TARGET_DIR)/$(LIB_NAME).so: $(LIB_TARGET_DIR)/$(LIB_NAME).so.$(SO_MAJOR_VER) | $(LIB_TARGET_DIR)

•   ( cd $(LIB_TARGET_DIR); ln -s $(LIB_NAME).so.$(SO_MAJOR_VER) $(LIB_NAME).so )
  

##### build rules

@@ -140,4 +158,4 @@ clean: rm -f $(OBJECTS) rm -f $(DEPS) rm -f $(LIB_TARGET_DIR)/$(LIB_NAME).a

•   rm -f $(LIB_TARGET_DIR)/$(LIB_NAME).so
  

•   rm -f $(LIB_TARGET_DIR)/$(LIB_NAME).so*
  
  
  
  

I'll look at the flags next.

Wookey

Principal hats: Linaro, Debian, Wookware, ARM http://wookware.org/

---

CoreSight mailing list CoreSight@lists.linaro.org https://lists.linaro.org/mailman/listinfo/coresight

---

CoreSight mailing list CoreSight@lists.linaro.org https://lists.linaro.org/mailman/listinfo/coresight

On 2018-03-15 15:42 +0000, Wookey wrote:

On 2018-03-15 15:21 +0000, Mike Leach wrote:

...

Strictly speaking, the current version of OpenCSD is 0.8.1 (as of 26/01/2018), so when any patches for adjusting makefiles for packaging purposes are added in, then the release number will increment to 0.8.2.

Right. I'll update to 0.8.1 so at least my patches are uptodate. I started with 0.8.0 because something pointed me at it when I started, but there is no good reason for that SFAIK.

So you are OK with the idea that release versioning from here in will follow the GNU library compitibility rules, and thus we can use the release number for the library versioning, right? I thnk that's sensible.

OK. I'm working from 0.8.1 now.

I put in some slightly scary make/shell to fish the version numbers out of the version header file, so that it doesn't need maintaining in two places.

The whole updated soname patch is attached, but this is the changed bit: Index: libopencsd-0.8.1/decoder/build/linux/makefile =================================================================== --- libopencsd-0.8.1.orig/decoder/build/linux/makefile +++ libopencsd-0.8.1/decoder/build/linux/makefile @@ -104,6 +104,14 @@ export LIB_TARGET_DIR=$(OCSD_LIB_ROOT)/$ export LIB_TEST_TARGET_DIR=$(OCSD_TESTS)/lib/$(PLAT_DIR) export BIN_TEST_TARGET_DIR=$(OCSD_TESTS)/bin/$(PLAT_DIR)

+# Fish version out of header file (converting from hex) +getver:=printf "%d" $$(awk '/#define VARNAME/ { print $$3 }' $(OCSD_ROOT)/include/ocsd_if_version.h) +export SO_MAJOR_VER := $(shell $(subst VARNAME,OCSD_VER_MAJOR,$(getver))) +SO_MINOR_VER := $(shell $(subst VARNAME,OCSD_VER_MINOR,$(getver))) +SO_PATCH_VER := $(shell $(subst VARNAME,OCSD_VER_PATCH,$(getver))) +export SO_VER := $(SO_MAJOR_VER).$(SO_MINOR_VER).$(SO_PATCH_VER) + + ########################################################### # build targets

This is trickier than one would like because the numbers in the header file are C defines in hex (0x0), and need converting to unadorned decimal.

There are probably various ways of doing this. Above is the neatest I could come up with, but it's still a bit cryptic due to nestedness, escaping, and putting it into a function to avoid repeating it 3 times (and it took me a while to get right). So I'll explain.

awk '/#define OCSD_VER_MAJOR/ { print $3 }' $(OCSD_ROOT)/include/ocsd_if_version.h pulls the '0x0' number out of the file. printf "%d" 0x0 turns it into decimal. I can't see a way to pipe this, so two shell substitutions are used: awk reult into print command and print command into SO_ variable.

When the awk bit is put into a subshell the # needs escaping otherwise rest of line turns into a comment (and terminating bracket gets lost) Both the subshell call '$(' and the '$3' need the dollars escaping because this is make.

It's a bit ugly to repeat 3 times for different defines, so I put it in a 'getver' fn, then used $(subst to replace VARNAME for each define.

Wookey

On 2018-04-05 07:50 +0000, Mike Leach wrote:

On 2018-03-22 02:59 +0000, Wookey wrote:

...

The .d problem:

...

My understanding is that because of the -MMD option the .d files should be built alongside the .o files as part of the compilation. Why might that not happen?

Not sure - that is my understanding too.

Not managed to reproduce this yet - but will revisit as soon as I get back from vacation (Mon 9th).

I had a proper peer at the entrails and eventually worked out how to reproduce this. It's the presence or otherwise of the decoder/build/linux/ref_trace_decode_lib/builddir build directory.

That needs to exist _before_ the first g++ command otherwise it fails, confusingly talking about the .d file it can't read, when really the problem is that it didn't _create_ it.

The fix is simple enough: add this dependency. This was an ommission from the rearangement of the dependcies in the sonames patch.

And it wasn't revealed (and thus much more obvious) because those two sub-builddirs weren't getting cleaned, so once they existed that source tree would always build.

This patch fixes both issues. Index: libopencsd-0.8.1/decoder/build/linux/rctdl_c_api_lib/makefile =================================================================== --- libopencsd-0.8.1.orig/decoder/build/linux/rctdl_c_api_lib/makefile +++ libopencsd-0.8.1/decoder/build/linux/rctdl_c_api_lib/makefile @@ -85,7 +85,7 @@ DEPS := $(OBJECTS:%.o=%.d) -include $(DEPS)

## object compile -$(BUILD_DIR)/%.o : %.cpp +$(BUILD_DIR)/%.o : %.cpp | $(BUILD_DIR) $(CXX) $(CXXFLAGS) $(CXX_INCLUDES) -MMD $< -o $@

@@ -96,6 +96,7 @@ clean: rm -f $(DEPS) rm -f $(LIB_TARGET_DIR)/$(LIB_NAME).a rm -f $(LIB_TARGET_DIR)/$(LIB_NAME).so* + -rmdir $(BUILD_DIR)

#### install the necessary include files for the c-api library on linux install_inc: Index: libopencsd-0.8.1/decoder/build/linux/ref_trace_decode_lib/makefile =================================================================== --- libopencsd-0.8.1.orig/decoder/build/linux/ref_trace_decode_lib/makefile +++ libopencsd-0.8.1/decoder/build/linux/ref_trace_decode_lib/makefile @@ -144,7 +144,7 @@ DEPS := $(OBJECTS:%.o=%.d) -include $(DEPS)

## object compile -$(BUILD_DIR)/%.o : %.cpp +$(BUILD_DIR)/%.o : %.cpp | $(BUILD_DIR) $(CXX) $(CXXFLAGS) $(CXX_INCLUDES) -MMD $< -o $@

@@ -155,3 +155,4 @@ clean: rm -f $(DEPS) rm -f $(LIB_TARGET_DIR)/$(LIB_NAME).a rm -f $(LIB_TARGET_DIR)/$(LIB_NAME).so* + -rmdir $(BUILD_DIR)

Wookey

Hi Wookey,

I've attached an updated patchset - largely based on your patches, adjusted to fix the above issues, and includes a development makefile.dev that retains the original arch/dbg|rel directory structure, but allows the master makefile to have the single builddir needed for the Debian distro. Other than that it should be largely what you developed over the past couple of weeks. Should apply to 0.8.1

The final upstream patchset will contain additional stuff (some expanded user docs and a couple of source fixes that have been made since 0.8.1), but if you could check that this set doesn't break the packaging build, then we will be almost there.

Thanks

Mike

On 10 April 2018 at 18:18, Mike Leach mike.leach@linaro.org wrote:

Hi Wookey,

Not added the above yet as there doesn't appear to be an actual attached .patch file, and it does appear that the patch may remove the reliance on including DEPS - which I think is required to ensure that we only build changed files.

However, the previous patches break the build in two fundamental ways.

1. running make from the build\linux directory results in awk failing

and thus the entire build failing

awk: cannot open /disk-2/mleach/opencsd-master/decoder/build/linux/decoder/include/ocsd_if_version.h (No such file or directory) awk: cannot open /disk-2/mleach/opencsd-master/decoder/build/linux/decoder/include/ocsd_if_version.h (No such file or directory) awk: cannot open /disk-2/mleach/opencsd-master/decoder/build/linux/decoder/include/ocsd_if_version.h (No such file or directory) cd /disk-2/mleach/opencsd-master/decoder/build/linux/decoder/build/linux/ref_trace_decode_lib && make clean /bin/sh: 1: cd: can't cd to /disk-2/mleach/opencsd-master/decoder/build/linux/decoder/build/linux/ref_trace_decode_lib

editing a single source file and rebuilding (without clean so that _only_ the changed files are altered) make results in an error:-

( cd /disk-2/mleach/opencsd-master/decoder/lib/builddir; ln -s libopencsd.so.0.8.3 libopencsd.so.0 ) ln: failed to create symbolic link 'libopencsd.so.0': File exists makefile:133: recipe for target '/disk-2/mleach/opencsd-master/decoder/lib/builddir/libopencsd.so.0' failed make[1]: *** [/disk-2/mleach/opencsd-master/decoder/lib/builddir/libopencsd.so.0] Error 1

The whole point of the dependencies etc.is to allow incremental build and development. Thus this has to be fixed.

These really need to be fixed before I can release a patchset to upstream - so any suggestions welcome.

Regards

Mike

On 8 April 2018 at 14:46, Wookey wookey@wookware.org wrote:

...

On 2018-04-05 07:50 +0000, Mike Leach wrote:

...

On 2018-03-22 02:59 +0000, Wookey wrote:

...

...

The .d problem:

...

...

My understanding is that because of the -MMD option the .d files should be built alongside the .o files as part of the compilation. Why might that not happen?

Not sure - that is my understanding too.

Not managed to reproduce this yet - but will revisit as soon as I get back from vacation (Mon 9th).

I had a proper peer at the entrails and eventually worked out how to reproduce this. It's the presence or otherwise of the decoder/build/linux/ref_trace_decode_lib/builddir build directory.

That needs to exist _before_ the first g++ command otherwise it fails, confusingly talking about the .d file it can't read, when really the problem is that it didn't _create_ it.

The fix is simple enough: add this dependency. This was an ommission from the rearangement of the dependcies in the sonames patch.

And it wasn't revealed (and thus much more obvious) because those two sub-builddirs weren't getting cleaned, so once they existed that source tree would always build.

This patch fixes both issues. Index: libopencsd-0.8.1/decoder/build/linux/rctdl_c_api_lib/makefile =================================================================== --- libopencsd-0.8.1.orig/decoder/build/linux/rctdl_c_api_lib/makefile +++ libopencsd-0.8.1/decoder/build/linux/rctdl_c_api_lib/makefile @@ -85,7 +85,7 @@ DEPS := $(OBJECTS:%.o=%.d) -include $(DEPS)

## object compile -$(BUILD_DIR)/%.o : %.cpp +$(BUILD_DIR)/%.o : %.cpp | $(BUILD_DIR) $(CXX) $(CXXFLAGS) $(CXX_INCLUDES) -MMD $< -o $@

@@ -96,6 +96,7 @@ clean: rm -f $(DEPS) rm -f $(LIB_TARGET_DIR)/$(LIB_NAME).a rm -f $(LIB_TARGET_DIR)/$(LIB_NAME).so*

•   -rmdir $(BUILD_DIR)
  
  

#### install the necessary include files for the c-api library on linux install_inc: Index: libopencsd-0.8.1/decoder/build/linux/ref_trace_decode_lib/makefile =================================================================== --- libopencsd-0.8.1.orig/decoder/build/linux/ref_trace_decode_lib/makefile +++ libopencsd-0.8.1/decoder/build/linux/ref_trace_decode_lib/makefile @@ -144,7 +144,7 @@ DEPS := $(OBJECTS:%.o=%.d) -include $(DEPS)

## object compile -$(BUILD_DIR)/%.o : %.cpp +$(BUILD_DIR)/%.o : %.cpp | $(BUILD_DIR) $(CXX) $(CXXFLAGS) $(CXX_INCLUDES) -MMD $< -o $@

@@ -155,3 +155,4 @@ clean: rm -f $(DEPS) rm -f $(LIB_TARGET_DIR)/$(LIB_NAME).a rm -f $(LIB_TARGET_DIR)/$(LIB_NAME).so*

•   -rmdir $(BUILD_DIR)
  
  
  
  
  

Wookey

Principal hats: Linaro, Debian, Wookware, ARM http://wookware.org/

---

CoreSight mailing list CoreSight@lists.linaro.org https://lists.linaro.org/mailman/listinfo/coresight

-- Mike Leach Principal Engineer, ARM Ltd. Blackburn Design Centre. UK

Hi Wookey,

On 12 April 2018 at 19:04, Wookey wookey@wookware.org wrote:

On 2018-04-11 23:13 +0100, Mike Leach wrote:

...

Hi Wookey,

I've attached an updated patchset - largely based on your patches, adjusted to fix the above issues, and includes a development makefile.dev that retains the original arch/dbg|rel directory structure, but allows the master makefile to have the single builddir needed for the Debian distro. Other than that it should be largely what you developed over the past couple of weeks.

Thanks muhcly for all that. Looks good.

...

Should apply to 0.8.1

It does.

...

The final upstream patchset will contain additional stuff (some expanded user docs and a couple of source fixes that have been made since 0.8.1), but if you could check that this set doesn't break the packaging build, then we will be almost there.

OK, I need a one-line patch to make it work, and there are a few other minor things which don't break the build but could be better:

Vital fix: Index: libopencsd-0.8.1/decoder/build/linux/makefile =================================================================== --- libopencsd-0.8.1.orig/decoder/build/linux/makefile +++ libopencsd-0.8.1/decoder/build/linux/makefile @@ -35,7 +35,7 @@

# Set project root - relative to build directory ifeq ($(OCSD_ROOT),) -OCSD_ROOT := $(shell pwd | sed 's,/build/linux.*,,') +OCSD_ROOT := $(CURDIR)/decoder export OCSD_ROOT endif

without that all the paths are one directory too high: cd /home/wookey/packages/libopencsd/debian/libopencsd-0.8.3pre.test/build/linux/ref_trace_decode_lib && make clean (instead of: cd /home/wookey/packages/libopencsd/debian/libopencsd-0.8.3pre.test/decoder/build/linux/ref_trace_decode_lib && make clean (and nothing builds)

I don't quite understand why what you have works upstream, but it must be something to do with using relative paths, and your platform dirs having two levels? There is probably a way to do this that works with both one and two-level platform dirnames and both relative and absolute paths. It just feels like "$(shell pwd | sed 's,/build/linux.*,,')" is making unwarranted assumptions about the layout. Is "$(CURDIR)/decoder" ever wrong for your purposes? Is there a reason you need to use relative paths?

I possibly never actually sent you this snippet of the patchset before

• sorry about that.

No - you did add it to one of the previous patches - but it broke the build I have been using since the start of the project. i.e. cd <opencsd-checkout-dir>/decoder/build/linux make

See point 1) a couple of my mails ago for a string of errors such as....

awk: cannot open /disk-2/mleach/opencsd-master/decoder/build/linux/decoder/include/ocsd_if_version.h (No such file or directory)

using $(CURDIR) assumes that build is being run from the root of the project - if I run as normal from the build directory $(CURDIR) is opencsd-checkout-dir>/decoder/build/linux, and that is uses as a basis for the rest of the make process, thus awk looking for a none-existent decode sub-dir in build/linux.

If you need to run from the root of the project it is possible using make -C decoder/build/linux which is pretty much what I do to build perf tools from the root of the linux kernel source.

Not sure from the description above precisely how you are launching the makefile, or why it breaks using the existing makefile.

Also the doc build is not getting cleaned: -clean: clean_libs clean_tests +clean: clean_libs clean_tests clean_docs

Even if you don't do the doc build by default, cleaning it doesn't do any harm, but I guess you see a warning and that was unpleasing so you removed it? If so I guess the clean way to do this is have a BUILD_DOCS flag to pass in which does and build and clean if set, but neither if not?

Actually, I am happy with not building docs by default, and hence not cleaning by default - and doing both as a specific make task, just as I would with install. I rarely build docs during development - just when looking at a final release.

If having a build_docs flag works better for the package build, that's fine too.

everything else looks sensible, except I'd do: ( cd $(LIB_TARGET_DIR); ln -sf $(LIB_NAME).so.$(SO_VER) $(LIB_NAME).so.$(SO_MAJOR_VER) )

rather than ( cd $(LIB_TARGET_DIR); rm -f $(LIB_NAME).so.$(SO_MAJOR_VER); ln -s $(LIB_NAME).so.$(SO_VER) $(LIB_NAME).so.$(SO_MAJOR_VER) )

(smae pattern on 3 lines in: decoder/build/linux/rctdl_c_api_lib/makefile decoder/build/linux/ref_trace_decode_lib/makefile

(I can see why you need that for non-clean rebuilds).

I added in the 'rm ...' command to prevent an error killing partial builds during the development process.

And finally there is trailing whitespace on: decoder/tests/build/linux/c_cpi_pkt_print_test/makefile:82 decoder/tests/build/linux/echo_test_dcd_lib/makefile:74 decoder/tests/build/linux/trc_pkt_lister/makefile:36

OK, I'll look to kill these prior to releasing a final patchset.

I need to dash now so I haven't got time to make you better suggested patches for 'BUILD_DOCS' of the OCSD_ROOT thing, but can have a look next week if you don;t beat me to it.

Wookey

Principal hats: Linaro, Debian, Wookware, ARM http://wookware.org/

Thanks

Mike

On 2018-03-19 03:38 +0000, Wookey wrote:

On 2018-03-16 16:54 +0000, Mike Leach wrote: I'll do the 'mechanism to pass in flags' thing next.

OK. This is the simplest patch that allow passing-in of external flags (by defining the flags on the make command line):

Index: libopencsd-0.8.1/decoder/build/linux/makefile =================================================================== --- libopencsd-0.8.1.orig/decoder/build/linux/makefile +++ libopencsd-0.8.1/decoder/build/linux/makefile @@ -76,9 +76,12 @@ INSTALL_LIB_DIR=$(PREFIX)/lib/$(DEB_HOST export INSTALL_INCLUDE_DIR=$(PREFIX)/include

# compile flags -MASTER_CC_FLAGS := -c -Wall -DLINUX -MASTER_CXX_FLAGS := -c -Wall -DLINUX -std=c++11 -MASTER_LINKER_FLAGS := -Wl,-z,defs +CFLAGS += -c -Wall -DLINUX +CXXFLAGS += -c -Wall -DLINUX -std=c++11 +LDFLAGS += -Wl,-z,defs +MASTER_CC_FLAGS := $(CFLAGS) +MASTER_CXX_FLAGS := $(CXXFLAGS) +MASTER_LINKER_FLAGS := $(LDFLAGS) MASTER_LIB_FLAGS := rcs

# debug variant

The current flags used in various places are: CC_FLAGS (CFLAGS) := -c -Wall -DLINUX CXX_FLAGS (CXXFLAGS):= -c -Wall -DLINUX -std=c++11 LINKER_FLAGS (LDFLAGS) := -Wl,-z,defs LIB_FLAGS (ARFLAGS) := rcs

rctdl_c_api_lib: CXXFLAGS += -fpic -Wno-switch LDFLAGS += -shared

ref_trace_decode_lib: CXXFLAGS += -fpic -Wno-switch LDFLAGS += -shared

echo_test_dcd_lib: CFLAGS += -fpic -Wno-switch ARFLAGS

snapshot_parser_lib: CFLAGS += -fpic -Wno-switch -Wno-deprecated-declarations -Wno-unused-variable -Wno-reorder ARFLAGS

c_api_pkt_print_test: CFLAGS += -Wno-switch LDFLAGS += -Wl,--start-group $(LIBS) -Wl,--end-group

trc_pkt_lister: CXXFLAGS += -Wno-switch LDFLAGS

So that shows that -Wno-switch should be in the defaults -fpic is not used in two places, but in fact this is now the compiler default, at least in debian, and enabling it everywhere seems to be fine. Was there some reason why it was turned off for c_api_pkt_print_test and trc_pkt_lister?

LDFLAGS is used without -shared in two places, and those seem to make sense, so putting -shared on the 'link .so' lines, the special -Wl,--start-group in the one place it's needed and just using LDFLAGS default everywhere works fine.

So there is no reason not to use standard flag variable names thoughout and skip the MASTER->local copying. I've done this, using these defaults: CFLAGS := $(CPPFLAGS) -c -Wall -DLINUX -Wno-switch -fpic CXXFLAGS := $(CPPFLAGS) -c -Wall -DLINUX -Wno-switch -fpic -std=c++11 LDFLAGS := -Wl,-z,defs ARFLAGS := rcs

The attached patch makes these changes, allowing extra options to be passed in from outside.

One other thing. Should we enable --as-needed in the link? I've done this for the debian package but if it's a good idea it should go upstream.

Wookey