Re: [PATCH v2 1/5] Makefile: wire up build option for deprecated features

From: Christian Couder <christian.couder@gmail.com>
Date: Jan 6, 2025, 13:20 UTC
Message-ID: <CAP8UFD1wM29BFX7ndh905dX8aDF_vdG_azf9phnY+Cgutgfj_Q@mail.gmail.com>
In-Reply-To: <20250106-pks-remote-branches-deprecation-v2-1-2ce87c053536@pks.im>

On Mon, Jan 6, 2025 at 8:51 AM Patrick Steinhardt <ps@pks.im> wrote:

Show 12 quoted lines

>
> With 57ec9254eb (docs: introduce document to announce breaking changes,
> 2024-06-14), we have introduced a new document that tracks upcoming
> breaking changes in the Git project. In 2454970930 (BreakingChanges:
> early adopter option, 2024-10-11) we have amended the document a bit to
> mention that any introduced breaking changes must be accompanied by
> logic that allows us to enable the breaking change at compile-time.
> While we already have two breaking changes lined up, neither of them has
> such a switch because they predate those instructions.
>
> Introduce the proposed `WITH_BREAKING_CHANGES` preprocessor macro and
> wire it up with both our Makefiles and Meson.

It's not clear from the above if the two already lined up breaking changes are going to use the new build option in this patch, in a following patch or in a future patch series after this one. Let's see...

[...]

Show 9 quoted lines

> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index 49904ca8a93981c514540bad5efa6833ddd14426..63d008892848c20d5937d9a624a480f700b19498 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -1198,6 +1198,7 @@ string(REPLACE "@GITWEBDIR@" "'${GITWEBDIR}'" git_build_options "${git_build_opt
>  string(REPLACE "@USE_GETTEXT_SCHEME@" "" git_build_options "${git_build_options}")
>  string(REPLACE "@LOCALEDIR@" "'${LOCALEDIR}'" git_build_options "${git_build_options}")
>  string(REPLACE "@BROKEN_PATH_FIX@" "" git_build_options "${git_build_options}")
> +string(REPLACE "@WITH_BREAKING_CHANGES@" "" git_build_options "${git_build_options}")

The commit message says that the new build option is wired up with both our Makefiles and Meson, so I didn't expect it to be also wired up with CMake, but it looks like it is.

[...]

Show 11 quoted lines

> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 62dfcc4aaf959d0cf066d07663d939e14f92485c..6e423f655d35adf5a2d4f8b3a78d9e8c1119caab 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -1864,6 +1864,10 @@ test_lazy_prereq CURL '
>         curl --version
>  '
>
> +test_lazy_prereq WITHOUT_BREAKING_CHANGES '
> +       test -z "$WITH_BREAKING_CHANGES"
> +'

Not expected from reading the commit message, but nice to have too.