@matheusbsbs / matheus-bagliano / commits / dc1cf3580e

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>

Ævar Arnfjörð Bjarmason committed Nov 7, 2022 at 22:23 UTC dc1cf3580e2ec7dd164c95cec2f5568beaf3324b

8 files changed +20 -9

Makefile

+4 -4

@@ -500,8 +500,10 @@ include shared.mak

500 # Define BLK_SHA1 to make use of optimized C SHA-1 routines bundled

501 # with git (in the block-sha1/ directory).

502 #

503 -# Define DC_SHA1 to enable the collision-detecting sha1

504 -# algorithm. This is slower, but may detect attempted collision attacks.

503 +# If don't enable any of the *_SHA1 settings in this section, Git will

504 +# default to its built-in sha1collisiondetection library, which is a

505 +# collision-detecting sha1 This is slower, but may detect attempted

506 +# collision attacks.

507 #

508 # ==== Options for the sha1collisiondetection library ====

509 #

@@ -1867,7 +1869,6 @@ ifdef APPLE_COMMON_CRYPTO

1869 COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL

1870 BASIC_CFLAGS += -DSHA1_APPLE

1871 else

1870 - override DC_SHA1 = YesPlease

1872 BASIC_CFLAGS += -DSHA1_DC

1873 LIB_OBJS += sha1dc_git.o

1874 ifdef DC_SHA1_EXTERNAL

@@ -3030,7 +3031,6 @@ GIT-BUILD-OPTIONS: FORCE

3031 @echo NO_REGEX=\''$(subst ','\'',$(subst ','\'',$(NO_REGEX)))'\' >>$@+

3032 @echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst ','\'',$(NO_UNIX_SOCKETS)))'\' >>$@+

3033 @echo PAGER_ENV=\''$(subst ','\'',$(subst ','\'',$(PAGER_ENV)))'\' >>$@+

3033 - @echo DC_SHA1=\''$(subst ','\'',$(subst ','\'',$(DC_SHA1)))'\' >>$@+

3034 @echo SANITIZE_LEAK=\''$(subst ','\'',$(subst ','\'',$(SANITIZE_LEAK)))'\' >>$@+

3035 @echo SANITIZE_ADDRESS=\''$(subst ','\'',$(subst ','\'',$(SANITIZE_ADDRESS)))'\' >>$@+

3036 @echo X=\'$(X)\' >>$@+

ci/lib.sh

+1 -1

@@ -260,7 +260,7 @@ macos-latest)

260 else

261 MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python2)"

262 MAKEFLAGS="$MAKEFLAGS NO_APPLE_COMMON_CRYPTO=NoThanks"

263 - MAKEFLAGS="$MAKEFLAGS DC_SHA1=YesPlease NO_OPENSSL=NoThanks"

263 + MAKEFLAGS="$MAKEFLAGS NO_OPENSSL=NoThanks"

264 fi

265 ;;

266 esac

contrib/buildsystems/CMakeLists.txt

-2

@@ -1025,7 +1025,6 @@ set(NO_PERL )

1025 set(NO_PTHREADS )

1026 set(NO_PYTHON )

1027 set(PAGER_ENV "LESS=FRX LV=-c")

1028 -set(DC_SHA1 YesPlease)

1028 set(RUNTIME_PREFIX true)

1029 set(NO_GETTEXT )

1030

@@ -1061,7 +1060,6 @@ file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_PERL='${NO_PERL}'\n")

1060 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_PTHREADS='${NO_PTHREADS}'\n")

1061 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_UNIX_SOCKETS='${NO_UNIX_SOCKETS}'\n")

1062 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "PAGER_ENV='${PAGER_ENV}'\n")

1064 -file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "DC_SHA1='${DC_SHA1}'\n")

1063 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "X='${EXE_EXTENSION}'\n")

1064 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "NO_GETTEXT='${NO_GETTEXT}'\n")

1065 file(APPEND ${CMAKE_BINARY_DIR}/GIT-BUILD-OPTIONS "RUNTIME_PREFIX='${RUNTIME_PREFIX}'\n")

sha1dc_git.h

@@ -17,6 +17,7 @@ void git_SHA1DCInit(SHA1_CTX *);

17 void git_SHA1DCFinal(unsigned char [20], SHA1_CTX *);

18 void git_SHA1DCUpdate(SHA1_CTX *ctx, const void *data, unsigned long len);

20 +#define platform_SHA_IS_SHA1DC /* used by "test-tool sha1-is-sha1dc" */

21 #define platform_SHA_CTX SHA1_CTX

22 #define platform_SHA1_Init git_SHA1DCInit

23 #define platform_SHA1_Update git_SHA1DCUpdate

t/helper/test-sha1.c

@@ -5,3 +5,11 @@ int cmd__sha1(int ac, const char **av)

5 {

6 return cmd_hash_impl(ac, av, GIT_HASH_SHA1);

7 }

8 +

9 +int cmd__sha1_is_sha1dc(int argc UNUSED, const char **argv UNUSED)

10 +{

11 +#ifdef platform_SHA_IS_SHA1DC

12 + return 0;

13 +#endif

14 + return 1;

15 +}

t/helper/test-tool.c

@@ -73,6 +73,7 @@ static struct test_cmd cmds[] = {

73 { "scrap-cache-tree", cmd__scrap_cache_tree },

74 { "serve-v2", cmd__serve_v2 },

75 { "sha1", cmd__sha1 },

76 + { "sha1-is-sha1dc", cmd__sha1_is_sha1dc },

77 { "sha256", cmd__sha256 },

78 { "sigchain", cmd__sigchain },

79 { "simple-ipc", cmd__simple_ipc },

t/helper/test-tool.h

@@ -66,6 +66,7 @@ int cmd__run_command(int argc, const char **argv);

66 int cmd__scrap_cache_tree(int argc, const char **argv);

67 int cmd__serve_v2(int argc, const char **argv);

68 int cmd__sha1(int argc, const char **argv);

69 +int cmd__sha1_is_sha1dc(int argc, const char **argv);

70 int cmd__oid_array(int argc, const char **argv);

71 int cmd__sha256(int argc, const char **argv);

72 int cmd__sigchain(int argc, const char **argv);

t/t0013-sha1dc.sh

+4 -2

@@ -6,9 +6,11 @@ TEST_PASSES_SANITIZE_LEAK=true

6 . ./test-lib.sh

7 TEST_DATA="$TEST_DIRECTORY/t0013"

9 -if test -z "$DC_SHA1"

9 +test_lazy_prereq SHA1_IS_SHA1DC 'test-tool sha1-is-sha1dc'

10 +

11 +if ! test_have_prereq SHA1_IS_SHA1DC

12 then

11 - skip_all='skipping sha1 collision tests, DC_SHA1 not set'

13 + skip_all='skipping sha1 collision tests, not using sha1collisiondetection'

14 test_done

15 fi