{"thread":{"id":"63212","subject":"[PATCH 0/2] help: include SHA build options in version info","startedAt":"2025-03-28T17:05:24Z","lastAt":"2025-04-08T00:33:16Z","messageCount":27,"participants":["Justin Tobler","Christian Couder","Junio C Hamano","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"515243","messageId":"20250328170121.157563-1-jltobler@gmail.com","threadId":"63212","inReplyTo":null,"subject":"[PATCH 0/2] help: include SHA build options in version info","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-03-28T17:01:19Z","receivedAt":"2025-03-28T17:05:24Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"Greetings,\n\nAdditional information regarding how Git was build can be found via the\n`--build-options` flag for git-verison(1). This currectly does not\ninclude information about the SHA-1 and SHA-256 implementations Git is\nbuilt with.\n\nThis short series adds build option info for the SHA-1, SHA-256, and\nunsafe-SHA-1 (if any) implementations which may be useful for diagnostic\npurposes.\n\nRegarding \"unsafe-SHA-1\", I wonder if we should use a different name in\nthe printed build options that sounds a little less scary. I was\nthinking maybe \"fast-SHA-1\" and document its meaning appropriately. I'm\ninterested to know if anyone has thoughts on this.\n\nThanks,\n-Justin\n\nJustin Tobler (2):\n help: include SHA implementation in version info\n help: include unsafe SHA-1 build info in version\n\n Documentation/git-version.adoc | 3 +++\n help.c | 36 ++++++++++++++++++++++++++++++++++\n 2 files changed, 39 insertions(+)\n\n\nbase-commit: 683c54c999c301c2cd6f715c411407c413b1d84e\n-- \n2.49.0\n\n"},{"id":"515244","messageId":"20250328170121.157563-2-jltobler@gmail.com","threadId":"63212","inReplyTo":"20250328170121.157563-1-jltobler@gmail.com","subject":"[PATCH 1/2] help: include SHA implementation in version info","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-03-28T17:01:20Z","receivedAt":"2025-03-28T17:05:25Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"When the `--build-options` flag is used with git-version(1), additional\ninformation about the built version of Git is printed. During build\ntime, different SHA implementations may be configured, but this\ninformation is not included in the version info.\n\nAdd the SHA implementations Git is built with to the version info.\n\nSigned-off-by: Justin Tobler \n---\n help.c | 28 ++++++++++++++++++++++++++++\n 1 file changed, 28 insertions(+)\n\ndiff --git a/help.c b/help.c\nindex c54bd9918a..32b5d4e6f5 100644\n--- a/help.c\n+++ b/help.c\n@@ -768,6 +768,33 @@ char *help_unknown_cmd(const char *cmd)\n \texit(1);\n }\n \n+static void get_sha_impl(struct strbuf *buf)\n+{\n+#if defined(SHA1_OPENSSL)\n+\tstrbuf_addstr(buf, \"SHA-1: OpenSSL\\n\");\n+#elif defined(SHA1_BLK)\n+\tstrbuf_addstr(buf, \"SHA-1: blk\\n\");\n+#elif defined(SHA1_APPLE)\n+\tstrbuf_addstr(buf, \"SHA-1: Apple CommonCrypto\\n\");\n+#elif defined(DC_SHA1_EXTERNAL)\n+\tstrbuf_addstr(buf, \"SHA-1: Collision Detection (External)\\n\");\n+#elif defined(DC_SHA1_SUBMODULE)\n+\tstrbuf_addstr(buf, \"SHA-1: Collision Detection (Submodule)\\n\");\n+#elif defined(SHA1_DC)\n+\tstrbuf_addstr(buf, \"SHA-1: Collision Detection\\n\");\n+#endif\n+\n+#if defined(SHA256_OPENSSL)\n+\tstrbuf_addstr(buf, \"SHA-256: OpenSSL\\n\");\n+#elif defined(SHA256_NETTLE)\n+\tstrbuf_addstr(buf, \"SHA-256: Nettle\\n\");\n+#elif defined(SHA256_GCRYPT)\n+\tstrbuf_addstr(buf, \"SHA-256: gcrypt\\n\");\n+#elif defined(SHA256_BLK)\n+\tstrbuf_addstr(buf, \"SHA-256: blk\\n\");\n+#endif\n+}\n+\n void get_version_info(struct strbuf *buf, int show_build_options)\n {\n \t/*\n@@ -803,6 +830,7 @@ void get_version_info(struct strbuf *buf, int show_build_options)\n #elif defined ZLIB_VERSION\n \t\tstrbuf_addf(buf, \"zlib: %s\\n\", ZLIB_VERSION);\n #endif\n+\t\tget_sha_impl(buf);\n \t}\n }\n \n-- \n2.49.0\n\n"},{"id":"515245","messageId":"20250328170121.157563-3-jltobler@gmail.com","threadId":"63212","inReplyTo":"20250328170121.157563-1-jltobler@gmail.com","subject":"[PATCH 2/2] help: include unsafe SHA-1 build info in version","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-03-28T17:01:21Z","receivedAt":"2025-03-28T17:05:26Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"In 06c92dafb8 (Makefile: allow specifying a SHA-1 for non-cryptographic\nuses, 2024-09-26), support for unsafe SHA-1 is added. Add the unsafe\nSHA-1 build info to `git version --build-info` and update corresponding\ndocumentation.\n\nSigned-off-by: Justin Tobler \n---\n Documentation/git-version.adoc | 3 +++\n help.c | 8 ++++++++\n 2 files changed, 11 insertions(+)\n\ndiff --git a/Documentation/git-version.adoc b/Documentation/git-version.adoc\nindex 80fa7754a6..53c8ba74c1 100644\n--- a/Documentation/git-version.adoc\n+++ b/Documentation/git-version.adoc\n@@ -22,6 +22,9 @@ OPTIONS\n --build-options::\n \tInclude additional information about how git was built for diagnostic\n \tpurposes.\n++\n+If built to use a faster SHA-1 implementation for non-cryptographic purposes,\n+the implmentation used is denoted as \"unsafe-SHA-1\".\n \n GIT\n ---\ndiff --git a/help.c b/help.c\nindex 32b5d4e6f5..7670e0a64a 100644\n--- a/help.c\n+++ b/help.c\n@@ -784,6 +784,14 @@ static void get_sha_impl(struct strbuf *buf)\n \tstrbuf_addstr(buf, \"SHA-1: Collision Detection\\n\");\n #endif\n \n+#if defined(SHA1_OPENSSL_UNSAFE)\n+\tstrbuf_addstr(buf, \"unsafe-SHA-1: OpenSSL\\n\");\n+#elif defined(SHA1_BLK_UNSAFE)\n+\tstrbuf_addstr(buf, \"unsafe-SHA-1: blk\\n\");\n+#elif defined(SHA1_APPLE_UNSAFE)\n+\tstrbuf_addstr(buf, \"unsafe-SHA-1: Apple CommonCrypto\\n\");\n+#endif\n+\n #if defined(SHA256_OPENSSL)\n \tstrbuf_addstr(buf, \"SHA-256: OpenSSL\\n\");\n #elif defined(SHA256_NETTLE)\n-- \n2.49.0\n\n"},{"id":"515265","messageId":"CAP8UFD3tXtoCcCt0+=SVh76JNvRGL7DOH9Td3igKn6AS07bfiw@mail.gmail.com","threadId":"63212","inReplyTo":"20250328170121.157563-3-jltobler@gmail.com","subject":"Re: [PATCH 2/2] help: include unsafe SHA-1 build info in version","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-03-29T08:42:39Z","receivedAt":"2025-03-29T08:42:53Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Mar 28, 2025 at 6:05 PM Justin Tobler wrote:\n>\n> In 06c92dafb8 (Makefile: allow specifying a SHA-1 for non-cryptographic\n> uses, 2024-09-26), support for unsafe SHA-1 is added. Add the unsafe\n> SHA-1 build info to `git version --build-info` and update corresponding\n> documentation.\n>\n> Signed-off-by: Justin Tobler \n> ---\n> Documentation/git-version.adoc | 3 +++\n> help.c | 8 ++++++++\n> 2 files changed, 11 insertions(+)\n>\n> diff --git a/Documentation/git-version.adoc b/Documentation/git-version.adoc\n> index 80fa7754a6..53c8ba74c1 100644\n> --- a/Documentation/git-version.adoc\n> +++ b/Documentation/git-version.adoc\n> @@ -22,6 +22,9 @@ OPTIONS\n> --build-options::\n> Include additional information about how git was built for diagnostic\n> purposes.\n> ++\n> +If built to use a faster SHA-1 implementation for non-cryptographic purposes,\n> +the implmentation used is denoted as \"unsafe-SHA-1\".\n\ns/the implmentation used/that implementation/\n"},{"id":"515266","messageId":"CAP8UFD1oYmr=2ZCOVVjk6NbSCtA1Zv+3jy26jSSwHuOfdS6BQA@mail.gmail.com","threadId":"63212","inReplyTo":"20250328170121.157563-1-jltobler@gmail.com","subject":"Re: [PATCH 0/2] help: include SHA build options in version info","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-03-29T08:58:34Z","receivedAt":"2025-03-29T08:58:48Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Mar 28, 2025 at 6:05 PM Justin Tobler wrote:\n>\n> Greetings,\n>\n> Additional information regarding how Git was build can be found via the\n\nMaybe: s/build/built/\n\n> `--build-options` flag for git-verison(1). This currectly does not\n\ns/git-verison/git-version/\ns/currectly/currently/\n\n> include information about the SHA-1 and SHA-256 implementations Git is\n> built with.\n>\n> This short series adds build option info for the SHA-1, SHA-256, and\n> unsafe-SHA-1 (if any) implementations which may be useful for diagnostic\n> purposes.\n>\n> Regarding \"unsafe-SHA-1\", I wonder if we should use a different name in\n> the printed build options that sounds a little less scary. I was\n> thinking maybe \"fast-SHA-1\" and document its meaning appropriately. I'm\n> interested to know if anyone has thoughts on this.\n\nMaybe we could use just \"SHA-1\" if a single algorithm is used for\neverything, and both \"SHA-1 for crypto\" and \"SHA-1 for non-crypto\"\notherwise.\n\nRelated to this I wonder if we should warn in some ways if a non\ncollision detection algorithm is used for crypto. For example we could\nprint \"SHA-1: OpenSSL (No collision detection!!!)\" instead of just\n\"SHA-1: OpenSSL\". And yeah that should be documented.\n\nThanks!\n"},{"id":"515270","messageId":"xmqq8qoodq5u.fsf@gitster.g","threadId":"63212","inReplyTo":"20250328170121.157563-2-jltobler@gmail.com","subject":"Re: [PATCH 1/2] help: include SHA implementation in version info","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-29T11:36:45Z","receivedAt":"2025-03-29T11:36:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Justin Tobler writes:\n\n> When the `--build-options` flag is used with git-version(1), additional\n> information about the built version of Git is printed. During build\n> time, different SHA implementations may be configured, but this\n> information is not included in the version info.\n>\n> Add the SHA implementations Git is built with to the version info.\n> ...\n> +static void get_sha_impl(struct strbuf *buf)\n> +{\n> +#if defined(SHA1_OPENSSL)\n> +\tstrbuf_addstr(buf, \"SHA-1: OpenSSL\\n\");\n> +#elif defined(SHA1_BLK)\n> +\tstrbuf_addstr(buf, \"SHA-1: blk\\n\");\n> +#elif defined(SHA1_APPLE)\n> +\tstrbuf_addstr(buf, \"SHA-1: Apple CommonCrypto\\n\");\n> +#elif defined(DC_SHA1_EXTERNAL)\n> +\tstrbuf_addstr(buf, \"SHA-1: Collision Detection (External)\\n\");\n> +#elif defined(DC_SHA1_SUBMODULE)\n> +\tstrbuf_addstr(buf, \"SHA-1: Collision Detection (Submodule)\\n\");\n> +#elif defined(SHA1_DC)\n> +\tstrbuf_addstr(buf, \"SHA-1: Collision Detection\\n\");\n> +#endif\n> +\n> +#if defined(SHA256_OPENSSL)\n> +\tstrbuf_addstr(buf, \"SHA-256: OpenSSL\\n\");\n> +#elif defined(SHA256_NETTLE)\n> +\tstrbuf_addstr(buf, \"SHA-256: Nettle\\n\");\n> +#elif defined(SHA256_GCRYPT)\n> +\tstrbuf_addstr(buf, \"SHA-256: gcrypt\\n\");\n> +#elif defined(SHA256_BLK)\n> +\tstrbuf_addstr(buf, \"SHA-256: blk\\n\");\n> +#endif\n> +}\n\nWhile I agree with the objective of the change, I am not sure how I\nfeel about the implementation. Given that\n\n - The code here, and probably the existing code paths that depend\n on these SHA1_$WHOSE symbols, assume that only one of them is\n defined;\n\n - The \"git help --build-options\" is not an end-user thing but more\n is a developer thing.\n\nThe thing I am most worried about is that it is unclear how the\norder in which the SHA1_$WHOSE symbols are inspected here and\nelsewhere in the code are kept in sync. What happens when, for\nexample, SHA1_OPENSSL and SHA1_APPLE_UNSAFE are both defined? The\nabove code will report that we are using SHA1_OPENSSL, but hash.h\nwould probably use SHA1_APPLE as it has its own if/elif/endif\ncascade.\n\nPerhaps it does not matter, if the build infrastructure ensures that\nthe build fails unless one and only one of SHA1_$WHOSE is defined.\n\nBut with the way how this part is written with an if/elif/endif\ncascade, it makes readers spend time wondering how the precedence\norder here is kept in sync throughout the system. If I am not\nmistaken, the top-level Makefile has its own ifdef/else/if/endif*\ncascade.\n\nI imagine that making all of the above not if/elif/endif chain, but\nmake them pretend as if they are independent and orthogonal choices,\nwould make it simpler to understand and also it will help us catch a\nmisconfiguration where more than one is defined, i.e.\n\n static void get_sha_impl(struct strbuf *buf)\n {\n #if defined(SHA1_OPENSSL)\n strbuf_addstr(buf, \"SHA-1: OpenSSL\\n\");\n #endif\n #if defined(SHA1_BLK)\n strbuf_addstr(buf, \"SHA-1: blk\\n\");\n #endif\n #if defined(SHA1_APPLE)\n ...\n\n\nThat way, we wouldn't force future devlopers who are plugging new\nimplementations of SHA-256 wonder where is the right place in the\nexisting if/elif/endif cascade their new one fits. It also allows\nus to catch misconfigurations to define more then one of them at the\nsame time, if such a thing becomes ever possible.\n\nAlso, wouldn't it make more sense to just reuse the internal symbol\nfor reporting, i.e.\n\n\tstrbuf_addstr(buf, \"SHA-1: SHA1_OPENSSL\\n\");\n\ninstead of having to come up with \"human readable\" name here\n\n"},{"id":"515326","messageId":"Z-pCG9d7Rf9SMuXJ@pks.im","threadId":"63212","inReplyTo":"xmqq8qoodq5u.fsf@gitster.g","subject":"Re: [PATCH 1/2] help: include SHA implementation in version info","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-31T07:19:55Z","receivedAt":"2025-03-31T07:20:01Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sat, Mar 29, 2025 at 04:36:45AM -0700, Junio C Hamano wrote:\n> Justin Tobler writes:\n> \n> > When the `--build-options` flag is used with git-version(1), additional\n> > information about the built version of Git is printed. During build\n> > time, different SHA implementations may be configured, but this\n> > information is not included in the version info.\n> >\n> > Add the SHA implementations Git is built with to the version info.\n> > ...\n> > +static void get_sha_impl(struct strbuf *buf)\n> > +{\n> > +#if defined(SHA1_OPENSSL)\n> > +\tstrbuf_addstr(buf, \"SHA-1: OpenSSL\\n\");\n> > +#elif defined(SHA1_BLK)\n> > +\tstrbuf_addstr(buf, \"SHA-1: blk\\n\");\n> > +#elif defined(SHA1_APPLE)\n> > +\tstrbuf_addstr(buf, \"SHA-1: Apple CommonCrypto\\n\");\n> > +#elif defined(DC_SHA1_EXTERNAL)\n> > +\tstrbuf_addstr(buf, \"SHA-1: Collision Detection (External)\\n\");\n> > +#elif defined(DC_SHA1_SUBMODULE)\n> > +\tstrbuf_addstr(buf, \"SHA-1: Collision Detection (Submodule)\\n\");\n> > +#elif defined(SHA1_DC)\n> > +\tstrbuf_addstr(buf, \"SHA-1: Collision Detection\\n\");\n> > +#endif\n> > +\n> > +#if defined(SHA256_OPENSSL)\n> > +\tstrbuf_addstr(buf, \"SHA-256: OpenSSL\\n\");\n> > +#elif defined(SHA256_NETTLE)\n> > +\tstrbuf_addstr(buf, \"SHA-256: Nettle\\n\");\n> > +#elif defined(SHA256_GCRYPT)\n> > +\tstrbuf_addstr(buf, \"SHA-256: gcrypt\\n\");\n> > +#elif defined(SHA256_BLK)\n> > +\tstrbuf_addstr(buf, \"SHA-256: blk\\n\");\n> > +#endif\n> > +}\n> \n> While I agree with the objective of the change, I am not sure how I\n> feel about the implementation. Given that\n> \n> - The code here, and probably the existing code paths that depend\n> on these SHA1_$WHOSE symbols, assume that only one of them is\n> defined;\n> \n> - The \"git help --build-options\" is not an end-user thing but more\n> is a developer thing.\n> \n> The thing I am most worried about is that it is unclear how the\n> order in which the SHA1_$WHOSE symbols are inspected here and\n> elsewhere in the code are kept in sync. What happens when, for\n> example, SHA1_OPENSSL and SHA1_APPLE_UNSAFE are both defined? The\n> above code will report that we are using SHA1_OPENSSL, but hash.h\n> would probably use SHA1_APPLE as it has its own if/elif/endif\n> cascade.\n> \n> Perhaps it does not matter, if the build infrastructure ensures that\n> the build fails unless one and only one of SHA1_$WHOSE is defined.\n> \n> But with the way how this part is written with an if/elif/endif\n> cascade, it makes readers spend time wondering how the precedence\n> order here is kept in sync throughout the system. If I am not\n> mistaken, the top-level Makefile has its own ifdef/else/if/endif*\n> cascade.\n> \n> I imagine that making all of the above not if/elif/endif chain, but\n> make them pretend as if they are independent and orthogonal choices,\n> would make it simpler to understand and also it will help us catch a\n> misconfiguration where more than one is defined, i.e.\n> \n> static void get_sha_impl(struct strbuf *buf)\n> {\n> #if defined(SHA1_OPENSSL)\n> strbuf_addstr(buf, \"SHA-1: OpenSSL\\n\");\n> #endif\n> #if defined(SHA1_BLK)\n> strbuf_addstr(buf, \"SHA-1: blk\\n\");\n> #endif\n> #if defined(SHA1_APPLE)\n> ...\n> \n> \n> That way, we wouldn't force future devlopers who are plugging new\n> implementations of SHA-256 wonder where is the right place in the\n> existing if/elif/endif cascade their new one fits. It also allows\n> us to catch misconfigurations to define more then one of them at the\n> same time, if such a thing becomes ever possible.\n\nAnother option: we could ask the implementations themselves to define a\nsymbol `SHA1_BACKEND` and use it here. This would automatically ensure\nthat any implementation must define the symbol as we'd otherwise get a\ncompile error. We could also conditionally define `SHA1_UNSAFE_BACKEND`\ndepending on whether or not we have it.\n\nPatrick\n"},{"id":"515389","messageId":"qtnlhy36oqqewpb5fnzirfzwtyetpad64rxwgokclb4p6fbsxp@bg7gdukunxgu","threadId":"63212","inReplyTo":"xmqq8qoodq5u.fsf@gitster.g","subject":"Re: [PATCH 1/2] help: include SHA implementation in version info","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-03-31T17:21:09Z","receivedAt":"2025-03-31T17:24:56Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/03/29 04:36AM, Junio C Hamano wrote:\n> Justin Tobler writes:\n> \n> > When the `--build-options` flag is used with git-version(1), additional\n> > information about the built version of Git is printed. During build\n> > time, different SHA implementations may be configured, but this\n> > information is not included in the version info.\n> >\n> > Add the SHA implementations Git is built with to the version info.\n> > ...\n> > +static void get_sha_impl(struct strbuf *buf)\n> > +{\n> > +#if defined(SHA1_OPENSSL)\n> > +\tstrbuf_addstr(buf, \"SHA-1: OpenSSL\\n\");\n> > +#elif defined(SHA1_BLK)\n> > +\tstrbuf_addstr(buf, \"SHA-1: blk\\n\");\n> > +#elif defined(SHA1_APPLE)\n> > +\tstrbuf_addstr(buf, \"SHA-1: Apple CommonCrypto\\n\");\n> > +#elif defined(DC_SHA1_EXTERNAL)\n> > +\tstrbuf_addstr(buf, \"SHA-1: Collision Detection (External)\\n\");\n> > +#elif defined(DC_SHA1_SUBMODULE)\n> > +\tstrbuf_addstr(buf, \"SHA-1: Collision Detection (Submodule)\\n\");\n> > +#elif defined(SHA1_DC)\n> > +\tstrbuf_addstr(buf, \"SHA-1: Collision Detection\\n\");\n> > +#endif\n> > +\n> > +#if defined(SHA256_OPENSSL)\n> > +\tstrbuf_addstr(buf, \"SHA-256: OpenSSL\\n\");\n> > +#elif defined(SHA256_NETTLE)\n> > +\tstrbuf_addstr(buf, \"SHA-256: Nettle\\n\");\n> > +#elif defined(SHA256_GCRYPT)\n> > +\tstrbuf_addstr(buf, \"SHA-256: gcrypt\\n\");\n> > +#elif defined(SHA256_BLK)\n> > +\tstrbuf_addstr(buf, \"SHA-256: blk\\n\");\n> > +#endif\n> > +}\n> \n> While I agree with the objective of the change, I am not sure how I\n> feel about the implementation. Given that\n> \n> - The code here, and probably the existing code paths that depend\n> on these SHA1_$WHOSE symbols, assume that only one of them is\n> defined;\n> \n> - The \"git help --build-options\" is not an end-user thing but more\n> is a developer thing.\n> \n> The thing I am most worried about is that it is unclear how the\n> order in which the SHA1_$WHOSE symbols are inspected here and\n> elsewhere in the code are kept in sync. What happens when, for\n> example, SHA1_OPENSSL and SHA1_APPLE_UNSAFE are both defined? The\n> above code will report that we are using SHA1_OPENSSL, but hash.h\n> would probably use SHA1_APPLE as it has its own if/elif/endif\n> cascade.\n> \n> Perhaps it does not matter, if the build infrastructure ensures that\n> the build fails unless one and only one of SHA1_$WHOSE is defined.\n> \n> But with the way how this part is written with an if/elif/endif\n> cascade, it makes readers spend time wondering how the precedence\n> order here is kept in sync throughout the system. If I am not\n> mistaken, the top-level Makefile has its own ifdef/else/if/endif*\n> cascade.\n\nGood point! Both hash.h and the Makefile have their own precedence\ndefined which makes this even more confusing to understand what we would\nshould use to keep things in sync.\n\n> I imagine that making all of the above not if/elif/endif chain, but\n> make them pretend as if they are independent and orthogonal choices,\n> would make it simpler to understand and also it will help us catch a\n> misconfiguration where more than one is defined, i.e.\n> \n> static void get_sha_impl(struct strbuf *buf)\n> {\n> #if defined(SHA1_OPENSSL)\n> strbuf_addstr(buf, \"SHA-1: OpenSSL\\n\");\n> #endif\n> #if defined(SHA1_BLK)\n> strbuf_addstr(buf, \"SHA-1: blk\\n\");\n> #endif\n> #if defined(SHA1_APPLE)\n> ...\n> \n> \n> That way, we wouldn't force future devlopers who are plugging new\n> implementations of SHA-256 wonder where is the right place in the\n> existing if/elif/endif cascade their new one fits. It also allows\n> us to catch misconfigurations to define more then one of them at the\n> same time, if such a thing becomes ever possible.\n\nKeeping each of the options independent certainly keeps things more\nsimple and avoids having to also manage the precedence of each option\nhere. For most of these SHA related options we only expect one to be set\nat a time anyway. I'll do this in the next version.\n\nOne exception though is that we do expect that when SHA1_DC is set,\neither DC_SHA1_SUBMODULE or DC_SHA1_EXTERNAL may also be set. In this\nscenario, maybe it would be fine for the printed build options to\ninclude both. Another option would be to treat the DC_SHA1_* options\nseparately from the SHA1_* ones and have a separate prefix key. Maybe\nsomething like `DC-SHA-1: {DC_SHA1_SUBMODULE,DC_SHA1_EXTERNAL}`.\n\nI'm currently leaning towards the latter option here.\n\n> Also, wouldn't it make more sense to just reuse the internal symbol\n> for reporting, i.e.\n> \n> \tstrbuf_addstr(buf, \"SHA-1: SHA1_OPENSSL\\n\");\n> \n> instead of having to come up with \"human readable\" name here\n\nYa, using the internal symbol instead of a new human readable name is a\nbetter idea. I'll update in the next version.\n\nThanks,\n-Justin\n"},{"id":"515392","messageId":"sqoplw2bek4xngjaq2ss54jqcn6k5hvltric2en2niiqebjfth@txxq2ga3weud","threadId":"63212","inReplyTo":"Z-pCG9d7Rf9SMuXJ@pks.im","subject":"Re: [PATCH 1/2] help: include SHA implementation in version info","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-03-31T17:46:26Z","receivedAt":"2025-03-31T17:50:13Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/03/31 09:19AM, Patrick Steinhardt wrote:\n> On Sat, Mar 29, 2025 at 04:36:45AM -0700, Junio C Hamano wrote:\n> > While I agree with the objective of the change, I am not sure how I\n> > feel about the implementation. Given that\n> > \n> > - The code here, and probably the existing code paths that depend\n> > on these SHA1_$WHOSE symbols, assume that only one of them is\n> > defined;\n> > \n> > - The \"git help --build-options\" is not an end-user thing but more\n> > is a developer thing.\n> > \n> > The thing I am most worried about is that it is unclear how the\n> > order in which the SHA1_$WHOSE symbols are inspected here and\n> > elsewhere in the code are kept in sync. What happens when, for\n> > example, SHA1_OPENSSL and SHA1_APPLE_UNSAFE are both defined? The\n> > above code will report that we are using SHA1_OPENSSL, but hash.h\n> > would probably use SHA1_APPLE as it has its own if/elif/endif\n> > cascade.\n> > \n> > Perhaps it does not matter, if the build infrastructure ensures that\n> > the build fails unless one and only one of SHA1_$WHOSE is defined.\n> > \n> > But with the way how this part is written with an if/elif/endif\n> > cascade, it makes readers spend time wondering how the precedence\n> > order here is kept in sync throughout the system. If I am not\n> > mistaken, the top-level Makefile has its own ifdef/else/if/endif*\n> > cascade.\n> > \n> > I imagine that making all of the above not if/elif/endif chain, but\n> > make them pretend as if they are independent and orthogonal choices,\n> > would make it simpler to understand and also it will help us catch a\n> > misconfiguration where more than one is defined, i.e.\n> > \n> > static void get_sha_impl(struct strbuf *buf)\n> > {\n> > #if defined(SHA1_OPENSSL)\n> > strbuf_addstr(buf, \"SHA-1: OpenSSL\\n\");\n> > #endif\n> > #if defined(SHA1_BLK)\n> > strbuf_addstr(buf, \"SHA-1: blk\\n\");\n> > #endif\n> > #if defined(SHA1_APPLE)\n> > ...\n> > \n> > \n> > That way, we wouldn't force future devlopers who are plugging new\n> > implementations of SHA-256 wonder where is the right place in the\n> > existing if/elif/endif cascade their new one fits. It also allows\n> > us to catch misconfigurations to define more then one of them at the\n> > same time, if such a thing becomes ever possible.\n> \n> Another option: we could ask the implementations themselves to define a\n> symbol `SHA1_BACKEND` and use it here. This would automatically ensure\n> that any implementation must define the symbol as we'd otherwise get a\n> compile error. We could also conditionally define `SHA1_UNSAFE_BACKEND`\n> depending on whether or not we have it.\n\nThe SHA backends get selected in hash.h, so we could conditionally\ndefine symbol values based on the backend that gets selected there. This\nhas the benefit of centralizing backend selection in one place and the\nprinted build options could just depend on that.\n\nI'll implement this approach instead in the next version.\n\n-Justin\n"},{"id":"515395","messageId":"4mivlvpsppq62qrotv7dnrib3izxmhf6zlqga33dm6olvge4tz@z725ju4cshv5","threadId":"63212","inReplyTo":"CAP8UFD1oYmr=2ZCOVVjk6NbSCtA1Zv+3jy26jSSwHuOfdS6BQA@mail.gmail.com","subject":"Re: [PATCH 0/2] help: include SHA build options in version info","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-03-31T18:17:01Z","receivedAt":"2025-03-31T18:20:48Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/03/29 09:58AM, Christian Couder wrote:\n> On Fri, Mar 28, 2025 at 6:05 PM Justin Tobler wrote:\n> > Regarding \"unsafe-SHA-1\", I wonder if we should use a different name in\n> > the printed build options that sounds a little less scary. I was\n> > thinking maybe \"fast-SHA-1\" and document its meaning appropriately. I'm\n> > interested to know if anyone has thoughts on this.\n> \n> Maybe we could use just \"SHA-1\" if a single algorithm is used for\n> everything, and both \"SHA-1 for crypto\" and \"SHA-1 for non-crypto\"\n> otherwise.\n\nMaybe we could leave \"SHA-1:\" alone and when unsafe is set use\n\"non-crypto-SHA-1\" instead of \"unsafe-SHA-1\"? It's a little wordy, but\nprobably not too bad.\n\n> Related to this I wonder if we should warn in some ways if a non\n> collision detection algorithm is used for crypto. For example we could\n> print \"SHA-1: OpenSSL (No collision detection!!!)\" instead of just\n> \"SHA-1: OpenSSL\". And yeah that should be documented.\n\nElsewhere in this thread it was suggested that we use the internal\nsymbol names instead of coming up with new human readable names [1].\nI'll append this warning to the appropriate options as well though and\ndocument it.\n\nThanks,\n-Justin\n\n[1]: \n"},{"id":"515417","messageId":"xmqq1pucb4d9.fsf@gitster.g","threadId":"63212","inReplyTo":"Z-pCG9d7Rf9SMuXJ@pks.im","subject":"Re: [PATCH 1/2] help: include SHA implementation in version info","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-04-01T09:47:14Z","receivedAt":"2025-04-01T09:47:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> Another option: we could ask the implementations themselves to define a\n> symbol `SHA1_BACKEND` and use it here. This would automatically ensure\n> that any implementation must define the symbol as we'd otherwise get a\n> compile error. We could also conditionally define `SHA1_UNSAFE_BACKEND`\n> depending on whether or not we have it.\n\nMuch simpler and less error prone.\n"},{"id":"515466","messageId":"20250401203630.285451-1-jltobler@gmail.com","threadId":"63212","inReplyTo":"20250328170121.157563-1-jltobler@gmail.com","subject":"[PATCH v2 0/2] help: include SHA build options in version info","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-04-01T20:36:28Z","receivedAt":"2025-04-01T20:40:23Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"Greetings,\n\nAdditional information regarding how Git was built can be found via the\n`--build-options` flag for git-version(1). This currently does not\ninclude information about the SHA-1 and SHA-256 implementations Git is\nbuilt with.\n\nThis short series adds build option info for the SHA-1, SHA-256, and\nnon-crypto-SHA-1 (if any) implementations which may be useful for\ndiagnostic purposes\n\nChanges since V1:\n\n - Each SHA backend is expected to define either `SHA1_BACKEND`,\n `SHA1_UNSAFE_BACKEND`, or `SHA256_BACKEND` as appropriate.\n These symbols are then used to print the SHA build options in\n the additional version info.\n\n - The names of the build options are used instead of\n human-readable names.\n\n - Appended \"(No collision detection)\" to warn about SHA1\n backends without collision detection.\n\n - Renamed \"unsafe-SHA-1\" to \"non-crypto-SHA-1\" in the printed\n build options.\n\n - Small updates to documentation.\n\nThanks,\n-Justin\n\nJustin Tobler (2):\n help: include SHA implementation in version info\n help: include unsafe SHA-1 build info in version\n\n Documentation/git-version.adoc | 6 ++++++\n hash.h | 11 +++++++++++\n help.c | 13 +++++++++++++\n 3 files changed, 30 insertions(+)\n\n\nbase-commit: 683c54c999c301c2cd6f715c411407c413b1d84e\n-- \n2.49.0\n\n"},{"id":"515467","messageId":"20250401203630.285451-2-jltobler@gmail.com","threadId":"63212","inReplyTo":"20250401203630.285451-1-jltobler@gmail.com","subject":"[PATCH v2 1/2] help: include SHA implementation in version info","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-04-01T20:36:29Z","receivedAt":"2025-04-01T20:40:25Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"When the `--build-options` flag is used with git-version(1), additional\ninformation about the built version of Git is printed. During build\ntime, different SHA implementations may be configured, but this\ninformation is not included in the version info.\n\nAdd the SHA implementations Git is built with to the version info by\nrequiring each backend to define a SHA1_BACKEND or SHA256_BACKEND symbol\nas appropriate and use the value in the printed build options.\n\nSigned-off-by: Justin Tobler \n---\n Documentation/git-version.adoc | 3 +++\n hash.h | 8 ++++++++\n help.c | 8 ++++++++\n 3 files changed, 19 insertions(+)\n\ndiff --git a/Documentation/git-version.adoc b/Documentation/git-version.adoc\nindex 80fa7754a6..f06758a7cf 100644\n--- a/Documentation/git-version.adoc\n+++ b/Documentation/git-version.adoc\n@@ -22,6 +22,9 @@ OPTIONS\n --build-options::\n \tInclude additional information about how git was built for diagnostic\n \tpurposes.\n++\n+Note that the SHA1 options `SHA1_APPLE`, `SHA1_OPENSSL`, and `SHA1_BLK` do not\n+have collision detection.\n \n GIT\n ---\ndiff --git a/hash.h b/hash.h\nindex 4367acfec5..51cd0ec7b6 100644\n--- a/hash.h\n+++ b/hash.h\n@@ -2,16 +2,20 @@\n #define HASH_H\n \n #if defined(SHA1_APPLE)\n+#define SHA1_BACKEND \"SHA1_APPLE (No collision detection)\"\n #include \n #elif defined(SHA1_OPENSSL)\n+# define SHA1_BACKEND \"SHA1_OPENSSL (No collision detection)\"\n # include \n # if defined(OPENSSL_API_LEVEL) && OPENSSL_API_LEVEL >= 3\n # define SHA1_NEEDS_CLONE_HELPER\n # include \"sha1/openssl.h\"\n # endif\n #elif defined(SHA1_DC)\n+#define SHA1_BACKEND \"SHA1_DC\"\n #include \"sha1dc_git.h\"\n #else /* SHA1_BLK */\n+#define SHA1_BACKEND \"SHA1_BLK (No collision detection)\"\n #include \"block-sha1/sha1.h\"\n #endif\n \n@@ -46,17 +50,21 @@\n #endif\n \n #if defined(SHA256_NETTLE)\n+#define SHA256_BACKEND \"SHA256_NETTLE\"\n #include \"sha256/nettle.h\"\n #elif defined(SHA256_GCRYPT)\n+#define SHA256_BACKEND \"SHA256_GCRYPT\"\n #define SHA256_NEEDS_CLONE_HELPER\n #include \"sha256/gcrypt.h\"\n #elif defined(SHA256_OPENSSL)\n+# define SHA256_BACKEND \"SHA256_OPENSSL\"\n # include \n # if defined(OPENSSL_API_LEVEL) && OPENSSL_API_LEVEL >= 3\n # define SHA256_NEEDS_CLONE_HELPER\n # include \"sha256/openssl.h\"\n # endif\n #else\n+#define SHA256_BACKEND \"SHA256_BLK\"\n #include \"sha256/block/sha256.h\"\n #endif\n \ndiff --git a/help.c b/help.c\nindex c54bd9918a..3aebfb3681 100644\n--- a/help.c\n+++ b/help.c\n@@ -9,6 +9,7 @@\n #include \"run-command.h\"\n #include \"levenshtein.h\"\n #include \"gettext.h\"\n+#include \"hash.h\"\n #include \"help.h\"\n #include \"command-list.h\"\n #include \"string-list.h\"\n@@ -768,6 +769,12 @@ char *help_unknown_cmd(const char *cmd)\n \texit(1);\n }\n \n+static void get_sha_impl(struct strbuf *buf)\n+{\n+\tstrbuf_addf(buf, \"SHA-1: %s\\n\", SHA1_BACKEND);\n+\tstrbuf_addf(buf, \"SHA-256: %s\\n\", SHA256_BACKEND);\n+}\n+\n void get_version_info(struct strbuf *buf, int show_build_options)\n {\n \t/*\n@@ -803,6 +810,7 @@ void get_version_info(struct strbuf *buf, int show_build_options)\n #elif defined ZLIB_VERSION\n \t\tstrbuf_addf(buf, \"zlib: %s\\n\", ZLIB_VERSION);\n #endif\n+\t\tget_sha_impl(buf);\n \t}\n }\n \n-- \n2.49.0\n\n"},{"id":"515468","messageId":"20250401203630.285451-3-jltobler@gmail.com","threadId":"63212","inReplyTo":"20250401203630.285451-1-jltobler@gmail.com","subject":"[PATCH v2 2/2] help: include unsafe SHA-1 build info in version","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-04-01T20:36:30Z","receivedAt":"2025-04-01T20:40:25Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"In 06c92dafb8 (Makefile: allow specifying a SHA-1 for non-cryptographic\nuses, 2024-09-26), support for unsafe SHA-1 is added. Add the unsafe\nSHA-1 build info to `git version --build-info` and update corresponding\ndocumentation.\n\nSigned-off-by: Justin Tobler \n---\n Documentation/git-version.adoc | 3 +++\n hash.h | 3 +++\n help.c | 5 +++++\n 3 files changed, 11 insertions(+)\n\ndiff --git a/Documentation/git-version.adoc b/Documentation/git-version.adoc\nindex f06758a7cf..753794988c 100644\n--- a/Documentation/git-version.adoc\n+++ b/Documentation/git-version.adoc\n@@ -25,6 +25,9 @@ OPTIONS\n +\n Note that the SHA1 options `SHA1_APPLE`, `SHA1_OPENSSL`, and `SHA1_BLK` do not\n have collision detection.\n++\n+If built to use a faster SHA-1 implementation for non-cryptographic purposes,\n+that implementation is denoted as \"non-crypto-SHA-1\".\n \n GIT\n ---\ndiff --git a/hash.h b/hash.h\nindex 51cd0ec7b6..72334d3506 100644\n--- a/hash.h\n+++ b/hash.h\n@@ -20,12 +20,14 @@\n #endif\n \n #if defined(SHA1_APPLE_UNSAFE)\n+# define SHA1_UNSAFE_BACKEND \"SHA1_APPLE_UNSAFE\"\n # include \n # define platform_SHA_CTX_unsafe CC_SHA1_CTX\n # define platform_SHA1_Init_unsafe CC_SHA1_Init\n # define platform_SHA1_Update_unsafe CC_SHA1_Update\n # define platform_SHA1_Final_unsafe CC_SHA1_Final\n #elif defined(SHA1_OPENSSL_UNSAFE)\n+# define SHA1_UNSAFE_BACKEND \"SHA1_OPENSSL_UNSAFE\"\n # include \n # if defined(OPENSSL_API_LEVEL) && OPENSSL_API_LEVEL >= 3\n # define SHA1_NEEDS_CLONE_HELPER_UNSAFE\n@@ -42,6 +44,7 @@\n # define platform_SHA1_Final_unsafe SHA1_Final\n # endif\n #elif defined(SHA1_BLK_UNSAFE)\n+# define SHA1_UNSAFE_BACKEND \"SHA1_BLK_UNSAFE\"\n # include \"block-sha1/sha1.h\"\n # define platform_SHA_CTX_unsafe blk_SHA_CTX\n # define platform_SHA1_Init_unsafe blk_SHA1_Init\ndiff --git a/help.c b/help.c\nindex 3aebfb3681..1238a962b0 100644\n--- a/help.c\n+++ b/help.c\n@@ -772,6 +772,11 @@ char *help_unknown_cmd(const char *cmd)\n static void get_sha_impl(struct strbuf *buf)\n {\n \tstrbuf_addf(buf, \"SHA-1: %s\\n\", SHA1_BACKEND);\n+\n+#if defined(SHA1_UNSAFE_BACKEND)\n+\tstrbuf_addf(buf, \"non-crypto-SHA-1: %s\\n\", SHA1_UNSAFE_BACKEND);\n+#endif\n+\n \tstrbuf_addf(buf, \"SHA-256: %s\\n\", SHA256_BACKEND);\n }\n \n-- \n2.49.0\n\n"},{"id":"515486","messageId":"Z-zpY3D01Fg4q0Je@pks.im","threadId":"63212","inReplyTo":"20250401203630.285451-2-jltobler@gmail.com","subject":"Re: [PATCH v2 1/2] help: include SHA implementation in version info","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-04-02T07:38:11Z","receivedAt":"2025-04-02T07:38:19Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Apr 01, 2025 at 03:36:29PM -0500, Justin Tobler wrote:\n> diff --git a/Documentation/git-version.adoc b/Documentation/git-version.adoc\n> index 80fa7754a6..f06758a7cf 100644\n> --- a/Documentation/git-version.adoc\n> +++ b/Documentation/git-version.adoc\n> @@ -22,6 +22,9 @@ OPTIONS\n> --build-options::\n> \tInclude additional information about how git was built for diagnostic\n> \tpurposes.\n> ++\n> +Note that the SHA1 options `SHA1_APPLE`, `SHA1_OPENSSL`, and `SHA1_BLK` do not\n> +have collision detection.\n> \n> GIT\n> ---\n\nI think this note is somewhat funny for an unsuspecting reader. On the\none hand they're going to be puzzled why you're talking about SHA1 in\nthe first place because it isn't mentioned at all beforehand. And on the\nother hand they will wonder what collision detection even is in the\nfirst place.\n\nSo I would either drop this paragraph completely or expand it to give a\nbit more context.\n\n> diff --git a/hash.h b/hash.h\n> index 4367acfec5..51cd0ec7b6 100644\n> --- a/hash.h\n> +++ b/hash.h\n> @@ -2,16 +2,20 @@\n> #define HASH_H\n> \n> #if defined(SHA1_APPLE)\n> +#define SHA1_BACKEND \"SHA1_APPLE (No collision detection)\"\n> #include \n> #elif defined(SHA1_OPENSSL)\n> +# define SHA1_BACKEND \"SHA1_OPENSSL (No collision detection)\"\n> # include \n> # if defined(OPENSSL_API_LEVEL) && OPENSSL_API_LEVEL >= 3\n> # define SHA1_NEEDS_CLONE_HELPER\n> # include \"sha1/openssl.h\"\n> # endif\n> #elif defined(SHA1_DC)\n> +#define SHA1_BACKEND \"SHA1_DC\"\n> #include \"sha1dc_git.h\"\n> #else /* SHA1_BLK */\n> +#define SHA1_BACKEND \"SHA1_BLK (No collision detection)\"\n> #include \"block-sha1/sha1.h\"\n> #endif\n> \n\nThis feels way less fragile indeed, thanks for adapting.\n\n> diff --git a/help.c b/help.c\n> index c54bd9918a..3aebfb3681 100644\n> --- a/help.c\n> +++ b/help.c\n> @@ -768,6 +769,12 @@ char *help_unknown_cmd(const char *cmd)\n> \texit(1);\n> }\n> \n> +static void get_sha_impl(struct strbuf *buf)\n> +{\n> +\tstrbuf_addf(buf, \"SHA-1: %s\\n\", SHA1_BACKEND);\n> +\tstrbuf_addf(buf, \"SHA-256: %s\\n\", SHA256_BACKEND);\n> +}\n> +\n> void get_version_info(struct strbuf *buf, int show_build_options)\n> {\n> \t/*\n> @@ -803,6 +810,7 @@ void get_version_info(struct strbuf *buf, int show_build_options)\n> #elif defined ZLIB_VERSION\n> \t\tstrbuf_addf(buf, \"zlib: %s\\n\", ZLIB_VERSION);\n> #endif\n> +\t\tget_sha_impl(buf);\n\nI don't quite see the need for a new function, but don't mind it too\nmuch, either.\n\nPatrick\n"},{"id":"515487","messageId":"Z-zpau_DsOGVVtjj@pks.im","threadId":"63212","inReplyTo":"20250401203630.285451-3-jltobler@gmail.com","subject":"Re: [PATCH v2 2/2] help: include unsafe SHA-1 build info in version","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-04-02T07:38:18Z","receivedAt":"2025-04-02T07:38:21Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Apr 01, 2025 at 03:36:30PM -0500, Justin Tobler wrote:\n> diff --git a/Documentation/git-version.adoc b/Documentation/git-version.adoc\n> index f06758a7cf..753794988c 100644\n> --- a/Documentation/git-version.adoc\n> +++ b/Documentation/git-version.adoc\n> @@ -25,6 +25,9 @@ OPTIONS\n> +\n> Note that the SHA1 options `SHA1_APPLE`, `SHA1_OPENSSL`, and `SHA1_BLK` do not\n> have collision detection.\n> ++\n> +If built to use a faster SHA-1 implementation for non-cryptographic purposes,\n> +that implementation is denoted as \"non-crypto-SHA-1\".\n> \n> GIT\n> ---\n\nI got basically the same comment for this new paragraph as for the first\none. I'd either drop it or expand it so that readers know what's going\non.\n\n> diff --git a/help.c b/help.c\n> index 3aebfb3681..1238a962b0 100644\n> --- a/help.c\n> +++ b/help.c\n> @@ -772,6 +772,11 @@ char *help_unknown_cmd(const char *cmd)\n> static void get_sha_impl(struct strbuf *buf)\n> {\n> \tstrbuf_addf(buf, \"SHA-1: %s\\n\", SHA1_BACKEND);\n> +\n> +#if defined(SHA1_UNSAFE_BACKEND)\n> +\tstrbuf_addf(buf, \"non-crypto-SHA-1: %s\\n\", SHA1_UNSAFE_BACKEND);\n> +#endif\n> +\n\nShould we maybe print the equivalent of \"none\" in case no unsafe backend\nwas selected?\n\nI also think we shouldn't name this \"non-crypto\". The backend still is\nSHA1, which is a proper cryptogtaphic hash function. It may be somewhat\nbroken nowadays, but that doesn't change the fact that it's a\ncryptographic primitive.\n\nHow about we rename this to \"SHA-1 without collision detection:\"?\n\nPatrick\n"},{"id":"515514","messageId":"CAP8UFD2UpEpqBYxqeo=vkDHwWdoO9v2RPVtnUBSOUS5MOK6n0Q@mail.gmail.com","threadId":"63212","inReplyTo":"Z-zpY3D01Fg4q0Je@pks.im","subject":"Re: [PATCH v2 1/2] help: include SHA implementation in version info","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-04-02T11:26:31Z","receivedAt":"2025-04-02T11:26:46Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Apr 2, 2025 at 9:38 AM Patrick Steinhardt wrote:\n>\n> On Tue, Apr 01, 2025 at 03:36:29PM -0500, Justin Tobler wrote:\n> > diff --git a/Documentation/git-version.adoc b/Documentation/git-version.adoc\n> > index 80fa7754a6..f06758a7cf 100644\n> > --- a/Documentation/git-version.adoc\n> > +++ b/Documentation/git-version.adoc\n> > @@ -22,6 +22,9 @@ OPTIONS\n> > --build-options::\n> > Include additional information about how git was built for diagnostic\n> > purposes.\n> > ++\n> > +Note that the SHA1 options `SHA1_APPLE`, `SHA1_OPENSSL`, and `SHA1_BLK` do not\n> > +have collision detection.\n>\n> I think this note is somewhat funny for an unsuspecting reader. On the\n> one hand they're going to be puzzled why you're talking about SHA1 in\n> the first place because it isn't mentioned at all beforehand. And on the\n> other hand they will wonder what collision detection even is in the\n> first place.\n>\n> So I would either drop this paragraph completely or expand it to give a\n> bit more context.\n\nYeah, I think it's worth giving more information, like perhaps:\n\n\"For the libraries used to implement the SHA-1 and SHA-2 algorithms\nonly symbolic information, like `SHA-1: SHA1_APPLE` or `SHA-256:\nSHA256_NETTLE` is displayed. Note that the SHA1 options `SHA1_APPLE`,\n`SHA1_OPENSSL`, and `SHA1_BLK` mean that no collision detection\nalgorithm is used, so known SHA-1 attacks might be possible, see\nhttps://en.wikipedia.org/wiki/SHA-1.\"\n"},{"id":"515515","messageId":"CAP8UFD3dgjzj-B4iY9AfnYnZFA2v_MQW+bWG-DXrweZDKb=N9A@mail.gmail.com","threadId":"63212","inReplyTo":"CAP8UFD2UpEpqBYxqeo=vkDHwWdoO9v2RPVtnUBSOUS5MOK6n0Q@mail.gmail.com","subject":"Re: [PATCH v2 1/2] help: include SHA implementation in version info","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-04-02T11:27:54Z","receivedAt":"2025-04-02T11:28:08Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Apr 2, 2025 at 1:26 PM Christian Couder\n wrote:\n>\n> On Wed, Apr 2, 2025 at 9:38 AM Patrick Steinhardt wrote:\n> >\n> > On Tue, Apr 01, 2025 at 03:36:29PM -0500, Justin Tobler wrote:\n> > > diff --git a/Documentation/git-version.adoc b/Documentation/git-version.adoc\n> > > index 80fa7754a6..f06758a7cf 100644\n> > > --- a/Documentation/git-version.adoc\n> > > +++ b/Documentation/git-version.adoc\n> > > @@ -22,6 +22,9 @@ OPTIONS\n> > > --build-options::\n> > > Include additional information about how git was built for diagnostic\n> > > purposes.\n> > > ++\n> > > +Note that the SHA1 options `SHA1_APPLE`, `SHA1_OPENSSL`, and `SHA1_BLK` do not\n> > > +have collision detection.\n> >\n> > I think this note is somewhat funny for an unsuspecting reader. On the\n> > one hand they're going to be puzzled why you're talking about SHA1 in\n> > the first place because it isn't mentioned at all beforehand. And on the\n> > other hand they will wonder what collision detection even is in the\n> > first place.\n> >\n> > So I would either drop this paragraph completely or expand it to give a\n> > bit more context.\n>\n> Yeah, I think it's worth giving more information, like perhaps:\n>\n> \"For the libraries used to implement the SHA-1 and SHA-2 algorithms\n\ns/SHA-2/SHA-256/\n\n> only symbolic information, like `SHA-1: SHA1_APPLE` or `SHA-256:\n> SHA256_NETTLE` is displayed. Note that the SHA1 options `SHA1_APPLE`,\n> `SHA1_OPENSSL`, and `SHA1_BLK` mean that no collision detection\n> algorithm is used, so known SHA-1 attacks might be possible, see\n> https://en.wikipedia.org/wiki/SHA-1.\"\n"},{"id":"515521","messageId":"rqytnnncju5pezkfidhd7u7vtaaop5fga7f5nyzwa74eiksezs@4t46ypevaiga","threadId":"63212","inReplyTo":"CAP8UFD2UpEpqBYxqeo=vkDHwWdoO9v2RPVtnUBSOUS5MOK6n0Q@mail.gmail.com","subject":"Re: [PATCH v2 1/2] help: include SHA implementation in version info","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-04-02T14:56:24Z","receivedAt":"2025-04-02T15:00:13Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/04/02 01:26PM, Christian Couder wrote:\n> On Wed, Apr 2, 2025 at 9:38 AM Patrick Steinhardt wrote:\n> >\n> > On Tue, Apr 01, 2025 at 03:36:29PM -0500, Justin Tobler wrote:\n> > > diff --git a/Documentation/git-version.adoc b/Documentation/git-version.adoc\n> > > index 80fa7754a6..f06758a7cf 100644\n> > > --- a/Documentation/git-version.adoc\n> > > +++ b/Documentation/git-version.adoc\n> > > @@ -22,6 +22,9 @@ OPTIONS\n> > > --build-options::\n> > > Include additional information about how git was built for diagnostic\n> > > purposes.\n> > > ++\n> > > +Note that the SHA1 options `SHA1_APPLE`, `SHA1_OPENSSL`, and `SHA1_BLK` do not\n> > > +have collision detection.\n> >\n> > I think this note is somewhat funny for an unsuspecting reader. On the\n> > one hand they're going to be puzzled why you're talking about SHA1 in\n> > the first place because it isn't mentioned at all beforehand. And on the\n> > other hand they will wonder what collision detection even is in the\n> > first place.\n> >\n> > So I would either drop this paragraph completely or expand it to give a\n> > bit more context.\n> \n> Yeah, I think it's worth giving more information, like perhaps:\n> \n> \"For the libraries used to implement the SHA-1 and SHA-2 algorithms\n> only symbolic information, like `SHA-1: SHA1_APPLE` or `SHA-256:\n> SHA256_NETTLE` is displayed. Note that the SHA1 options `SHA1_APPLE`,\n> `SHA1_OPENSSL`, and `SHA1_BLK` mean that no collision detection\n> algorithm is used, so known SHA-1 attacks might be possible, see\n> https://en.wikipedia.org/wiki/SHA-1.\"\n\nYa the documentation here should be expanded to provide some more\ncontext. I'll adapt in my next version to something like this:\n\n\"The libraries used to implement the SHA-1 and SHA-256 algorithms are\ndisplayed in the form `SHA-1: