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