gitgitgadget / git

GitGitGadget's Git fork. Open Pull Requests here to submit them to the Git mailing list
https://gitgitgadget.github.io/
Other
222 stars 133 forks source link

cmake: generalize the handling of the `UNIT_TEST_OBJS` list #1797

Closed dscho closed 2 months ago

dscho commented 2 months ago

This is an add-on for ps/reftable-exclude to let it build with CMake and Visual C.

This patch on its own does not actually fix vs-build; The patch "cmake: stop looking for REFTABLE_TEST_OBJS in the Makefile" which I submitted in https://lore.kernel.org/git/pull.1796.git.1726687400286.gitgitgadget@gmail.com is also needed for a successful vs-build.

dscho commented 2 months ago

/submit

gitgitgadget[bot] commented 2 months ago

Submitted as pull.1797.git.1726687769585.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1797/dscho/reftable-exclude+cmake-v1

To fetch this version to local tag pr-1797/dscho/reftable-exclude+cmake-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1797/dscho/reftable-exclude+cmake-v1
gitgitgadget[bot] commented 2 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/35b7d19b5f45aa4ad8f873af66f3fac56e5cfc56.

gitgitgadget[bot] commented 2 months ago

This branch is now known as jc/cmake-unit-test-updates.

gitgitgadget[bot] commented 2 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/2c4e8a806a9814c06d2c76ccd6efe08a95161e7d.

gitgitgadget[bot] commented 2 months ago

This patch series was integrated into next via https://github.com/git/git/commit/d892dcdcdd656c16370b586a56702de7f4047d07.

gitgitgadget[bot] commented 2 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/a452b348de9c49f924b1774fc1da39e034febd67.

gitgitgadget[bot] commented 2 months ago

There was a status update in the "Cooking" section about the branch jc/cmake-unit-test-updates on the Git mailing list:

CMake adjustments for recent changes around unit tests.

Will merge to 'master'.
source: <pull.1797.git.1726687769585.gitgitgadget@gmail.com>
gitgitgadget[bot] commented 2 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/9a970bd1e03cae2318d2c0182f4a1ba71cbccf51.

gitgitgadget[bot] commented 2 months ago

There was a status update in the "Cooking" section about the branch jc/cmake-unit-test-updates on the Git mailing list:

CMake adjustments for recent changes around unit tests.

Will merge to 'master'.
source: <pull.1797.git.1726687769585.gitgitgadget@gmail.com>
gitgitgadget[bot] commented 2 months ago

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> With a4f50bb1e9b (t/unit-tests: introduce reftable library, 2024-09-16),
> however, the `UNIT_TEST_OBJS` list became a trio, and the CMake
> definition has to be adjusted again. Now that we can use the
> `parse_makefile_for_sources()` function without many complications,
> let's do that.

Am I correct to understand that it is not "trio"-ness (has three
things) that makes the approach feasible, but the fact that all
three things are concrete values?

The longer-term aspiration is to migrate everything to clar-based
unit tests, so the list will hopefully keep getting shorter and then
become empty (https://lore.kernel.org/git/Zt_lLsnylKJ9uoqj@pks.im/).

Will queue.  Thanks.
gitgitgadget[bot] commented 2 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/1050870ecc02b4f155b6c2ba91d1726fb293ce71.

gitgitgadget[bot] commented 2 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/cbb5b53a9c0e6bbe6f8df2dd371735ecbc710fe7.

gitgitgadget[bot] commented 2 months ago

This patch series was integrated into master via https://github.com/git/git/commit/cbb5b53a9c0e6bbe6f8df2dd371735ecbc710fe7.

gitgitgadget[bot] commented 2 months ago

This patch series was integrated into next via https://github.com/git/git/commit/cbb5b53a9c0e6bbe6f8df2dd371735ecbc710fe7.

gitgitgadget[bot] commented 2 months ago

Closed via cbb5b53a9c0e6bbe6f8df2dd371735ecbc710fe7.

gitgitgadget[bot] commented 2 months ago

On the Git mailing list, Johannes Schindelin wrote (reply to this):


Hi,

On Mon, 23 Sep 2024, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > With a4f50bb1e9b (t/unit-tests: introduce reftable library, 2024-09-16),
> > however, the `UNIT_TEST_OBJS` list became a trio, and the CMake
> > definition has to be adjusted again. Now that we can use the
> > `parse_makefile_for_sources()` function without many complications,
> > let's do that.
>
> Am I correct to understand that it is not "trio"-ness (has three
> things) that makes the approach feasible, but the fact that all
> three things are concrete values?

No, the fact that a third entry was added made it obvious that adding
hard-coded equivalents to the CMake definition is too much of a
whac-a-mole thing and that the `Makefile` parsing approach would be
preferable. With just two items, it was still kind of okay to duplicate
the list, but three is a trend.

> The longer-term aspiration is to migrate everything to clar-based
> unit tests, so the list will hopefully keep getting shorter and then
> become empty (https://lore.kernel.org/git/Zt_lLsnylKJ9uoqj@pks.im/).

Once it becomes empty, I will have to send another CMake-related patch
similar to when the `REFTABLE_TEST_OBJS` list went away, as the CMake
definition cannot handle such empty lists.

Ciao,
Johannes
gitgitgadget[bot] commented 2 months ago

On the Git mailing list, Junio C Hamano wrote (reply to this):

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> The longer-term aspiration is to migrate everything to clar-based
>> unit tests, so the list will hopefully keep getting shorter and then
>> become empty (https://lore.kernel.org/git/Zt_lLsnylKJ9uoqj@pks.im/).
>
> Once it becomes empty, I will have to send another CMake-related patch
> similar to when the `REFTABLE_TEST_OBJS` list went away, as the CMake
> definition cannot handle such empty lists.

It may be ideal if all contributors learned and updated inputs to
all the build process options like cmake, make, and autoconf at the
same time as they futz with the code they build (in other words, a
change that makes such a list to empty includes cmake update), but
different people chipping in with help in their strong areas and
polishing the system as a whole as a collaborative effort is
probably the best practical alternative.

Thanks.