git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v5 08/10] Makefile & test-tool: replace "DC_SHA1" variable with a "define"

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Nov 7, 2022, 21:23 UTC
Message-ID
<patch-v5-08.10-6daac578fd0-20221107T211736Z-avarab@gmail.com>
In-Reply-To
<cover-v5-00.10-00000000000-20221107T211736Z-avarab@gmail.com>

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>
---
 Makefile                            | 8 ++++----
 ci/lib.sh                           | 2 +-
 contrib/buildsystems/CMakeLists.txt | 2 --
 sha1dc_git.h                        | 1 +
 t/helper/test-sha1.c                | 8 ++++++++
 t/helper/test-tool.c                | 1 +
 t/helper/test-tool.h                | 1 +
 t/t0013-sha1dc.sh                   | 6 ++++--
 8 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/Makefile b/Makefile
index 9b5f872d107..0c79546712f 100644
--- 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 1808e3b1ce1..24d20a5d648 100755
--- 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 3957e4cf8cd..2f6e0197ffa 100644
--- 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 41e1c3fd3f7..60e3ce84395 100644
--- 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 d860c387c38..71fe5c61455 100644
--- 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 01cda9358df..775854c9f96 100644
--- 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 ca2948066fd..7cf84eca12e 100644
--- 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 9ad76080aa4..53240476896 100755
--- 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
 

-- 
2.38.0.1464.gea6794aacbc
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 53 of 55 in “core: update our SHA-1 docs, use sha1collisiondetection on OSX too”
  1. 0/5 core: update our SHA-1 docs, use sha1collisiondetection on OSX tooÆvar Arnfjörð Bjarmason, Apr 22, 2022
  2. 1/5 Makefile: create and use sections for "define" flag listingÆvar Arnfjörð Bjarmason, Apr 22, 2022
  3. 3/5 Makefile: rephrase the discussion of *_SHA1 knobsÆvar Arnfjörð Bjarmason, Apr 22, 2022
  4. 5/5 Makefile: use $(OBJECTS) instead of $(C_OBJ)Ævar Arnfjörð Bjarmason, Apr 22, 2022
  5. 2/5 Makefile: really use and document sha1collisiondetection by defaultÆvar Arnfjörð Bjarmason, Apr 22, 2022
  6. 4/5 Makefile + hash.h: remove PPC_SHA1 implementationÆvar Arnfjörð Bjarmason, Apr 22, 2022
  7. Junio C HamanoApr 22, 2022
  8. Ævar Arnfjörð BjarmasonMay 19, 2022
  9. Ævar Arnfjörð BjarmasonMay 26, 2022
  10. 0/4 core: update our SHA-1 docs, use sha1collisiondetection on OSX tooÆvar Arnfjörð Bjarmason, Oct 19, 2022
  11. 1/4 fsmonitor OSX: compile with DC_SHA1=YesPleaseÆvar Arnfjörð Bjarmason, Oct 19, 2022
  12. 3/4 Makefile: really use and document sha1collisiondetection by defaultÆvar Arnfjörð Bjarmason, Oct 19, 2022
  13. Eric SunshineOct 19, 2022
  14. Junio C HamanoOct 19, 2022
  15. Ævar Arnfjörð BjarmasonOct 19, 2022
  16. Junio C HamanoOct 19, 2022
  17. Junio C HamanoOct 19, 2022
  18. Junio C HamanoOct 19, 2022
  19. brian m. carlsonOct 20, 2022
  20. 4/4 Makefile: rephrase the discussion of *_SHA1 knobsÆvar Arnfjörð Bjarmason, Oct 19, 2022
  21. 2/4 Makefile: create and use sections for "define" flag listingÆvar Arnfjörð Bjarmason, Oct 19, 2022
  22. 0/9 Makefile & docs: document SHA-{1,256} behavior, fix bugÆvar Arnfjörð Bjarmason, Oct 20, 2022
  23. 1/9 Makefile: always (re)set DC_SHA1 on fallbackÆvar Arnfjörð Bjarmason, Oct 20, 2022
  24. 2/9 INSTALL: remove discussion of SHA-1 backendsÆvar Arnfjörð Bjarmason, Oct 20, 2022
  25. 3/9 Makefile: correct DC_SHA1 documentationÆvar Arnfjörð Bjarmason, Oct 20, 2022
  26. 5/9 Makefile: rephrase the discussion of *_SHA1 knobsÆvar Arnfjörð Bjarmason, Oct 20, 2022
  27. 6/9 Makefile: document default SHA-256 backendÆvar Arnfjörð Bjarmason, Oct 20, 2022
  28. 7/9 Makefile: document SHA-1 and SHA-256 default and selection orderÆvar Arnfjörð Bjarmason, Oct 20, 2022
  29. Eric SunshineOct 20, 2022
  30. 4/9 Makefile: create and use sections for "define" flag listingÆvar Arnfjörð Bjarmason, Oct 20, 2022
  31. 8/9 Makefile: document default SHA-1 backend on OSXÆvar Arnfjörð Bjarmason, Oct 20, 2022
  32. Eric SunshineOct 20, 2022
  33. 9/9 Makefile: discuss SHAttered in *_SHA{1,256} discussionÆvar Arnfjörð Bjarmason, Oct 20, 2022
  34. 0/9 Makefile & docs: document SHA-{1,256} behavior, fix bugÆvar Arnfjörð Bjarmason, Oct 26, 2022
  35. 1/9 Makefile: always (re)set DC_SHA1 on fallbackÆvar Arnfjörð Bjarmason, Oct 26, 2022
  36. 2/9 INSTALL: remove discussion of SHA-1 backendsÆvar Arnfjörð Bjarmason, Oct 26, 2022
  37. 3/9 Makefile: correct DC_SHA1 documentationÆvar Arnfjörð Bjarmason, Oct 26, 2022
  38. 5/9 Makefile: rephrase the discussion of *_SHA1 knobsÆvar Arnfjörð Bjarmason, Oct 26, 2022
  39. 6/9 Makefile: document default SHA-256 backendÆvar Arnfjörð Bjarmason, Oct 26, 2022
  40. 7/9 Makefile: document SHA-1 and SHA-256 default and selection orderÆvar Arnfjörð Bjarmason, Oct 26, 2022
  41. Junio C HamanoOct 26, 2022
  42. 4/9 Makefile: create and use sections for "define" flag listingÆvar Arnfjörð Bjarmason, Oct 26, 2022
  43. 8/9 Makefile: document default SHA-1 backend on OSXÆvar Arnfjörð Bjarmason, Oct 26, 2022
  44. 9/9 Makefile: discuss SHAttered in *_SHA{1,256} discussionÆvar Arnfjörð Bjarmason, Oct 26, 2022
  45. 00/10 Makefile, docs & code: document & fix SHA-{1,256} selection behaviorÆvar Arnfjörð Bjarmason, Nov 7, 2022
  46. 01/10 Makefile: always (re)set DC_SHA1 on fallbackÆvar Arnfjörð Bjarmason, Nov 7, 2022
  47. 02/10 INSTALL: remove discussion of SHA-1 backendsÆvar Arnfjörð Bjarmason, Nov 7, 2022
  48. 03/10 Makefile: correct DC_SHA1 documentationÆvar Arnfjörð Bjarmason, Nov 7, 2022
  49. 06/10 Makefile: document default SHA-256 backendÆvar Arnfjörð Bjarmason, Nov 7, 2022
  50. 05/10 Makefile: rephrase the discussion of *_SHA1 knobsÆvar Arnfjörð Bjarmason, Nov 7, 2022
  51. 07/10 Makefile: document SHA-1 and SHA-256 default and selection orderÆvar Arnfjörð Bjarmason, Nov 7, 2022
  52. 04/10 Makefile: create and use sections for "define" flag listingÆvar Arnfjörð Bjarmason, Nov 7, 2022
  53. 08/10 Makefile & test-tool: replace "DC_SHA1" variable with a "define"Ævar Arnfjörð Bjarmason, Nov 7, 2022
  54. 09/10 Makefile: document default SHA-1 backend on OSXÆvar Arnfjörð Bjarmason, Nov 7, 2022
  55. 10/10 Makefile: discuss SHAttered in *_SHA{1,256} discussionÆvar Arnfjörð Bjarmason, Nov 7, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.