On 2018-03-08 05:18 +0000, Wookey 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.
2 items here:
5) Various files/dirs are not cleaned up by the test makefiles. This patch fixes this:
Index: libopencsd-0.8.0/decoder/tests/build/linux/c_api_pkt_print_test/makefile =================================================================== --- libopencsd-0.8.0.orig/decoder/tests/build/linux/c_api_pkt_print_test/makefile +++ libopencsd-0.8.0/decoder/tests/build/linux/c_api_pkt_print_test/makefile @@ -88,5 +88,7 @@ $(BUILD_DIR)/%.o : %.c clean : rm -f $(BIN_TEST_TARGET_DIR)/$(PROG) $(OBJECTS) rm -f $(DEPS) + -rmdir $(BUILD_DIR) + -rm ./*.so
# end of file makefile Index: libopencsd-0.8.0/decoder/tests/build/linux/echo_test_dcd_lib/makefile =================================================================== --- libopencsd-0.8.0.orig/decoder/tests/build/linux/echo_test_dcd_lib/makefile +++ libopencsd-0.8.0/decoder/tests/build/linux/echo_test_dcd_lib/makefile @@ -84,5 +84,6 @@ clean: rm -f $(OBJECTS) rm -f $(LIB_TEST_TARGET_DIR)/$(LIB_NAME).a rm -f $(DEPS) + -rmdir $(BUILD_DIR)
# end of file makefile Index: libopencsd-0.8.0/decoder/tests/build/linux/snapshot_parser_lib/makefile =================================================================== --- libopencsd-0.8.0.orig/decoder/tests/build/linux/snapshot_parser_lib/makefile +++ libopencsd-0.8.0/decoder/tests/build/linux/snapshot_parser_lib/makefile @@ -90,3 +90,4 @@ clean: rm -f $(OBJECTS) rm -f $(DEPS) rm -f $(LIB_TEST_TARGET_DIR)/$(LIB_NAME).a + -rmdir $(BUILD_DIR) Index: libopencsd-0.8.0/decoder/tests/build/linux/trc_pkt_lister/makefile =================================================================== --- libopencsd-0.8.0.orig/decoder/tests/build/linux/trc_pkt_lister/makefile +++ libopencsd-0.8.0/decoder/tests/build/linux/trc_pkt_lister/makefile @@ -87,5 +87,7 @@ $(BUILD_DIR)/%.o : %.cpp clean : rm -f $(BIN_TEST_TARGET_DIR)/$(PROG) $(OBJECTS) rm -f $(DEPS) + -rmdir $(BUILD_DIR) + -rm $(BIN_TEST_TARGET_DIR)/*.so
# end of file makefile
6) I find it good practice to use '-' in makefiles for lines that may fail (like rm in clean targets, when maybe there is nothing to clean). As opposed to using rm -f, or rm -rf.
'-' means "don't consider this line failing to be an error", which is exactly what we want here.
rm -rf $(foo) always has the risk of deleting everything if $(foo) turns out empty.
The patch also fixes up some pointless whitespace in one clean rule so they are consistent.
So this is mostly stylistic, but here's the patch if you agree: Index: libopencsd-0.8.0/decoder/tests/build/linux/c_api_pkt_print_test/makefile =================================================================== --- libopencsd-0.8.0.orig/decoder/tests/build/linux/c_api_pkt_print_test/makefile +++ libopencsd-0.8.0/decoder/tests/build/linux/c_api_pkt_print_test/makefile @@ -86,9 +86,9 @@ $(BUILD_DIR)/%.o : %.c #### clean .PHONY: clean clean : - rm -f $(BIN_TEST_TARGET_DIR)/$(PROG) $(OBJECTS) - rm -f $(DEPS) - -rmdir $(BUILD_DIR) + -rm $(BIN_TEST_TARGET_DIR)/$(PROG) $(OBJECTS) + -rm $(DEPS) -rm ./*.so + -rmdir $(BUILD_DIR)
# end of file makefile Index: libopencsd-0.8.0/decoder/tests/build/linux/echo_test_dcd_lib/makefile =================================================================== --- libopencsd-0.8.0.orig/decoder/tests/build/linux/echo_test_dcd_lib/makefile +++ libopencsd-0.8.0/decoder/tests/build/linux/echo_test_dcd_lib/makefile @@ -81,9 +81,9 @@ $(BUILD_DIR)/%.o : %.c #### clean .PHONY: clean clean: - rm -f $(OBJECTS) - rm -f $(LIB_TEST_TARGET_DIR)/$(LIB_NAME).a - rm -f $(DEPS) + -rm $(OBJECTS) + -rm $(LIB_TEST_TARGET_DIR)/$(LIB_NAME).a + -rm $(DEPS) -rmdir $(BUILD_DIR)
# end of file makefile Index: libopencsd-0.8.0/decoder/tests/build/linux/snapshot_parser_lib/makefile =================================================================== --- libopencsd-0.8.0.orig/decoder/tests/build/linux/snapshot_parser_lib/makefile +++ libopencsd-0.8.0/decoder/tests/build/linux/snapshot_parser_lib/makefile @@ -87,7 +87,7 @@ $(BUILD_DIR)/%.o : %.cpp ### clean .PHONY: clean clean: - rm -f $(OBJECTS) - rm -f $(DEPS) - rm -f $(LIB_TEST_TARGET_DIR)/$(LIB_NAME).a + -rm $(OBJECTS) + -rm $(DEPS) + -rm $(LIB_TEST_TARGET_DIR)/$(LIB_NAME).a -rmdir $(BUILD_DIR) Index: libopencsd-0.8.0/decoder/tests/build/linux/trc_pkt_lister/makefile =================================================================== --- libopencsd-0.8.0.orig/decoder/tests/build/linux/trc_pkt_lister/makefile +++ libopencsd-0.8.0/decoder/tests/build/linux/trc_pkt_lister/makefile @@ -85,9 +85,9 @@ $(BUILD_DIR)/%.o : %.cpp #### clean .PHONY: clean clean : - rm -f $(BIN_TEST_TARGET_DIR)/$(PROG) $(OBJECTS) - rm -f $(DEPS) - -rmdir $(BUILD_DIR) - -rm $(BIN_TEST_TARGET_DIR)/*.so + -rm $(BIN_TEST_TARGET_DIR)/$(PROG) $(OBJECTS) + -rm $(DEPS) + -rm $(BIN_TEST_TARGET_DIR)/*.so + -rmdir $(BUILD_DIR)
# end of file makefile 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 @@ -180,7 +180,7 @@ clean_tests: 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,a} - rm -f $(INSTALL_LIB_DIR)/lib$(LIB_CAPI_NAME).{so,a} - rm -rf $(INSTALL_INCLUDE_DIR)/$(LIB_UAPI_INC_DIR) - rm -rf $(OCSD_ROOT)/docs/html + -rm $(INSTALL_LIB_DIR)/lib$(LIB_BASE_NAME).{so,a} + -rm $(INSTALL_LIB_DIR)/lib$(LIB_CAPI_NAME).{so,a} + -rm -r $(INSTALL_INCLUDE_DIR)/$(LIB_UAPI_INC_DIR) + -rm -r $(OCSD_ROOT)/docs/html
Wookey