Re: Git in Outreachy December 2019?

Hi Emily,

On Mon, 16 Sep 2019, Emily Shaffer wrote:

Thank you for doing this!

Nice projects, all. There are a couple more ideas on https://github.com/gitgitgadget/git/issues, they could probably use some tagging.

Good project, if a bit boring ;-) Also, `the_index` is used a lot in `builtin/*.c`, still.

I am of two minds there. But mostly, I am of the "friends don't let friends use submodules" camp, so I would not even want to mentor for this project: it just makes me shudder too much every time I have to work with/on submodules.

Of course, if others are interested, I'd hardly object to turn this into a built-in.

That would be my top recommendation, especially given how much effort Tanushree put in last winter to make this conversion to C so much more achievable than before.

No. `rebase -p` is already deprecated in favor of `rebase -r` (which _is_ already built-in).

I already have patches lined up to drop that rebase backend. Let's not waste effort on converting this script to C.

Please see PRs #170-#175 on https://github.com/gitgitgadget/git/pulls, and please do help by adding your review of #170 (which was already submitted as v4: https://public-inbox.org/git/pull.170.v4.git.gitgitgadget@gmail.com/

In other words: this project is well under way. In fact, Git for Windows users enjoy this as an opt-in already.

Converting shell/Perl scripts into built-in C never looks as much fun as open-ended projects with lots of playing around, but the advantage of the former is that they can be easily structured, offer a lot of opportunity for learning, and they are ultimately more rewarding because the goals are much better defined than many other projects'.

Another script that would _really_ benefit from being converted to C: `mergetool`. Especially on Windows, where the over-use of spawned processes really hurts, it is awfully slow a command.

To complete the list of sh/pl commands:

- git-merge-octopus.sh
- git-merge-one-file.sh
- git-merge-resolve.sh

These seem to be good candidates for conversion to built-ins. Their functionality is well-exercised in the test suite, their complexity is quite manageable, and there is no good reason that these should be scripted.

The only slightly challenging aspect might be that `merge-one-file` is actually not a merge strategy, but it is used as helper to be passed to `git merge-index` via the `-o <helper>` option, which makes it slightly awkward to be implemented as a built-in. A better approach would therefore be to special-case this value in `merge-index` and execute the C code directly, without the detour of spawning a built-in.

- git-difftool--helper.sh
- git-mergetool--lib.sh

These would be converted as part of making `mergetool` a built-in, I believe.

- git-filter-branch.sh

This one is in the process of being deprecated in favor of `git filter-repo` (which is an external tool), so I don't think there would be much use in wasting energy on trying to convert it to C. Especially given that it wants to call shell script snippets all over the place, and those shell script snippets are supposed to run in the same context, which might actually make it completely impossible to convert this to C at all.

- git-legacy-stash.sh

This will go away once the built-in stash is considered good enough.

- git-instaweb.sh
- git-request-pull.sh
- git-send-email.perl
- git-web--browse.sh

I don't think that any of these should be converted. They are just too unimportant from a performance point of view, and obscure enough that even their portability issues don't matter too much.

As to `send-email` in particular: I would not want anybody to drag in all the dependencies required to convert `send-email` to a built-in to begin with.

- git-archimport.perl
- git-cvsexportcommit.perl
- git-cvsimport.perl
- git-cvsserver.perl
- git-quiltimport.sh
- git-svn.perl

These are all connectors of some sort to other version control software. It also feels like they become less and less important, as Git really takes over the world.

At some stage, I think, it would make sense to push those scripts out into their own repositories, looking for new maintainers (and if none can be found, then there really is not enough need for them to begin with, and they can be archived).

I don't think that this would be a good project for anybody except people who are already really, really familiar with our mailing list-centric workflow.

Another, potentially more rewarding, project would be to modernize our test suite framework, so that it is not based on Unix shell scripting, but on C instead.

The fact that it is based on Unix shell scripting not only costs a lot of speed, especially on Windows, it also limits us quite a bit, and I am talking about a lot more than just the awkwardness of having to think about options of BSD vs GNU variants of common command-line tools.

For example, many, many, if not all, test cases, spend the majority of their code on setting up specific scenarios. I don't know about you, but personally I have to dive into many of them when things fail (and I _dread_ the numbers 0021, 0025 and 3070, let me tell you) and I really have to say that most of that code is hard to follow and does not make it easy to form a mental model of what the code tries to accomplish.

To address this, a while ago Thomas Rast started to use `fast-export`ed commit histories in test scripts (see e.g. `t/t3206/history.export`). I still find that this fails to make it easier for occasional readers to understand the ideas underlying the test cases.

Another approach is to document heavily the ideas first, then use code to implement them. For example, t3430 starts with this:

	[...]

	Initial setup:

	    -- B --                   (first)
	   /       \
	 A - C - D - E - H            (master)
	   \    \       /
	    \    F - G                (second)
	     \
	      Conflicting-G

	[...]

	test_commit A &&
	git checkout -b first &&
	test_commit B &&
	git checkout master &&
	test_commit C &&
	test_commit D &&
	git merge --no-commit B &&
	test_tick &&
	git commit -m E &&
	git tag -m E E &&
	git checkout -b second C &&
	test_commit F &&
	test_commit G &&
	git checkout master &&
	git merge --no-commit G &&
	test_tick &&
	git commit -m H &&
	git tag -m H H &&
	git checkout A &&
	test_commit conflicting-G G.t

	[...]

While this is _somewhat_ better than having only the code, I am still unhappy about it: this wall of `test_commit` lines interspersed with other commands is very hard to follow.

If we were to (slowly) convert our test suite framework to C, we could change that.

One idea would be to allow recreating commit history from something that looks like the output of `git log`, or even `git log --graph --oneline`, much like `git mktree` (which really should have been a test helper instead of a Git command, but I digress) takes something that looks like the output of `git ls-tree` and creates a tree object from it.

Another thing that would be much easier if we moved more and more parts of the test suite framework to C: we could implement more powerful assertions, a lot more easily. For example, the trace output of a failed `test_i18ngrep` (or `mingw_test_cmp`!!!) could be made a lot more focused on what is going wrong than on cluttering the terminal window with almost useless lines which are tedious to sift through.

Likewise, having a framework in C would make it a lot easier to improve debugging, e.g. by making test scripts "resumable" (guarded by an option, it could store a complete state, including a copy of the trash directory, before executing commands, which would allow "going back in time" and calling a failing command with a debugger, or with valgrind, or just seeing whether the command would still fail, i.e. whether the test case is flaky).

Also, things like the code tracing via `-x` (which relies on Bash functionality in order to work properly, and which _still_ does not work as intended if your test case evaluates a lazy prereq that has not been evaluated before) could be "done right".

In many ways, our current test suite seems to test Git's functionality as much as (core) contributors' abilities to implement test cases in Unix shell script, _correctly_, and maybe also contributors' patience. You could say that it tests for the wrong thing at least half of the time, by design.

It might look like a somewhat less important project, but given that we exercise almost 150,000 test cases with every CI build, I think it does make sense to grind our axe for a while, so to say.

Therefore, it might be a really good project to modernize our test suite. To take ideas from modern test frameworks such as Jest and try to bring them to C. Which means that new contributors would probably be better suited to work on this project than Git old-timers!

And the really neat thing about this project is that it could be done incrementally.

I don't find that list particularly big, to be honest ;-)

Ciao, Dscho