From 934341104ba057927216f5a138308ef919f7d18c Mon Sep 17 00:00:00 2001 From: Jonathan Ben-Avraham Date: Thu, 12 Nov 2020 15:56:48 +0200 Subject: [PATCH 1/6] Added Makefile ergonomic features These features are added in order to: 1. Warn the user when root privileges are necessary 2. Warn the user when root UID is probably not intended 3. Bring the non-default 'example' target to the user's attention more clearly 4. Build and install the util applications whenever the example app is built or installed, because the most likely use of the example app is to run the tests, which require the util applications 5. Provide a new target 'util' for building the util applications 6. Provide a new target 'install-example' for installing the example app 7. Provide a new target 'mrproper' to uninstall and clean everything, in order to avoid missing files that need to be removed before regression testing, but might be forgotten, for example, the yang specs --- Makefile.in | 60 ++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 48 insertions(+), 12 deletions(-) diff --git a/Makefile.in b/Makefile.in index f40a083c..a0c2a369 100644 --- a/Makefile.in +++ b/Makefile.in @@ -58,12 +58,28 @@ SUBDIRS1 = include lib SUBDIRS2 = apps etc yang # without include lib for circular dependency SUBDIRS= $(SUBDIRS1) $(SUBDIRS2) -.PHONY: doc example all clean depend $(SUBDIRS) install loc TAGS .config.status docker test +.PHONY: doc example install-example clean-example all clean depend $(SUBDIRS) \ + install loc TAGS .config.status docker test util checkroot mrproper \ + checkinstall warnroot -all: $(SUBDIRS2) +all: $(SUBDIRS2) warnroot + @echo "\e[32mAfter 'make install' as euid root, build example app and test utils: 'make example'\e[0m" + +checkroot: + @if [ "$${DESTDIR}" = "" -a $$(id -u) != "0" ]; \ + then echo "\e[31mThis target must be made as euid root\e[0m"; exit 1; fi; + +warnroot: + @if [ $$(id -u) = 0 ]; \ + then echo "\e[36mWarning: You built this target as uid root\e[0m"; exit 0; fi; + +checkinstall: + @if [ ! -f $(prefix)/include/clixon/clixon.h ]; then \ + echo "\e[31mclixon must be installed first to build this target. "\ + "Run 'make'. Then run 'make install' as root.\e[0m"; exit 1; fi; } # May cause circular include->include,lib -$(SUBDIRS2): include lib # Cannot build app before lib (for parallel make -j) +$(SUBDIRS2): $(SUBDIRS1) # Cannot build app before lib (for parallel make -j) (cd $@ && $(MAKE) $(MFLAGS) all) $(SUBDIRS1): @@ -73,7 +89,8 @@ depend: for i in $(SUBDIRS) doc example docker; \ do (cd $$i && $(MAKE) $(MFLAGS) depend); done -install: +# Needs root permissions on most systems by default +install: checkroot for i in $(SUBDIRS) doc; \ do (cd $$i; $(MAKE) $(MFLAGS) $@)||exit 1; done; $(MAKE) $(MFLAGS) install-include @@ -81,30 +98,47 @@ install: install-include: for i in $(SUBDIRS) doc; \ do (cd $$i && $(MAKE) $(MFLAGS) $@)||exit 1; done; - echo "To install example app: cd example; make; make install" + @echo "\e[32mTo install example app and test utils: make install-example\e[0m" -uninstall: - for i in $(SUBDIRS) doc example docker; \ +uninstall: checkroot + for i in $(SUBDIRS) doc example util docker; \ do (cd $$i && $(MAKE) $(MFLAGS) $@)||exit 1; done; -doc: +doc: warnroot cd $@; $(MAKE) $(MFLAGS) $@ +util: + cd $@; $(MAKE) $(MFLAGS) + +clean-example: + for i in example util; \ + do (cd $$i && $(MAKE) $(MFLAGS) clean) || exit 1; done; + +install-example: checkroot + for i in example util; \ + do (cd $$i && $(MAKE) $(MFLAGS) install) || exit 1; done; + +uninstall-example: checkroot + for i in example util; \ + do (cd $$i && $(MAKE) $(MFLAGS) uninstall) || exit 1; done; + config.status: configure $(SHELL) config.status --recheck -configure: configure.ac +configure: configure.ac cd $(srcdir) && autoconf clean: - for i in $(SUBDIRS) doc example docker; \ + for i in $(SUBDIRS) doc example util docker; \ do (cd $$i && $(MAKE) $(MFLAGS) $@); done; +mrproper: uninstall uninstall-example clean clean-example + distclean: rm -f Makefile TAGS config.status config.log *~ .depend rm -rf autom4te.cache build-root/rpmbuild rm -f build-root/*.tar.xz build-root/*.rpm extras/rpm/Makefile - for i in $(SUBDIRS) doc example docker; \ + for i in $(SUBDIRS) doc example util docker; \ do (cd $$i && $(MAKE) $(MFLAGS) $@); done export BR=$(CURDIR)/build-root @@ -146,8 +180,10 @@ pkg-rpm: dist pkg-srpm: dist make -C extras/rpm srpm -example: +# To make the example you need to run the "install-include" target first +example: checkinstall util warnroot (cd $@ && $(MAKE) $(MFLAGS) all) + @echo "\e[36mRemember to run 'make install-example' as euid root\e[0m" # Run a clixon test container. # Alt: cd test; ./all.sh From b1c742ff41d09787120a16582ed29e372890e38a Mon Sep 17 00:00:00 2001 From: Jonathan Ben-Avraham Date: Thu, 12 Nov 2020 10:28:53 +0200 Subject: [PATCH 2/6] Test script documentation fixup This commit provides a scriptable headline for the octal dump output. When the received output of a test is empty or only whitespace, this commit makes it clear that the octal dump putput is not the expected output. --- test/lib.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/test/lib.sh b/test/lib.sh index e1ec4138..6f95092f 100755 --- a/test/lib.sh +++ b/test/lib.sh @@ -201,6 +201,7 @@ err(){ echo "Received: $2" fi echo -e "\e[0m" + echo "Diff between Expected and Received:" echo "$ret"| od -t c > $dir/clixon-ret echo "$expect"| od -t c > $dir/clixon-expect diff $dir/clixon-expect $dir/clixon-ret From 8701303e7ff6b41b888cb45f342ffc90cffbde77 Mon Sep 17 00:00:00 2001 From: Jonathan Ben-Avraham Date: Fri, 13 Nov 2020 13:02:23 +0200 Subject: [PATCH 3/6] Added clean-util, install-util and uninstall-util Added more top-level Makefile targets for convenience and to make the new users more aware of these targets. --- Makefile.in | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/Makefile.in b/Makefile.in index a0c2a369..f1f86fcd 100644 --- a/Makefile.in +++ b/Makefile.in @@ -60,7 +60,7 @@ SUBDIRS= $(SUBDIRS1) $(SUBDIRS2) .PHONY: doc example install-example clean-example all clean depend $(SUBDIRS) \ install loc TAGS .config.status docker test util checkroot mrproper \ - checkinstall warnroot + checkinstall warnroot install-util clean-util all: $(SUBDIRS2) warnroot @echo "\e[32mAfter 'make install' as euid root, build example app and test utils: 'make example'\e[0m" @@ -110,6 +110,15 @@ doc: warnroot util: cd $@; $(MAKE) $(MFLAGS) +clean-util: + cd $@; $(MAKE) $(MFLAGS) clean + +install-util: + cd $@; $(MAKE) $(MFLAGS) install + +uninstall-util: + cd $@; $(MAKE) $(MFLAGS) uninstall + clean-example: for i in example util; \ do (cd $$i && $(MAKE) $(MFLAGS) clean) || exit 1; done; @@ -132,7 +141,7 @@ clean: for i in $(SUBDIRS) doc example util docker; \ do (cd $$i && $(MAKE) $(MFLAGS) $@); done; -mrproper: uninstall uninstall-example clean clean-example +mrproper: uninstall uninstall-example uninstall-util clean clean-example clean-util distclean: rm -f Makefile TAGS config.status config.log *~ .depend From 665f5402205dc3a857edddd1a74939205f6028d3 Mon Sep 17 00:00:00 2001 From: Jonathan Ben-Avraham Date: Fri, 13 Nov 2020 13:18:13 +0200 Subject: [PATCH 4/6] Fixed small shell error in Makefile Fixed small shell error in Makefile when testing on Ubuntu 20.04. --- Makefile.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile.in b/Makefile.in index f1f86fcd..00850ae5 100644 --- a/Makefile.in +++ b/Makefile.in @@ -76,7 +76,7 @@ warnroot: checkinstall: @if [ ! -f $(prefix)/include/clixon/clixon.h ]; then \ echo "\e[31mclixon must be installed first to build this target. "\ - "Run 'make'. Then run 'make install' as root.\e[0m"; exit 1; fi; } + "Run 'make'. Then run 'make install' as root.\e[0m"; exit 1; fi; # May cause circular include->include,lib $(SUBDIRS2): $(SUBDIRS1) # Cannot build app before lib (for parallel make -j) From b37cec53fb0059cc0fe53676813148769bcdf1a1 Mon Sep 17 00:00:00 2001 From: Jonathan Ben-Avraham Date: Sat, 14 Nov 2020 21:36:32 +0200 Subject: [PATCH 5/6] Added sanity test of TIMEFN This commit adds a sanity check of the test script TIMEFN, which is by default 'time -p', for the scripts that define it. The scripts are currently written such that if there is no 'time' executable, such as in Debian 9, then some of the scripts fail and some don't but should. This commit fixes that problem. --- test/test_perf_cli.sh | 2 ++ test/test_perf_netconf.sh | 2 ++ test/test_perf_restconf.sh | 1 + test/test_perf_state.sh | 1 + test/test_perf_state_only.sh | 1 + 5 files changed, 7 insertions(+) diff --git a/test/test_perf_cli.sh b/test/test_perf_cli.sh index 1eed7dce..abcbb08b 100755 --- a/test/test_perf_cli.sh +++ b/test/test_perf_cli.sh @@ -19,7 +19,9 @@ s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi # -f %e gives elapsed wall clock time but is not available on all systems # so we use time -p for POSIX compliance and awk to get wall clock time # Note sometimes time -p is used and sometimes $TIMEFN, cant get it to work same everywhere +# time function (this is a mess to get right on freebsd/linux) : ${TIMEFN:=time -p} # portability: 2>&1 | awk '/real/ {print $2}' +if ! $TIMEFN true; then err "A working time function" "'$TIMEFN' does not work"; fi APPNAME=example diff --git a/test/test_perf_netconf.sh b/test/test_perf_netconf.sh index d523e4b4..441ecc4b 100755 --- a/test/test_perf_netconf.sh +++ b/test/test_perf_netconf.sh @@ -20,7 +20,9 @@ s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi # -f %e gives elapsed wall clock time but is not available on all systems # so we use time -p for POSIX compliance and awk to get wall clock time # Note sometimes time -p is used and sometimes $TIMEFN, cant get it to work same everywhere +# time function (this is a mess to get right on freebsd/linux) : ${TIMEFN:=time -p} # portability: 2>&1 | awk '/real/ {print $2}' +if ! $TIMEFN true; then err "A working time function" "'$TIMEFN' does not work"; fi APPNAME=example diff --git a/test/test_perf_restconf.sh b/test/test_perf_restconf.sh index 49dc2c3a..e6d3c58c 100755 --- a/test/test_perf_restconf.sh +++ b/test/test_perf_restconf.sh @@ -21,6 +21,7 @@ s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi # so we use time -p for POSIX compliance and awk to get wall clock time # Note sometimes time -p is used and sometimes $TIMEFN, cant get it to work same everywhere : ${TIMEFN:=time -p} # portability: 2>&1 | awk '/real/ {print $2}' +if ! $TIMEFN true; then err "A working time function" "'$TIMEFN' does not work"; fi APPNAME=example diff --git a/test/test_perf_state.sh b/test/test_perf_state.sh index 020353d6..6c93787b 100755 --- a/test/test_perf_state.sh +++ b/test/test_perf_state.sh @@ -20,6 +20,7 @@ fin=$dir/fin # time function (this is a mess to get right on freebsd/linux) : ${TIMEFN:=time -p} # portability: 2>&1 | awk '/real/ {print $2}' +if ! $TIMEFN true; then err "A working time function" "'$TIMEFN' does not work"; fi APPNAME=example diff --git a/test/test_perf_state_only.sh b/test/test_perf_state_only.sh index 471ed06b..66ae3b18 100755 --- a/test/test_perf_state_only.sh +++ b/test/test_perf_state_only.sh @@ -20,6 +20,7 @@ s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi # time function (this is a mess to get right on freebsd/linux) : ${TIMEFN:=time -p} # portability: 2>&1 | awk '/real/ {print $2}' +if ! $TIMEFN true; then err "A working time function" "'$TIMEFN' does not work"; fi APPNAME=example From ef3a8ed970ff937720cf60b8fb17fa67657c5900 Mon Sep 17 00:00:00 2001 From: Jonathan Ben-Avraham Date: Mon, 16 Nov 2020 11:19:44 +0200 Subject: [PATCH 6/6] Fixed broken *-util target 'cd' commands, comment Fixed broken *-util target 'cd' commands, and added a comment explaining the intended purpose of the 'mrproper' target. --- Makefile.in | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Makefile.in b/Makefile.in index 00850ae5..c60585b6 100644 --- a/Makefile.in +++ b/Makefile.in @@ -111,13 +111,13 @@ util: cd $@; $(MAKE) $(MFLAGS) clean-util: - cd $@; $(MAKE) $(MFLAGS) clean + cd util; $(MAKE) $(MFLAGS) clean install-util: - cd $@; $(MAKE) $(MFLAGS) install + cd util; $(MAKE) $(MFLAGS) install uninstall-util: - cd $@; $(MAKE) $(MFLAGS) uninstall + cd util; $(MAKE) $(MFLAGS) uninstall clean-example: for i in example util; \ @@ -141,6 +141,9 @@ clean: for i in $(SUBDIRS) doc example util docker; \ do (cd $$i && $(MAKE) $(MFLAGS) $@); done; +# Uninstall and clean all the targets used for testing, but without cloning or +# checking-out from git. Provides a reliabily clean slate for testing changes +# before commit. mrproper: uninstall uninstall-example uninstall-util clean clean-example clean-util distclean: