On 2018-03-13 11:35 +0000, Mike Leach wrote:
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.
Sorry I messed with a couple of them for re-ordering reasons. I might have broken things. I've just updated the doc-build one last night because there was a bug.
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:
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.
I am about 3 hours into a long post on the soname issue/patch. This is discussed there. But yes we (you) can chose to use the release version for the soname so long as you follow suitable rules for when/how it changes. And that is generally a good idea.
As soon as I get the damn build to work right I'll send 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 @@ -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/htmlHappy with this too - for some reason the indexing was out compared to my system - did not apply.
That rm -rf $(OCSD_ROOT)/docs/html in the clean_install target doesn't actually work if the docs are built (because clean_install) isn't called by default.
also there was a misplaced bracket: $(OCSD_ROOT/docs) -> $(OCSD_ROOT)/docs
This patch is more reliable/correct: 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,12 +155,19 @@ 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 # -clean: clean_libs clean_tests +clean: clean_libs clean_tests clean_docs
-.PHONY: clean_libs clean_tests +.PHONY: clean_libs clean_tests clean_docs clean_install
clean_libs: cd $(OCSD_ROOT)/build/linux/ref_trace_decode_lib && make clean @@ -172,6 +179,9 @@ clean_tests: cd $(OCSD_ROOT)/tests/build/linux/trc_pkt_lister && make clean cd $(OCSD_ROOT)/tests/build/linux/c_api_pkt_print_test && make clean
+clean_docs: + rm -rf $(OCSD_ROOT)/docs/html + clean_install: rm -f $(INSTALL_LIB_DIR)/lib$(LIB_BASE_NAME).{so,a} rm -f $(INSTALL_LIB_DIR)/lib$(LIB_CAPI_NAME).{so,a}
- 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,
OK. my mistake. Apologies.
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.
It requires arch-specific boilerplate like the above in makefiles, which shouldn't be necessary if your toolchain is sane. I contend that that is the wrong place for it. But ultimately it's up to you.
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.
- 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.
Well, yes, but why would you expect that sequence to do anything sensible? If you don't clean before a different build you are likely to get incomplete results. make clean; make make clean; make DEBUG=1 are both sensible things to do and will produce non-debug and debug variants.
Missing out one of the cleans and relying on multiple build-dirs to make it come out right seems an odd design.
It only re-links the second time because the makefile design always re-does linking when called, even if the files are already up to date. (see my forthcoming soname mail where I go into some detail about this, and offer a more correct design).
- 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.
That sort of thing should be done at a higher level than in the makefile for the library. If you want to batch-build lots of copies for different targets then do so with lots of copies of the library. The library makefile should just build the library for the target requested.
This sort of thinking is (IMHO) a side-effect of ARMs persistence in never native-building anything on their own architecture if they can help it...
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
OK, such a mechanism should work for the debian build (although so does replacing this whole section with export PLAT_DIR=builddir which should give you pause for thought :-)
so although this patch does apply - I'd really rather not!
Well I can't make you, but I really think this machinery is at the wrong level. A library makefile should just build the library for one target. Then simple builds are simple, and more complicated multiple arch builds can just be done with copies from 'outside'.
I'll send the soname mail now, even though I want to refine the patch, as I've cross-referecned it above.
Wookey