| Message ID | 20241206-pks-meson-v11-25-525ed4792b88@pks.im (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Modernize the build system | expand |
Patrick Steinhardt <ps@pks.im> writes: > As support for Meson is still in-flight we have to accommodate for > conflicts with topics in "seen". The following conflicts are being > addressed in this commit: > > - ej/cat-file-remote-object-info adds t1017 and "fetch-object-info.c". > > - ds/path-walk-1 adds t6601 as well as "path-walk.c" and > "test-path-walk.c". > > - js/libgit-rust adds "common-exit.c" and "common-init.c". > > - ds/name-hash-tweaks adds "t/helper/test-name-hash.c". > > This is somewhat painful in the current state where Meson is not yet > part of the main tree, but we'll have to live with that for the time > being. > > I've split this commit out into a separate fixup-style commit such that > it is possible to test this topic both with and without "seen" merged > into it. You can simply revert this commit to test without "seen". Now I had to reconstruct these "fixup-style" commits and they appear under ref/merge-fix/ hierarchy in my broken-out repository published at https://github.com/gitster/git.git/ (and no other fallback URLs; it might make sense to have another repository for redundancy, but it is an unrelated tangent). In addition to these listed four, a newly added ds/backfill also needs refs/merge-fix/ds/backfill to adjust. What I noticed while performing this exercise to place ps/build~1 (i.e. the series without this step) immediately on top of where the history leading to 'seen' has a commit whose tree matches that of 'next' is that we need some "distributed" (read: put the burden on topic authors, not on the onwer of ps/build topic, and not on the maintainer) way to make sure various list of files in meson.build, t/meson.build, and t/helper/meson.build are in sync with corresponding list of files in the Makefile world. It _should_ be automatable in theory, and to help those who develop against the current practice of treating Makefile as the authoritative build system, a Makefile target that tells them that they added a new file to Makefile and removed a file from t/Makefile but they forgot to make corresponding changes to meson.build and t/meson.build files would be very beneficial. We could run that target as a part of "make test". Of course, those who care about keeping CMake build up to date can add something similar hooked up to help others help themselves.
Junio C Hamano <gitster@pobox.com> writes: > Patrick Steinhardt <ps@pks.im> writes: > >> I've split this commit out into a separate fixup-style commit such that >> it is possible to test this topic both with and without "seen" merged >> into it. You can simply revert this commit to test without "seen". > > Now I had to reconstruct these "fixup-style" commits and they appear > under ref/merge-fix/ hierarchy in my broken-out repository published > at https://github.com/gitster/git.git/ (and no other fallback URLs; > it might make sense to have another repository for redundancy, but > it is an unrelated tangent). In addition to these listed four, a > newly added ds/backfill also needs refs/merge-fix/ds/backfill to > adjust. I now have merge-fix changes for the topics that add new sources and test scripts that are not covered by [01-24/26] of this series, and dropped [26/26] (the reversion of [25/26]). The [25/26], adjustment for topics in flight in 'seen', is still at the tip of ps/build but the merge of ps/build into 'seen' is now a no-op, which is a good sign, as these merge-fix changes for each topic are designed to make resulting merge of each of these topics successfully work with the meson build infrastructure, and ps/build~1 [01-24/26] is merged a lot earlier on the master..seen chain in preparation for merging them to 'next'. The next step for me is to see if we want v12 of the series, and if not, merge [01-24/26] down to 'next'. Thanks.
On Sun, Dec 08, 2024 at 12:00:46PM +0900, Junio C Hamano wrote: > Patrick Steinhardt <ps@pks.im> writes: > > > As support for Meson is still in-flight we have to accommodate for > > conflicts with topics in "seen". The following conflicts are being > > addressed in this commit: > > > > - ej/cat-file-remote-object-info adds t1017 and "fetch-object-info.c". > > > > - ds/path-walk-1 adds t6601 as well as "path-walk.c" and > > "test-path-walk.c". > > > > - js/libgit-rust adds "common-exit.c" and "common-init.c". > > > > - ds/name-hash-tweaks adds "t/helper/test-name-hash.c". > > > > This is somewhat painful in the current state where Meson is not yet > > part of the main tree, but we'll have to live with that for the time > > being. > > > > I've split this commit out into a separate fixup-style commit such that > > it is possible to test this topic both with and without "seen" merged > > into it. You can simply revert this commit to test without "seen". > > Now I had to reconstruct these "fixup-style" commits and they appear > under ref/merge-fix/ hierarchy in my broken-out repository published > at https://github.com/gitster/git.git/ (and no other fallback URLs; > it might make sense to have another repository for redundancy, but > it is an unrelated tangent). In addition to these listed four, a > newly added ds/backfill also needs refs/merge-fix/ds/backfill to > adjust. Okay. > What I noticed while performing this exercise to place ps/build~1 > (i.e. the series without this step) immediately on top of where the > history leading to 'seen' has a commit whose tree matches that of > 'next' is that we need some "distributed" (read: put the burden on > topic authors, not on the onwer of ps/build topic, and not on the > maintainer) way to make sure various list of files in meson.build, > t/meson.build, and t/helper/meson.build are in sync with > corresponding list of files in the Makefile world. It _should_ be > automatable in theory, and to help those who develop against the > current practice of treating Makefile as the authoritative build > system, a Makefile target that tells them that they added a new file > to Makefile and removed a file from t/Makefile but they forgot to > make corresponding changes to meson.build and t/meson.build files > would be very beneficial. We could run that target as a part of > "make test". > > Of course, those who care about keeping CMake build up to date can > add something similar hooked up to help others help themselves. I have the following change queued up in a local patch series: # Sanity check that we are not missing any tests present in 't/'. This check # only runs once at configure time and is thus best-effort, only. It is # sufficient to catch missing test suites in our CI though. actual_tests = run_command(shell, '-c', 'ls t[0-9][0-9][0-9][0-9]-*.sh', check: true).stdout().strip().split('\n') if integration_tests != actual_tests missing_tests = [ ] foreach actual_test : actual_tests if actual_test not in integration_tests missing_tests += actual_test endif endforeach if missing_tests.length() > 0 error('Missing tests:\n\n - ' + '\n - '.join(missing_tests)) endif superfluous_tests = [ ] foreach integration_test : integration_tests if integration_test not in actual_tests superfluous_tests += integration_test endif endforeach if superfluous_tests.length() > 0 error('Superfluous tests:\n\n - ' + '\n - '.join(superfluous_tests)) endif endif What it does is to verify that test suites wired up in Meson are up-to-date, and if they aren't `meson setup` will fail and print all missing or extraneous test suites. For code it's a bit harder to do as a lot of it only compiles conditionally, so I don't have a solution for that yet. It also doesn't execute as part of `make test` -- if we want that I'd have to implement it via a separate shell script instead. Which isn't too bad, we'll simply have to agree on a direction. But the logic will execute as part of CI so that any discrepancies will be flagged. Patrick
Patrick Steinhardt <ps@pks.im> writes: > I have the following change queued up in a local patch series: > ... > What it does is to verify that test suites wired up in Meson are > up-to-date, and if they aren't `meson setup` will fail and print all > missing or extraneous test suites. For code it's a bit harder to do as a > lot of it only compiles conditionally, so I don't have a solution for > that yet. It would certaionly help those who build work "meson" to remind themselves that they need to make matching changes to Makefile world when they update their meson.build files. > It also doesn't execute as part of `make test` -- if we want that I'd > have to implement it via a separate shell script instead. Which isn't > too bad, we'll simply have to agree on a direction. But the logic will > execute as part of CI so that any discrepancies will be flagged. And catching a breakage as part of CI is a good first step nevertheless. Having said all that, with a longer term evolution of the build systems in mind: - Currently, anybody who is working on our codebase is expected to make "make all doc test" succeed before even running format-patch and/or send-email. But unless you are testing with ps/build topic, you won't be checking "meson test". The same is likely be true for CMake (I am not encouraging folks to send a change that is OK only for "make" and deliberatly breaks "meson" and "cmake"; but for those who are used to build with "make", having to _also_ check with "meson" and "cmake" is additional barriers. - The ideal world, at least from the point of view of those advocate ps/build topic, should be that "meson" is treated at least as another build system with equal footing as "make". With even longer timeframe, the effort may even aim to supersede and deprecate "make", but before that happens, it has to at least become build system with equal importance. - In the longer timeframe where many more folks work with "meson" than "make", check implemented on the "meson" side to make sure that "make" world will not left rotting would be helpful before we finally kill off "make" support (if that is what we are aiming for). But until then, I think the pay off would be larger if we helped "make" population to avoid leaving "meson" and "cmake" builds out of sync when they update Makefile. And that is why I suggested "make test" as that is currently what is run by everybody. It is like &&-check that helps individual developers with their test before even submitting their changes. I am unlikely to run the check locally before pushing them out, simply because meson build is not yet part of my everyday workflow, but even after I made it part of my workflow to check, in a sense, it it too late. Once we adopt ps/build~1 as a part of our official world, if a developer, who builds on top of the 'master' in the not so distant future, does not pay attention when they touch Makefile without making matching changes to meson.build files, I'd want to be able to scold them for not running "make test"; otherwise the process will not scale. I also had to wonder if we can share the common list of files and have both make and meson worlds include them from the same source, but if the eventual goal is to migrate into a single system, it is not all that great an idea. After giving up on one of the systems, we would not want to keep a system where we keep a list of source files in a separate file (possibly in a separate syntax), which is not how either world would normally work. Thanks.
On Mon, Dec 09, 2024 at 04:58:59PM +0900, Junio C Hamano wrote: > Patrick Steinhardt <ps@pks.im> writes: > > > I have the following change queued up in a local patch series: > > ... > > What it does is to verify that test suites wired up in Meson are > > up-to-date, and if they aren't `meson setup` will fail and print all > > missing or extraneous test suites. For code it's a bit harder to do as a > > lot of it only compiles conditionally, so I don't have a solution for > > that yet. > > It would certaionly help those who build work "meson" to remind > themselves that they need to make matching changes to Makefile world > when they update their meson.build files. > > > It also doesn't execute as part of `make test` -- if we want that I'd > > have to implement it via a separate shell script instead. Which isn't > > too bad, we'll simply have to agree on a direction. But the logic will > > execute as part of CI so that any discrepancies will be flagged. > > And catching a breakage as part of CI is a good first step > nevertheless. > > Having said all that, with a longer term evolution of the build > systems in mind: > > - Currently, anybody who is working on our codebase is expected to > make "make all doc test" succeed before even running format-patch > and/or send-email. But unless you are testing with ps/build > topic, you won't be checking "meson test". The same is likely be > true for CMake (I am not encouraging folks to send a change that > is OK only for "make" and deliberatly breaks "meson" and "cmake"; > but for those who are used to build with "make", having to _also_ > check with "meson" and "cmake" is additional barriers. Fair. > - The ideal world, at least from the point of view of those > advocate ps/build topic, should be that "meson" is treated at > least as another build system with equal footing as "make". With > even longer timeframe, the effort may even aim to supersede and > deprecate "make", but before that happens, it has to at least > become build system with equal importance. Agreed, feature parity is a necessary requirement and I'll of course continue to iterate on it once Meson has landed. We might get away with removing the autoconf framework without feature parity as users can still use "make" without it, and because it's seldomly used and broken. But replacing "make", if it happens at all, is a much larger undertaking that will require many releases. I'll also align with Dscho to figure out what to do with CMake. > - In the longer timeframe where many more folks work with "meson" > than "make", check implemented on the "meson" side to make sure > that "make" world will not left rotting would be helpful before > we finally kill off "make" support (if that is what we are aiming > for). But until then, I think the pay off would be larger if we > helped "make" population to avoid leaving "meson" and "cmake" > builds out of sync when they update Makefile. > > And that is why I suggested "make test" as that is currently what is > run by everybody. It is like &&-check that helps individual developers > with their test before even submitting their changes. > > I am unlikely to run the check locally before pushing them out, > simply because meson build is not yet part of my everyday workflow, > but even after I made it part of my workflow to check, in a sense, > it it too late. Once we adopt ps/build~1 as a part of our official > world, if a developer, who builds on top of the 'master' in the not > so distant future, does not pay attention when they touch Makefile > without making matching changes to meson.build files, I'd want to be > able to scold them for not running "make test"; otherwise the > process will not scale. Fair. > I also had to wonder if we can share the common list of files and > have both make and meson worlds include them from the same source, > but if the eventual goal is to migrate into a single system, it is > not all that great an idea. After giving up on one of the systems, > we would not want to keep a system where we keep a list of source > files in a separate file (possibly in a separate syntax), which is > not how either world would normally work. That would be great indeed, but it's somewhat incompatible with how higher-level build systems work. We do it in CMake by parsing our Makefile and extracting source files, but the problem is that this only happens at configuration time. So the set of source files would not be updated properly when your Makefile changes. An alternative would be to use globbing, but that is not recommended, either. Quoting CMake docs: We do not recommend using GLOB to collect a list of source files from your source tree. If no CMakeLists.txt file changes when a source is added or removed then the generated build system cannot know when to ask CMake to regenerate. The CONFIGURE_DEPENDS flag may not work reliably on all generators, or if a new generator is added in the future that cannot support it, projects using it will be stuck. Even if CONFIGURE_DEPENDS works reliably, there is still a cost to perform the check on every rebuild. The same is true for Meson: Meson does not support this syntax and the reason for this is simple. This cannot be made both reliable and fast. By reliable we mean that if the user adds a new source file to the subdirectory, Meson should detect that and make it part of the build automatically. One of the main requirements of Meson is that it must be fast. This means that a no-op build in a tree of 10 000 source files must take no more than a fraction of a second. This is only possible because Meson knows the exact list of files to check. If any target is specified as a wildcard glob, this is no longer possible. Meson would need to re-evaluate the glob every time and compare the list of files produced against the previous list. This means inspecting the entire source tree (because the glob pattern could be src/\*/\*/\*/\*.cpp or something like that). This is impossible to do efficiently. We could of course eventually reverse the logic and have Makefiles parse files from Meson. But as mentioned already, the biggest issue is that we have lots of source files that get compiled conditionally, and properly parsing these would be non-trivial. I guess we'll see over time how much of a problem it is to keep build systems in sync. Proper tooling is a first step to make things a bit less tedious, but if this still proves to be too painful I'll have a look for how to further automate things. Patrick
Patrick Steinhardt <ps@pks.im> writes: >> I also had to wonder if we can share the common list of files and >> have both make and meson worlds include them from the same source, >> but if the eventual goal is to migrate into a single system, it is >> not all that great an idea. After giving up on one of the systems, >> we would not want to keep a system where we keep a list of source >> files in a separate file (possibly in a separate syntax), which is >> not how either world would normally work. > > That would be great indeed, Well, what I wanted to say is that it is not a good idea, especially if our eventual goal were to ditch either one of these systems, so we are on the same page. After ps/build~1 graduates to 'master', it may be sufficient to add a Makefile target that runs the check script you sent earlier to this thread as a part of "make test". That way, those developers who work primarily with "make" can minimally install "meson" and do nothing else and we make sure that their changes in the "make" world would not make "meson" (and possibly "cmake") world stale. Those who do not have "meson" may skip the check during their "make test" without any negative (perhaps other than a message to nag them to install "meson") consequences, or even a hard failure may be acceptable, if the necessary adjustment is trivial (like "I added a command in builtin/ and listed it in Makefile---add it to this list in meson.build file"). After all, we are talking about developers who are willing to run "make test", so requiring them to have a build system if they want to develop for this project may not be too bad. I dunno. > I guess we'll see over time how much of a problem it is to keep build > systems in sync. Proper tooling is a first step to make things a bit > less tedious, but if this still proves to be too painful I'll have a > look for how to further automate things. With very small set of topics between 'next' and 'seen', we know that we already needed 4 or 5 merge-fix changes. The authors of these topics would have wanted all the help making sure their updates are good in both "make" and "meson" world, if they started their topic in a not-so-distant future where ps/build~1 has graduated to 'master'. Thanks.
diff --git a/meson.build b/meson.build index 0dccebcdf16b07650d943e53643f0e09e2975cc9..442d7bbabf407aee00cce280f40a4d5527f18d9c 100644 --- a/meson.build +++ b/meson.build @@ -235,6 +235,8 @@ libgit_sources = [ 'commit-graph.c', 'commit-reach.c', 'commit.c', + 'common-exit.c', + 'common-init.c', 'compat/nonblock.c', 'compat/obstack.c', 'compat/terminal.c', @@ -273,6 +275,7 @@ libgit_sources = [ 'ewah/ewah_rlw.c', 'exec-cmd.c', 'fetch-negotiator.c', + 'fetch-object-info.c', 'fetch-pack.c', 'fmt-merge-msg.c', 'fsck.c', @@ -347,6 +350,7 @@ libgit_sources = [ 'parse-options.c', 'patch-delta.c', 'patch-ids.c', + 'path-walk.c', 'path.c', 'pathspec.c', 'pkt-line.c', diff --git a/t/helper/meson.build b/t/helper/meson.build index 5e83884246edc7841738de5085f3255aa1fa3fbe..1d6154ce9756db17bc9f69bc3cd71a32b93857c5 100644 --- a/t/helper/meson.build +++ b/t/helper/meson.build @@ -34,12 +34,14 @@ test_tool_sources = [ 'test-match-trees.c', 'test-mergesort.c', 'test-mktemp.c', + 'test-name-hash.c', 'test-online-cpus.c', 'test-pack-mtimes.c', 'test-parse-options.c', 'test-parse-pathspec-file.c', 'test-partial-clone.c', 'test-path-utils.c', + 'test-path-walk.c', 'test-pcre2-config.c', 'test-pkt-line.c', 'test-proc-receive.c', diff --git a/t/meson.build b/t/meson.build index 13fe854ba0a18f9b83dbc48651f581198042ffd3..1c82e27a1d48dd6a163d7b26c74b5ebf6f258d41 100644 --- a/t/meson.build +++ b/t/meson.build @@ -177,6 +177,7 @@ integration_tests = [ 't1014-read-tree-confusing.sh', 't1015-read-index-unmerged.sh', 't1016-compatObjectFormat.sh', + 't1017-cat-file-remote-object-info.sh', 't1020-subdirectory.sh', 't1021-rerere-in-workdir.sh', 't1022-read-tree-partial-clone.sh', @@ -829,6 +830,7 @@ integration_tests = [ 't6500-gc.sh', 't6501-freshen-objects.sh', 't6600-test-reach.sh', + 't6601-path-walk.sh', 't6700-tree-depth.sh', 't7001-mv.sh', 't7002-mv-sparse-checkout.sh',
As support for Meson is still in-flight we have to accommodate for conflicts with topics in "seen". The following conflicts are being addressed in this commit: - ej/cat-file-remote-object-info adds t1017 and "fetch-object-info.c". - ds/path-walk-1 adds t6601 as well as "path-walk.c" and "test-path-walk.c". - js/libgit-rust adds "common-exit.c" and "common-init.c". - ds/name-hash-tweaks adds "t/helper/test-name-hash.c". This is somewhat painful in the current state where Meson is not yet part of the main tree, but we'll have to live with that for the time being. I've split this commit out into a separate fixup-style commit such that it is possible to test this topic both with and without "seen" merged into it. You can simply revert this commit to test without "seen". Signed-off-by: Patrick Steinhardt <ps@pks.im> --- meson.build | 4 ++++ t/helper/meson.build | 2 ++ t/meson.build | 2 ++ 3 files changed, 8 insertions(+)