Makefile & test-tool: replace "DC_SHA1" variable with a "define"

Address the root cause of technical debt we've been carrying since
sha1collisiondetection was made the default in [1]. In a preceding
commit we narrowly fixed a bug where the "DC_SHA1" variable would be
unset (in combination with "NO_APPLE_COMMON_CRYPTO=" on OSX), even
though we had the sha1collisiondetection library enabled.

But the only reason we needed to have such a user-exposed knob went
away with [1], and it's been doing nothing useful since then. We don't
care if you define DC_SHA1=*, we only care that you don't ask for any
other SHA-1 implementation. If it turns out that you didn't, we'll use
sha1collisiondetection, whether you had "DC_SHA1" set or not.

As a result of this being confusing we had e.g. [2] for cmake and the
recent [3] for ci/lib.sh setting "DC_SHA1" explicitly, even though
this was always a NOOP.

A much simpler way to do this is to stop having the Makefile and
CMakeLists.txt set "DC_SHA1" to be picked up by the test-lib.sh, let's
instead add a trivial "test-tool sha1-is-sha1dc". It returns zero if
we're using sha1collisiondetection, non-zero otherwise.

1. e6b07da2780 (Makefile: make DC_SHA1 the default, 2017-03-17)
2. c4b2f41b5f5 (cmake: support for testing git with ctest, 2020-06-26)
3. 1ad5c3df35a (ci: use DC_SHA1=YesPlease on osx-clang job for CI,
   2022-10-20)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>

diff --git a/Makefile b/Makefile
index 9b5f872d10724e06e926311c065c3cf9415c89ab..0c79546712f6629e633ed643ba60d71fc8883de5 100644 (file)
--- a/Makefile
+++ b/Makefile
@@ -500,8 +500,10 @@ include shared.mak
 # Define BLK_SHA1 to make use of optimized C SHA-1 routines bundled
 # with git (in the block-sha1/ directory).
 #
-# Define DC_SHA1 to enable the collision-detecting sha1
-# algorithm. This is slower, but may detect attempted collision attacks.
+# If don't enable any of the *_SHA1 settings in this section, Git will
+# default to its built-in sha1collisiondetection library, which is a
+# collision-detecting sha1 This is slower, but may detect attempted
+# collision attacks.
 #
 # ==== Options for the sha1collisiondetection library ====
 #
@@ -1867,7 +1869,6 @@ ifdef APPLE_COMMON_CRYPTO
        COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
        BASIC_CFLAGS += -DSHA1_APPLE
 else
-       override DC_SHA1 = YesPlease
        BASIC_CFLAGS += -DSHA1_DC
        LIB_OBJS += sha1dc_git.o
 ifdef DC_SHA1_EXTERNAL
@@ -3030,7 +3031,6 @@ GIT-BUILD-OPTIONS: FORCE
        @echo NO_REGEX=\''$(subst ','\'',$(subst ','\'',$(NO_REGEX)))'\' >>$@+
        @echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst ','\'',$(NO_UNIX_SOCKETS)))'\' >>$@+
        @echo PAGER_ENV=\''$(subst ','\'',$(subst ','\'',$(PAGER_ENV)))'\' >>$@+
-       @echo DC_SHA1=\''$(subst ','\'',$(subst ','\'',$(DC_SHA1)))'\' >>$@+
        @echo SANITIZE_LEAK=\''$(subst ','\'',$(subst ','\'',$(SANITIZE_LEAK)))'\' >>$@+
        @echo SANITIZE_ADDRESS=\''$(subst ','\'',$(subst ','\'',$(SANITIZE_ADDRESS)))'\' >>$@+
        @echo X=\'$(X)\' >>$@+

diff --git a/ci/lib.sh b/ci/lib.sh
index 1808e3b1ce1d1c12428511bac6cba5182991ce36..24d20a5d648d74596d57bddb29597e4766cf275f 100755 (executable)
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -260,7 +260,7 @@ macos-latest)
        else
                MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python2)"
                MAKEFLAGS="$MAKEFLAGS NO_APPLE_COMMON_CRYPTO=NoThanks"
-               MAKEFLAGS="$MAKEFLAGS DC_SHA1=YesPlease NO_OPENSSL=NoThanks"
+               MAKEFLAGS="$MAKEFLAGS NO_OPENSSL=NoThanks"
        fi
        ;;
 esac

diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index 3957e4cf8cd5be6e2524b50a51032d6a0b811aaa..2f6e0197ffa489ccc14b0665cdf78dba43b39e3e 100644 (file)
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -1025,7 +1025,6 @@ set(NO_PERL )
 set(NO_PTHREADS )
 set(NO_PYTHON )
 set(PAGER_ENV "LESS=FRX LV=-c")
-set(DC_SHA1 YesPlease)
 set(RUNTIME_PREFIX true)
 set(NO_GETTEXT )
 
@@ -1061,7 +1060,6 @@ file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_PERL='${NO_PERL}'\n")
 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_PTHREADS='${NO_PTHREADS}'\n")
 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_UNIX_SOCKETS='${NO_UNIX_SOCKETS}'\n")
 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "PAGER_ENV='${PAGER_ENV}'\n")
-file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "DC_SHA1='${DC_SHA1}'\n")
 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "X='${EXE_EXTENSION}'\n")
 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_GETTEXT='${NO_GETTEXT}'\n")
 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "RUNTIME_PREFIX='${RUNTIME_PREFIX}'\n")

diff --git a/sha1dc_git.h b/sha1dc_git.h
index 41e1c3fd3f787e04d6e4fa9eb7c56b617f1c5fa5..60e3ce84395ceaa70648a828ea1610a7fe9705a6 100644 (file)
--- a/sha1dc_git.h
+++ b/sha1dc_git.h
@@ -17,6 +17,7 @@ void git_SHA1DCInit(SHA1_CTX *);
 void git_SHA1DCFinal(unsigned char [20], SHA1_CTX *);
 void git_SHA1DCUpdate(SHA1_CTX *ctx, const void *data, unsigned long len);
 
+#define platform_SHA_IS_SHA1DC /* used by "test-tool sha1-is-sha1dc" */
 #define platform_SHA_CTX SHA1_CTX
 #define platform_SHA1_Init git_SHA1DCInit
 #define platform_SHA1_Update git_SHA1DCUpdate

diff --git a/t/helper/test-sha1.c b/t/helper/test-sha1.c
index d860c387c3846d69b7bd63a144ede2f93da60886..71fe5c61455a89eddd6363a87e3c4eb7968f09b6 100644 (file)
--- a/t/helper/test-sha1.c
+++ b/t/helper/test-sha1.c
@@ -5,3 +5,11 @@ int cmd__sha1(int ac, const char **av)
 {
        return cmd_hash_impl(ac, av, GIT_HASH_SHA1);
 }
+
+int cmd__sha1_is_sha1dc(int argc UNUSED, const char **argv UNUSED)
+{
+#ifdef platform_SHA_IS_SHA1DC
+       return 0;
+#endif
+       return 1;
+}

diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index 01cda9358df01f50de95a93c8aee795696da377d..775854c9f96ea9ba59a8339b788b0b1afe06f05f 100644 (file)
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -73,6 +73,7 @@ static struct test_cmd cmds[] = {
        { "scrap-cache-tree", cmd__scrap_cache_tree },
        { "serve-v2", cmd__serve_v2 },
        { "sha1", cmd__sha1 },
+       { "sha1-is-sha1dc", cmd__sha1_is_sha1dc },
        { "sha256", cmd__sha256 },
        { "sigchain", cmd__sigchain },
        { "simple-ipc", cmd__simple_ipc },

diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index ca2948066fd64ec4743e498fffd06829c2cfce2b..7cf84eca12e4fa01c7bf19c9c0c5101f0f81e06b 100644 (file)
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -66,6 +66,7 @@ int cmd__run_command(int argc, const char **argv);
 int cmd__scrap_cache_tree(int argc, const char **argv);
 int cmd__serve_v2(int argc, const char **argv);
 int cmd__sha1(int argc, const char **argv);
+int cmd__sha1_is_sha1dc(int argc, const char **argv);
 int cmd__oid_array(int argc, const char **argv);
 int cmd__sha256(int argc, const char **argv);
 int cmd__sigchain(int argc, const char **argv);

diff --git a/t/t0013-sha1dc.sh b/t/t0013-sha1dc.sh
index 9ad76080aa49d46d03e2bb805d039e6a88d87e5c..53240476896d8e80a969f1b9b448fd01b45fa711 100755 (executable)
--- a/t/t0013-sha1dc.sh
+++ b/t/t0013-sha1dc.sh
@@ -6,9 +6,11 @@ TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 TEST_DATA="$TEST_DIRECTORY/t0013"
 
-if test -z "$DC_SHA1"
+test_lazy_prereq SHA1_IS_SHA1DC 'test-tool sha1-is-sha1dc'
+
+if ! test_have_prereq SHA1_IS_SHA1DC
 then
-       skip_all='skipping sha1 collision tests, DC_SHA1 not set'
+       skip_all='skipping sha1 collision tests, not using sha1collisiondetection'
        test_done
 fi