{"thread":{"id":"58976","subject":"[PATCH] cmake: don't invoke msgfmt with --statistics","startedAt":"2022-12-19T10:27:05Z","lastAt":"2022-12-27T22:33:08Z","messageCount":5,"participants":["Ævar Arnfjörð Bjarmason","Phillip Wood","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"469275","messageId":"patch-1.1-0fa41115261-20221219T102205Z-avarab@gmail.com","threadId":"58976","inReplyTo":null,"subject":"[PATCH] cmake: don't invoke msgfmt with --statistics","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-19T10:26:46Z","receivedAt":"2022-12-19T10:27:05Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"In [1] I made the same change to our Makefile, let's follow-up and do\nthe same here.\n\nFor \"cmake\" this is particularly nice with \"-G Ninja\", as before we'd\nemit ~40 lines of overflowed progress bar output, but now it's only\nthe one line of \"ninja\"'s progress bar.\n\n1. 2f12b31b746 (Makefile: don't invoke msgfmt with --statistics,\n   2021-12-17)\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n\nThis trivial fix is extracted from the ab/cmake-nix-and-ci topic which\nwas ejected around the time of the release for that previous\nsubmission see [1], the range-diff is to that topic.\n\nI'm re-arranging and re-submitting topic more piecemeal. There were no\noutstanding issues or feedback with this part of it, so hopefully this\ncan advance relatively quickly.\n\nI'll also submit some of the other uncontroversial bits today\nindependently, none of which conflict with one another. Then once\nthose have landed try to find some acceptable way forward for the\nlater bits, which at that point will be easier to review.\n\n1. https://lore.kernel.org/git/cover-v6-00.15-00000000000-20221206T001617Z-avarab@gmail.com/\n\nRange-diff:\n 1:  fc190b379cd =  1:  0fa41115261 cmake: don't invoke msgfmt with --statistics\n 2:  1a11aa233a3 <  -:  ----------- cmake: use \"-S\" and \"-B\" to specify source and build directories\n 3:  b9ddb5db1d3 <  -:  ----------- cmake: update instructions for portable CMakeLists.txt\n 4:  7b7850c00ee <  -:  ----------- cmake: don't copy chainlint.pl to build directory\n 5:  82ecb797915 <  -:  ----------- cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4\n 6:  1f326944a07 <  -:  ----------- cmake & test-lib.sh: add a $GIT_SOURCE_DIR variable\n 7:  973c8038f54 <  -:  ----------- cmake: set \"USE_LIBPCRE2\" in \"GIT-BUILD-OPTIONS\" for test-lib.sh\n 8:  b8448c7a8a6 <  -:  ----------- Makefile + test-lib.sh: don't prefer cmake-built to make-built git\n 9:  5135e40969e <  -:  ----------- test-lib.sh: support a \"GIT_TEST_BUILD_DIR\"\n10:  65204463730 <  -:  ----------- cmake: optionally be able to run tests before \"ctest\"\n11:  e25992b16f1 <  -:  ----------- cmake: support GIT_TEST_OPTS, abstract away WIN32 defaults\n12:  4905ce5321d <  -:  ----------- cmake: increase test timeout on Windows only\n13:  6c6b530965d <  -:  ----------- cmake: only look for \"sh\" in \"C:/Program Files\" on Windows\n14:  563f1b9b045 <  -:  ----------- cmake: copy over git-p4.py for t983[56] perforce test\n15:  917a884eb65 <  -:  ----------- CI: add a \"linux-cmake-test\" to run cmake & ctest on linux\n\n contrib/buildsystems/CMakeLists.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 2f6e0197ffa..8f8b6f375f7 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -897,7 +897,7 @@ if(MSGFMT_EXE)\n \tforeach(po ${po_files})\n \t\tfile(MAKE_DIRECTORY ${CMAKE_BINARY_DIR}/po/build/locale/${po}/LC_MESSAGES)\n \t\tadd_custom_command(OUTPUT ${CMAKE_BINARY_DIR}/po/build/locale/${po}/LC_MESSAGES/git.mo\n-\t\t\t\tCOMMAND ${MSGFMT_EXE} --check --statistics -o ${CMAKE_BINARY_DIR}/po/build/locale/${po}/LC_MESSAGES/git.mo ${CMAKE_SOURCE_DIR}/po/${po}.po)\n+\t\t\t\tCOMMAND ${MSGFMT_EXE} --check -o ${CMAKE_BINARY_DIR}/po/build/locale/${po}/LC_MESSAGES/git.mo ${CMAKE_SOURCE_DIR}/po/${po}.po)\n \t\tlist(APPEND po_gen ${CMAKE_BINARY_DIR}/po/build/locale/${po}/LC_MESSAGES/git.mo)\n \tendforeach()\n \tadd_custom_target(po-gen ALL DEPENDS ${po_gen})\n-- \n2.39.0.1071.g97ce8966538\n\n"},{"id":"469292","messageId":"45d8d9fc-ca68-5902-0aa7-70034f8383ff@dunelm.org.uk","threadId":"58976","inReplyTo":"patch-1.1-0fa41115261-20221219T102205Z-avarab@gmail.com","subject":"Re: [PATCH] cmake: don't invoke msgfmt with --statistics","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-12-19T15:00:06Z","receivedAt":"2022-12-19T15:00:33Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Ævar\n\nOn 19/12/2022 10:26, Ævar Arnfjörð Bjarmason wrote:\n> In [1] I made the same change to our Makefile, let's follow-up and do\n> the same here.\n> \n> For \"cmake\" this is particularly nice with \"-G Ninja\", as before we'd\n> emit ~40 lines of overflowed progress bar output, but now it's only\n> the one line of \"ninja\"'s progress bar.\n\nI don't really have a strong opinion either way on this but if it \nmatches what we do in the Makefile than it sounds sensible.\n\nBest Wishes\n\nPhillip\n\n\n> 1. 2f12b31b746 (Makefile: don't invoke msgfmt with --statistics,\n>     2021-12-17)\n> \n> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> ---\n> \n> This trivial fix is extracted from the ab/cmake-nix-and-ci topic which\n> was ejected around the time of the release for that previous\n> submission see [1], the range-diff is to that topic.\n> \n> I'm re-arranging and re-submitting topic more piecemeal. There were no\n> outstanding issues or feedback with this part of it, so hopefully this\n> can advance relatively quickly.\n> \n> I'll also submit some of the other uncontroversial bits today\n> independently, none of which conflict with one another. Then once\n> those have landed try to find some acceptable way forward for the\n> later bits, which at that point will be easier to review.\n> \n> 1. https://lore.kernel.org/git/cover-v6-00.15-00000000000-20221206T001617Z-avarab@gmail.com/\n> \n> Range-diff:\n>   1:  fc190b379cd =  1:  0fa41115261 cmake: don't invoke msgfmt with --statistics\n>   2:  1a11aa233a3 <  -:  ----------- cmake: use \"-S\" and \"-B\" to specify source and build directories\n>   3:  b9ddb5db1d3 <  -:  ----------- cmake: update instructions for portable CMakeLists.txt\n>   4:  7b7850c00ee <  -:  ----------- cmake: don't copy chainlint.pl to build directory\n>   5:  82ecb797915 <  -:  ----------- cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4\n>   6:  1f326944a07 <  -:  ----------- cmake & test-lib.sh: add a $GIT_SOURCE_DIR variable\n>   7:  973c8038f54 <  -:  ----------- cmake: set \"USE_LIBPCRE2\" in \"GIT-BUILD-OPTIONS\" for test-lib.sh\n>   8:  b8448c7a8a6 <  -:  ----------- Makefile + test-lib.sh: don't prefer cmake-built to make-built git\n>   9:  5135e40969e <  -:  ----------- test-lib.sh: support a \"GIT_TEST_BUILD_DIR\"\n> 10:  65204463730 <  -:  ----------- cmake: optionally be able to run tests before \"ctest\"\n> 11:  e25992b16f1 <  -:  ----------- cmake: support GIT_TEST_OPTS, abstract away WIN32 defaults\n> 12:  4905ce5321d <  -:  ----------- cmake: increase test timeout on Windows only\n> 13:  6c6b530965d <  -:  ----------- cmake: only look for \"sh\" in \"C:/Program Files\" on Windows\n> 14:  563f1b9b045 <  -:  ----------- cmake: copy over git-p4.py for t983[56] perforce test\n> 15:  917a884eb65 <  -:  ----------- CI: add a \"linux-cmake-test\" to run cmake & ctest on linux\n> \n>   contrib/buildsystems/CMakeLists.txt | 2 +-\n>   1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\n> index 2f6e0197ffa..8f8b6f375f7 100644\n> --- a/contrib/buildsystems/CMakeLists.txt\n> +++ b/contrib/buildsystems/CMakeLists.txt\n> @@ -897,7 +897,7 @@ if(MSGFMT_EXE)\n>   \tforeach(po ${po_files})\n>   \t\tfile(MAKE_DIRECTORY ${CMAKE_BINARY_DIR}/po/build/locale/${po}/LC_MESSAGES)\n>   \t\tadd_custom_command(OUTPUT ${CMAKE_BINARY_DIR}/po/build/locale/${po}/LC_MESSAGES/git.mo\n> -\t\t\t\tCOMMAND ${MSGFMT_EXE} --check --statistics -o ${CMAKE_BINARY_DIR}/po/build/locale/${po}/LC_MESSAGES/git.mo ${CMAKE_SOURCE_DIR}/po/${po}.po)\n> +\t\t\t\tCOMMAND ${MSGFMT_EXE} --check -o ${CMAKE_BINARY_DIR}/po/build/locale/${po}/LC_MESSAGES/git.mo ${CMAKE_SOURCE_DIR}/po/${po}.po)\n>   \t\tlist(APPEND po_gen ${CMAKE_BINARY_DIR}/po/build/locale/${po}/LC_MESSAGES/git.mo)\n>   \tendforeach()\n>   \tadd_custom_target(po-gen ALL DEPENDS ${po_gen})\n"},{"id":"469333","messageId":"xmqqcz8e29d5.fsf@gitster.g","threadId":"58976","inReplyTo":"45d8d9fc-ca68-5902-0aa7-70034f8383ff@dunelm.org.uk","subject":"Re: [PATCH] cmake: don't invoke msgfmt with --statistics","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-12-20T00:43:02Z","receivedAt":"2022-12-20T00:43:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> On 19/12/2022 10:26, Ævar Arnfjörð Bjarmason wrote:\n>> In [1] I made the same change to our Makefile, let's follow-up and do\n>> the same here.\n>> For \"cmake\" this is particularly nice with \"-G Ninja\", as before\n>> we'd\n>> emit ~40 lines of overflowed progress bar output, but now it's only\n>> the one line of \"ninja\"'s progress bar.\n>\n> I don't really have a strong opinion either way on this but if it\n> matches what we do in the Makefile than it sounds sensible.\n\nAs a one-shot change, it might be sensible to claim consistency by\nsaying \"we do the same thing in two places\", but I'd worry more\nabout the root cause of such inconsistency in the first place, i.e.\ncan we have some trick to ensure that two build systems will not\nreimplement the same thing slightly differently?\n\nIt also is worth examining if having \"the same change\" is a good\nidea in the first place.  The justification given \"In [1]\" was that\na build driven by our Makefile were concise and non-verbose overall,\nbut with --stat that concise output pattern was broken.\n\nI do not know (and I do not have particular interest in knowing) how\na build driven by cmake looks like, but does it also aim the same\nconcise output where output --stat does not fit well, or do folks\nwho daily build with cmake find the output with --stat sit well in\nthe output from other things given there?  If the latter, making\n\"the same change\" as the Makefile side may not make much sense.\n"},{"id":"469549","messageId":"221227.86tu1huevt.gmgdl@evledraar.gmail.com","threadId":"58976","inReplyTo":"xmqqcz8e29d5.fsf@gitster.g","subject":"Re: [PATCH] cmake: don't invoke msgfmt with --statistics","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-27T13:51:17Z","receivedAt":"2022-12-27T13:59:08Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Dec 20 2022, Junio C Hamano wrote:\n\n> Phillip Wood <phillip.wood123@gmail.com> writes:\n>\n>> On 19/12/2022 10:26, Ævar Arnfjörð Bjarmason wrote:\n>>> In [1] I made the same change to our Makefile, let's follow-up and do\n>>> the same here.\n>>> For \"cmake\" this is particularly nice with \"-G Ninja\", as before\n>>> we'd\n>>> emit ~40 lines of overflowed progress bar output, but now it's only\n>>> the one line of \"ninja\"'s progress bar.\n>>\n>> I don't really have a strong opinion either way on this but if it\n>> matches what we do in the Makefile than it sounds sensible.\n>\n> As a one-shot change, it might be sensible to claim consistency by\n> saying \"we do the same thing in two places\", but I'd worry more\n> about the root cause of such inconsistency in the first place, i.e.\n> can we have some trick to ensure that two build systems will not\n> reimplement the same thing slightly differently?\n\nWe could & should, but I think doing prep changes like these first makes\nsense, as eventually e.g. driving both the Makefile & CMake via some\nshared resource won't have to waste time on explaining why the msgfmt\ninvocation is slightly different.\n\n> It also is worth examining if having \"the same change\" is a good\n> idea in the first place.  The justification given \"In [1]\" was that\n> a build driven by our Makefile were concise and non-verbose overall,\n> but with --stat that concise output pattern was broken.\n>\n> I do not know (and I do not have particular interest in knowing) how\n> a build driven by cmake looks like, but does it also aim the same\n> concise output where output --stat does not fit well, ...\n\nIt's the same with CMake, as e.g. the reference to \"ninja\" in the commit\nmessage covers (it would also happen with the \"make\" backend, but that\none's a bit more verbose by default).\n\n> [...] or do folks who daily build with cmake find the output with\n> --stat sit well in the output from other things given there?  If the\n> latter, making \"the same change\" as the Makefile side may not make\n> much sense.\n\nI think the only justification that's needed here (and which should\nshort-circuit any questions about what someone using cmake may or may\nnot like) is the one given in my 2f12b31b746 (Makefile: don't invoke\nmsgfmt with --statistics, 2021-12-17).\n\nI.e. this was something I added as part of the initial i18n support, but\nI had no good reason for using --statistics other than ad-hoc eyeballing\nthe output at the time.\n\nThe CMake recipe then just copy/pasted whatever it found in the\nMakefile, and the two then drifted apart.\n\nSo, in general with those sorts of changes I think it's sufficient to\nsay that we're not bringing them in line again, unless there's some\nreason to suppose that the cmake version has since come to rely on the\ndivergence for some reason.\n\nWhich, in this case is clearly not the case, as we're just spewing this\noutput to the user's terminal.\n"},{"id":"469565","messageId":"xmqq1qoktr33.fsf@gitster.g","threadId":"58976","inReplyTo":"221227.86tu1huevt.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] cmake: don't invoke msgfmt with --statistics","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-12-27T22:33:04Z","receivedAt":"2022-12-27T22:33:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> I think the only justification that's needed here (and which should\n> short-circuit any questions about what someone using cmake may or may\n> not like) is the one given in my 2f12b31b746 (Makefile: don't invoke\n> msgfmt with --statistics, 2021-12-17).\n>\n> I.e. this was something I added as part of the initial i18n support, but\n> I had no good reason for using --statistics other than ad-hoc eyeballing\n> the output at the time.\n>\n> The CMake recipe then just copy/pasted whatever it found in the\n> Makefile, and the two then drifted apart.\n>\n> So, in general with those sorts of changes I think it's sufficient to\n> say that we're not bringing them in line again, unless there's some\n> reason to suppose that the cmake version has since come to rely on the\n> divergence for some reason.\n>\n> Which, in this case is clearly not the case, as we're just spewing this\n> output to the user's terminal.\n\nOK, let's hear from Windows folks about that.  I do not care either\nway myself (after all it is just extra lines in the output), but for\na topic that was once merged to 'next' that later turned out to be\nunwanted (instead of simply being a buggy implementation of the\nright idea), I'd like to hear from those who have been depending on\nwhatever the current behaviour is.\n\nThanks.\n\n\n"}]}