Non-Windows fixes

[dscho]

This Pull Request contains changes that are not specific to Windows, but were made while developing Git for Windows nevertheless.

[gitster]

I am waiting for your submitGit experiment using this branch ;-) Let's see how it goes.

Thanks.

[dscho]

I tried again with the current SubmitGit, but it still had the same issues as I reported back to the announcement mail of SubmitGit. I now opened these two tickets:

• Add a From: line when the author differs from the sender rtyley/submitgit#8
• Add a cover letter for multi-patch Pull Requests rtyley/submitgit#9

@rtyley I would be eager to help; could you give me a quick tutorial how to set up Scala to develop and test SubmitGit? I am more of a Java developer than Scala developer, used to Maven, so I am a bit of a newbie as far as Scala web applications are concerned.

[rtyley]

@dscho I've added some instructions at https://github.com/rtyley/submitgit/blob/master/BUILD.md - I hope you find them useful, though there's a fairly long list of credentials to sort out before you can successfully run submitGit locally...

[dscho]

Thanks. I was under the impression that https://github.com/rtyley/submitgit/tree/master/test contains some sort of unit tests, is that correct? And if it is, are they run as part of the sbt build?

[rtyley]

I was under the impression that https://github.com/rtyley/submitgit/tree/master/test contains some sort of unit tests, is that correct? And if it is, are they run as part of the sbt build?

Yup, a very small number of tests, run as part of the build: https://travis-ci.org/rtyley/submitgit/builds/66290778#L127

[dscho]

@gitster I finally bit the bullet and wrote a shell script to help me with the mailing list-based code contribution.

Originally, I really wanted to work on submitGit, because it seems more scalable.

However, the mailing list-based contribution process has so many challenges that can only be remedied by working substantially harder on the submitter's side, such as generating interdiffs (which of course need to account for the fact that upstream does not hold still).

I also found that it is particularly tedious to keep track of multiple iterations (and the trend on the Git mailing list in particular goes to the double digits of iterations required before merging): it is simply impossible to do without a tool that at least tries to bring back the ease of Pull Requests to the developer.

Needless to say, my shell script is not half as comfortable as GitHub Pull Requests. It has to simulate the convenient links by embedding them into local tags' messages. But at least now I can hope to juggle more than three code submissions at any given time.

@rtyley I am really sorry that I could not contribute more to submitGit. I still think that this is the best solution for code submission to the Git project so far (even if the hurdle of requiring contributors to learn not only Scala, but also how to use Scala in a Rails-like fashion, is rather steep), yet I am unable to think of a way how submitGit could store tags, generate interdiffs, force-rebase tags when upstream has moved, link to previous iterations, etc.

[dscho]

Oh, and for the convenience of lurkers, here is the submission: http://thread.gmane.org/gmane.comp.version-control.git/280190

[rtyley]

how submitGit could store tags, generate interdiffs, force-rebase tags when upstream has moved, link to previous iterations, etc

Thanks @dscho - these are features that I maybe could make work, tho' as I'm a very infrequent contributor to the Git project itself, it would help me if you could verbosely describe these items of Git-submission culture - assume I know virtually nothing, because... it's true.

• store tags & force-rebase tags when upstream has moved - what would are these tags for (and where do the tags live)? If the tags are to record what version of a patch has already been submitted through submitGit (so it can be compared to new versions of the patch) then it shouldn't be too hard to get submitGit to store the commit-id of the base and the patch-tip in it's PR submission-comment. It already uses these comments to automatically work out what the previously sent emails are, as you can see in the submitGit interface :
• generate interdiffs - 'interdiff' isn't a term that shows up in Documentation/SubmittingPatches - is there an authoritative guide on how to interdiff, or is it just diffing two patch sets using http://linux.die.net/man/1/interdiff ?

[dscho]

store tags & force-rebase tags when upstream has moved - what would are these tags for (and where do the tags live)?

First, let's talk about the stored tags.

On the Git mailing list, you will frequently find something like [PATCH v4 0/2] ..., i.e. the _n_th iteration of a patch series. The number of iteration can actually get very high (and contributors are not unheard of that just give up after too many iterations, resulting in a net loss for the Git project).

This is somewhat comparable to looking at a Pull Request's current commits, except that the mailing list approach forces you to make sure that you are actually looking at the newest iteration of the patch series.

The only viable way I found to emulate somewhat Pull Request-like convenience with a mailing list-based process is to create tags for every submitted iteration. For example, the first iteration of my non-win-fixes patch series is tagged as non-win-fixes-v1. The next iteration will be tagged as non-win-fixes-v2, and so on.

As the mailing list does not offer any way to fetch revisions, it is advisable to provide some sort of record of what has changed between revisions. This is typically done in the way of an "interdiff", i.e. a diff showing the differences between the iterations.

Now, there are a couple of problems with such an interdiff:

• it does not exactly show which patch was updated,
• it does not show differences of the commit message (or other commit metadata), and
• it does not keep a full tally as to which concerns have been addressed in which way.

That latter point really cannot be done with mailing lists, and since Git will not move away from that method, there is simply nothing at all we can do about it.

Even so, interdiffs have become the common way to illustrate what changed between iterations.

There is an even more troublesome problem with interdiffs: sometimes, upstream (i.e. Git's next or master, depending on the nature of the patch series) changes, and I highly recommend to rebase the patch series onto the latest and greatest upstream revision. Because if you don't, after iteration and iteration of patches, the series is accepted, or so you think, only to not apply cleanly anymore. This would not be a problem if we used an approach that did not only use Git before sending mails and after reviewing them. But we use Git for no step of of the review process. Not for commenting, not for revisioning patch series' iterations, not for commit messages, not for replies, not for listing who reviewed the changes, not for connecting any of the patches to the conversation on the mailing list. And certainly not for rebasing a patch series to a branch that advanced in the meantime because you cannot rebase mails, only commits.

However, if you generated an interdiff blindly after rebasing your branch to the newest upstream revision, your interdiff would not really be helpful: it would include all the changes that have been applied to upstream in the meantime!

The solution I came up with is to maintain another set of tags with the rebased version of the already submitted patch series. In my example above, non-win-fixes-v1-rebased (reflecting non-win-fixes-v1, rebased onto next) will be updated until non-win-fixes-v2 is tagged, and the diff between the two is the interdiff. This is what I called the force-rebase tags.

Now, the biggest problem is of course if there are merge conflicts when rebasing. I have no real solution for that.

This is not ideal, of course, because it is not the real diff between the iterations, but again, a mailing list-based patch submission process simply prevents a more optimal solution.

If the tags are to record what version of a patch has already been submitted through submitGit (so it can be compared to new versions of the patch) then it shouldn't be too hard to get submitGit to store the commit-id of the base and the patch-tip in it's PR submission-comment.

While I think that this would be good, the canonical way of Git would be to "store" the commit-id by tagging it. That's what my script does.

It already uses these comments to automatically work out what the previously sent emails are, as you can see in the submitGit interface

Yes, it does that, but only to the previous iteration, not to all of them. My script does that, and then also adds to the tag message a Submitted-As: <URL> line for the current iteration and In-Reply-To: <URL> lines for all previous iterations. This serves as a semi-convenient tool to allow me to make sure that I did not leave any comment unaddressed in new iterations. Of course, it is suboptimal in that it cannot link into my mail program where I record which mails I read and which I could not read yet. But that's something I have forced myself to accept.

But even if the tagging and interdiffing was addressed somehow, I still would miss two features in submitGit:

• to provide the initial Pull Request comment as a cover letter, and
• to allow me to submit patches authored by somebody else

I imagine that these would be not only possible to implement, but actually quite easy for somebody familiar with using Scala in a Rails-like manner. Not more than 5 minutes, each.

The crucial bit is "for somebody familiar with". As I am not, and I had to switch machines three times since I tried to dabble with contributing to submitGit (so I forgot half of what I learned, and I would have to re-setup Scala and SBT), it would take me a substantially longer time to get there, more in the "days" than in the "minutes" realm.

I guess I am not sure that I would have chosen Scala as a platform to implement submitGit, it is not like upstream Git didn't require you to know quite a number of programming languages already (C, Make, Shell, Perl, Tcl/Tk, Python, sed, awk, and I probably forgot some).

[rtyley]

Thanks for that!

Regarding the stored tags, that seems to be a (totally reasonable) custom flow you're using as a power-contributor to handle the long-lived-patchset-problem - I guess at least some other Git contributors are doing something similar. Tagging is definitely more Git-ish than storing metadata in PR comments, but as far as submitGit goes, the comments are about the best option available - I've just tweaked the messages to add that information in the comment, eg submitgit#1 (comment)

I still would miss two features in submitGit:

to provide the initial Pull Request comment as a cover letter, and

Fair enough, rtyley/submitgit#9 has been around for 4 months and I've not got round to it yet - like everyone, I have lots of draws on my time - so many projects, so little time...!

to allow me to submit patches authored by somebody else

I don't understand this one - I believe you can already do that with submitGit? GitHub allows you to open a PR using any commits, even commits authored/commited by someone else - and submitGit will allow you to submit a PR so long as you opened it.

rtyley/submitgit#11 should also mean that the commits are properly attributed?

I guess I am not sure that I would have chosen Scala as a platform to implement submitGit, it is not like upstream Git didn't require you to know quite a number of programming languages already (C, Make, Shell, Perl, Tcl/Tk, Python, sed, awk, and I probably forgot some).

Of those, I guess Perl and Python are reasonably popular languages for developing web apps- they're not ones that personally I would enjoy writing a large application with tho' - I just went with what I'm familiar with. I already have several Scala-based-GitHub apps, and could leverage them. Not on the list of Git-core languages are Ruby and Node, which are even more popular for developing web apps, but even less familiar to me...

My core aim with submitGit is to make it easier for new contributors to make a contribution to Git (something I haven't pushed hard yet - most new users don't know it exists).

[dscho]

Re: stored tags, yep, I agree that submitGit cannot do more without asking for real invasive privileges.

As to rtyley/submitgit#9, I would have loved to contribute. Sincerely.

to allow me to submit patches authored by somebody else

I don't understand this one - I believe you can already do that with submitGit? GitHub allows you to open a PR using any commits, even commits authored/commited by someone else - and submitGit will allow you to submit a PR so long as you opened it.

rtyley/submitgit#11 should also mean that the commits are properly attributed?

Whoops, I forgot to test that one. In my script, I also Cc: the original author, see e.g. @patthoyts' patch.

My core aim with submitGit is to make it easier for new contributors to make a contribution to Git (something I haven't pushed hard yet - most new users don't know it exists).

I really hope that you will find a little bit of time to get there. I understand that everybody has their preferences as to programming languages. I do not mind Scala, but that Rails-like style just requires so much prior knowledge that I would have to acquire first. Now, if submitGit used Node, I would be fairly fluent...

Anyways. I appreciate how much work you put into this already.

[dscho]

I appreciate how much work you put into this already.

... and please accept my apologies for venting my frustration at being unable to contribute to submitGit (I know that you did not "keep me out" on purpose).

[rtyley]

Whoops, I forgot to test that one. In my script, I also Cc: the original author, see e.g. @patthoyts' patch.

Sounds reasonable - if it's a large patch set, would you CC them for all patches, or just the one with their name on?

[rtyley]

Anyways. I appreciate how much work you put into this already.

... and please accept my apologies for venting my frustration at being unable to contribute to submitGit (I know that you did not "keep me out" on purpose).

No worries - thank you for taking the time to explain your flow so clearly 👍

[dscho]

if it's a large patch set, would you CC them for all patches, or just the one with their name on?

My preference is to Cc: them only on the patches they authored (and that is what my script does, too).