git-subtree Ready #2

[This bounced for some reason.]

Ok, I have http access now:

git clone http://sources.obbligato.org/git/git.git git pull origin subtree

I might need to fiddle with permissions, let me know.

                              -Dave

greened@obbligato.org (David A. Greene) writes:

Everybody's client can talk smart http these days. Please don't host/publish the code behind a dumb HTTP server.

Junio C Hamano <gitster@pobox.com> writes:

I'll have to learn how to set up a smart http.

In the meantime, this should also work:

git clone git://sources.obbligato.org/git/git.git

Or via gitweb:

http://sources.obbligato.org

                               -Dave

greened@obbligato.org (David A. Greene) writes:

Haven't heard anything lately so I want to make sure there's not an access problem.

This is also available at:

git clone git://sources.obbligato.org/git/git.git

or by gitweb:

http://sources.obbligato.org

                          -Dave

On Tue, Feb 14, 2012 at 10:30:16PM -0600, David A. Greene wrote:

Hmm. So it seems like a pretty straightforward subtree merge of git-subtree. I can't say I'm super excited about having copied bits like contrib/subtree/t/Makefile that are basically replicas of git's t/Makefile. But there's not that much of it, the cruft lives in contrib, and there's really not a good solution short of actually making git-subtree a first-class git command.

But more important than the physical layout is the maintenance plan going forward. Is Avery going to keep maintaining git-subtree, and we will just occasionally pull? Are you maintaining it? Where will patches go? To a github repo? To git@vger?

-Peff

Jeff King <peff@peff.net> writes:

Yep.

I know, I didn't like it either but could not think of a better way.

That's the conclusion I came to. Moving it to a first-class command should be pretty simple when the time comes.

I am still waiting to hear from Avery on that. I will ping him again. My intention is to certainly participate in maintenance. I use git-subtree daily so it's in my interest to keep it working.

I plan to send patcher to git@vger. I don't think a pull would be practical given the removal of redundant files and other things.

                           -Dave

greened@obbligato.org (David A. Greene) writes:

I've attached Avery's response below. The short summary is that he thinks maintaining it in the vger git repository is the way to go and that he's fine moving patches to/from GitHub as necessary.

So again, I will certainly be part of the maintenance team. There are a few other people currently helping out with maintenance and I will help Avery to get the word out about the switch as he requires.

                            -Dave

--8<-----------------------------------------------------------------------

Thanks for putting work into this. You can feel free to cc: me on any git-list discussions if you want.

I haven't done any significant amount of git-subtree maintenance lately, but even if I did, since it's only one file it should be easy to put move patches between github and git whenever we want. I would suggest that just maintaining it as part of git is the best way to go, since having diverging versions doesn't really help anyone.

I'm sure the potential benefit of putting git-subtree in the contrib/ directory is that we could then use git-subtree to maintain the git-subtree git subtree, which is a fun wordplay, but perhaps ironically, as a single rarely-changing file, git-subtree is probably not the right tool for these purposes :)

Have fun,

Avery

greened@obbligato.org (David A. Greene) writes:

So what's the next step? I guess one of the git maintaners will have to do a pull and merge. Anything I need to do on this end for that to happen?

Thanks!

                             -Dave

On Wed, Feb 15, 2012 at 10:07:16PM -0600, David A. Greene wrote:

I'm not a git-subtree user, nor am I the maintainer who would pull from you. So I am somewhat on the sidelines of this particular discussion.

Usually we would incubate new and radically different commands in contrib, and then if they prove to be good, make first-class commands of them (e.g., git-new-workdir has been in contrib for a while, and as it has proven itself to many people, there is talk of including it as a core command).

My impression is that git-subtree has already done this incubation and proving step in its own repository (but like I said, I do not use it myself, so that is just going on list hearsay). So it seems like the logical step would be to graduate into the main git repository. And I gather from Avery's response that he agrees.

Of course there's no real reason we can't take it slow by putting it in contrib, and then graduating from there. It just seems like an unnecessary and complicated interim step. Either way, I do think it's worth saving the commit history by doing a real merge.

I dunno. It is really up to Junio, I guess. He usually relies on list consensus for decisions like this, and there has not been that much discussion. What do users of git-subtree think, as this would primarily benefit them? And what do other members of the git@vger community who do not use git-subtree think of the burden of carrying it as a first-class command (not so much the burden of adding it, but of maintaining it, fielding reports when it is broken, etc)?

As a non-user, I am totally fine with it. I think the burden is not that high, and you have already promised to deal with ongoing maintenance issues.

-Peff

Jeff King <peff@peff.net> writes:

It sounds like the simplest and cleanest would be to treat it as if its current version came as a patch submission, cook it just like any other topic in 'pu' down to 'next' down to eventually 'master', with the usual review cycle of pointing out what is wrong and needs fixing followed by a series of re-rolls.

The total amount of change does not look too bad, either:

    $ git diff --stat master...origin/subtree
     contrib/subtree/.gitignore         |    5 +
     contrib/subtree/COPYING            |  339 +++++++++++++++++
     contrib/subtree/Makefile           |   45 +++
     contrib/subtree/README             |    8 +
     contrib/subtree/git-subtree.sh     |  712 ++++++++++++++++++++++++++++++++++++
     contrib/subtree/git-subtree.txt    |  366 ++++++++++++++++++
     contrib/subtree/t/Makefile         |   71 ++++
     contrib/subtree/t/t7900-subtree.sh |  508 +++++++++++++++++++++++++
     contrib/subtree/todo               |   50 +++
     t/test-lib.sh                      |   11 +-
     10 files changed, 2114 insertions(+), 1 deletion(-)

It does look like it needs to start its life in contrib/ if we were to put this in git.git. I haven't looked at the script fully, but it has an issue from its first line, which is marked with "#!/bin/bash". It is unclear if it is infested by bash-isms beyond repair (in which case "#!/bin/bash" is fine), or it was written portably but was marked with "#!/bin/bash" just by inertia. A patch that corresponds to the above diffstat immediately shows many style issues including trailing eye-sore whitespaces.

It seems that it is even capable of installing from contrib/subtree, so keeping it in contrib/ while many issues it may have gets fixed would not hurt the original goal of giving the script more visibility.

The change to t/test-lib.sh should be made independent of this topic, I would think.

----------------------------------------------------------------

Junio C Hamano <gitster@pobox.com> writes:

Ok, but we will preserve the history via the subtree merge, yes?

Yes, it's a fairly small tool.

That sounds good to me.  It should get a good shakedown before graduating.

Ok.

Right, I intentially designed it that way.

Ok, I'll propose those changes separately. They are a prerequisite for a git-subtree that is easily testable while in contrib.

I'll put a patch together with a more extensive explanation. Basically, tests run outside of the top-level t/ directory don't work because there are all sort of assumptions in test-lib.sh about where they live. There are comments in test-lib.sh indicating that it should support tests in other directories but I could not make it work out of the box.

Ok, these are Avery's commits. I don't know that I have enough context to improve the logs but I will look throught revisions and try to figure things out. Avery, could you be of any help here? It sounds like we need more descriptive log messages.

                               -Dave

greened@obbligato.org (David A. Greene) writes:

I'll comment on just this part, but a short answer is "no, I do not think so".

Even though you left "Jeff King writes", you removed everything he said that I was quoting, and in order to understand why the answer is 'no', it would have been better if you kept this part from what he said in your reply:

as that was what I was agreeing to with my "as if ... a patch submission".

That was not what I was suggesting.

I was saying that the history up to the current state, littered with these commits that are not "logical progression" but merely "a snapshot of then-current state" may not be worth preserving, with or without better messages.

Rewriting the entire history to make it a logical progression just for the sake of history is obviously not worth the effort.

Which suggests that taking the end result that exists at the tip of your subtree branch as a single code-drop, without pulling its history, lets us start from a reasonably well tested state and would not lose us anything of value. And that was what I was suggesting. For our history to explain why/how the code got there better, another approach might be to instead treat your bd7b2cf (Add 'contrib/subtree/' from commit '2793ee6ba...', 2012-01-29), which is where you took Avery's then-current state, as the code-drop event that adds everything in contrib/subtree/ with a single patch submission. I.e. in a git.git repository:

	git checkout -b subtree master
	git fetch git://sources.obbligato.org/git/git.git subtree
        git merge --squash bd7b2cf
        git commit -m "contrib: add git-subtree from Avery's tree"

to take the tip of your subtree branch. The history up to that point is in Avery's repository where he stopped, which such an approach will not pull in to git.git. And then we can replay bd7b2cf..FETCH_HEAD like so:

	git checkout FETCH_HEAD
	git rebase --onto subtree bd7b2cf
	git push . HEAD:subtree
        git checkout pu
        git merge subtree

to preserve the history since that single code-drop event that records your effort to adjust the code-dump into a better shape to live in our contrib/ area. That will make it clear the division of blame on the code added to git.git between Avery (everything before the squashed merge) and you (everything after that).

Junio C Hamano <gitster@pobox.com> writes:

Ehh, s/agree/disagree/;

Junio C Hamano <gitster@pobox.com> writes:

Having said all that, my preference is not so strong to out-right veto doing a true merge; I wouldn't lose sleep if we end up merging the tip of your subtree branch with all the history behind it as-is.

BUT.

Even though I freely admit that I was the guilty one who came up with "merge -s subtree", and Linus's "gitk merge" was the original sin, having a subtree merge like gitk, git-gui and gitweb in the history is not without downsides.

The most problematic one that we regularly suffer from is that the commands in the log family cannot work well across a subtree merge with pathspec limiting, e.g. "git log git-gui/po", and we have to resort to something like:

    $ cd git-gui/po &&
      git rev-list --parents HEAD . |
      while read commit parent
      do
        git log --pretty=short $parent..$commit^2 -- :/po
      done | git shortlog -n -e

to achieve what should be a simple "git shortlog -n -e git-gui/po". I suspect that a subtree merge may also lead bisection into uninteresting tangents as it joins otherwise disjoint history.

If we still have an active upstream that grows its history in a separate repository, like gitk and git-gui do, we cannot avoid a subtree merge in order to continue merging from them. Because you seem to be taking over and are going to maintain it as part of git.git proper, eventually aiming to move it out of contrib/, it's just that I do not think it is worth the trouble.

greened@obbligato.org (David A. Greene) writes:

Note that this will conflict with tr/perftest, which is already in next. It had a similar override, but then made do with the existing GIT_TEST_INSTALLED facility. I think you do need the above here, because you are not sourcing test-lib from the directory where the test lives. It may then be cleaner to again use GIT_BUILD_DIR in t/perf/perf-lib.sh since it does not require knowledge (outside of test-lib) that $GIT_BUILD_DIR/bin-wrappers/ holds the binaries.

On Mon, Feb 20, 2012 at 6:14 PM, Junio C Hamano <gitster@pobox.com> wrote:

Yeah, my original intent with git-subtree was to one day submit it as basically a single patch against git. That's why I have some slightly suspicious commit messages in there (though in my defense, I think *most* of the commit messages are quite sensible :)).

That commit message in particular I thought was perfectly fine; it's specifically a fix to the git-subtree docs to clarify a question from an actual user.

Overall I agree that there's little benefit in preserving the history, at least as far as I can see, *except* that some code changes were submitted by people other than me and squashing those changes might conceivably cause licensing confusion down the road. It's probably a fairly quick exercise with git-filter-branch to get rid of the more egregious commit message problems, if that's what we want to do. (In particular, just expurgating the entire 'todo' file from the history probably makes plenty of sense.)

There's no value I can see in being able to do future merges from outside the tree, so a filter-branch or rebase before merging should be pretty harmless.

Note that COPYING, .gitignore, Makefile, t/Makefile, todo, and README should probably be ditched if it weren't going into contrib. The interesting files are git-subtree.{sh,txt} and t7900-subtree.sh.

I'm generally pretty careful to avoid bashisms, but since my /bin/sh is bash, I usually mark scripts with /bin/bash just to be safe until someone has actually verified them with a non-bash shell. I expect few if any problems with that part.

Personally, I would prefer to just iterate the patch a few times to correct the coding style problems you see, rather than merging into contrib where it might be forgotten rather than fixed. As Peff alluded, people who want to install it separately from git already can; if we're going to merge it into git, let's do it right.

Have fun,

AVery

Avery Pennarun <apenwarr@gmail.com> writes:

That is a good point, and it sounds like a good enough justification to merge with history, at least for me.

On Fri, Feb 24, 2012 at 3:56 PM, Junio C Hamano <gitster@pobox.com> wrote:

Should we filter-branch or rebase the history first, or just leave it as is?

Like I said, since I don't expect there to be any more back-and-forth development, rebasing should be pretty harmless.

Avery

Avery Pennarun <apenwarr@gmail.com> writes:

Catching up on e-mail.  :)

I'm happy to do either (rebase or filter-branch). Just let me know. I'm about the send the test-lib.sh patch separately as it's a prereq for putting git-subtrees tests in contrib and I think it's generally useful anyway.

                           -Dave

greened@obbligato.org (David A. Greene) writes:

I would understand Avery's "should we filter-branch/rebase, or is it OK as-is?", but I do not understand what you mean by "either rebase or filter-branch is fine".

Thanks, this I understand and should not be a part of git-subtree topic.

Junio C Hamano <gitster@pobox.com> writes:

Sorry, got mixed up there. I'm not that familiar with filter-branch. Now I understand you do both. :)

So have we decided to keep the history?

                                 -Dave

greened@obbligato.org (David A. Greene) writes:

I think the discussion so far was:

 - Peff suggested to keep the history with a true merge;

 - I said the history before the final commit in Avery's tree did not look
   so useful for future archaeology; and then

 - Avery corrected me that there are contributions by other people and the
   credits will be lost if we discarded the history;

and everybody (including me) now favors to have the history.

So the answer to your question is yes, but I do not think we heard opinion from anybody regarding the question by Avery yet. I personally do not see how it would help us if the old history is rewritten at this point.

On Mon, Feb 27, 2012 at 01:06:02PM -0800, Junio C Hamano wrote:

Yeah, I don't see much point in rewriting. If parts of the history suck, then so be it. It's probably not that big to store. And while it's sometimes easier to fix bad commit messages when they are recent and in your memory (rather than trying to remember later what you meant to say), I think it is already too late for that. Any archaeology you do now to make good commit messages could probably just as easily be done if and when somebody actually needs the commit message later (emphasis on the "if" -- it's likely that nobody will care about most of the commit messages later at all).

-Peff

On Mon, Feb 27, 2012 at 04:21:57PM -0500, Jeff King wrote:

Sorry, the "you" there is meant to be David. Forgot who I was responding to for a minute.

-Peff

Jeff King <peff@peff.net> writes:

Anyway we already have subtree merges if subsystem with bad error messages -- see gitweb.

On Mon, Feb 27, 2012 at 9:04 PM, Jakub Narebski <jnareb@gmail.com> wrote:

So be it then! May my lame commit messages persist forever! As if I don't have enough embarrassing stuff on the Internet.

(Personally I think the vast majority of the commit messages are perfectly fine, and the ones that aren't generally describe boring commits anyway, like changes to the 'todo' file.)

Have fun,

Avery

Junio C Hamano <gitster@pobox.com> writes:

Looks like eveyone's on board to keep the history as-is. I cleaned a few things up and will re-post the subtree once the test-lib.sh changes go through.

                        -Dave

Jeff King <peff@peff.net> writes:

I have no problem starting off in contrib. I was asking more about the logistics of getting there.

Indeed, I'm more than happy to help with maintenance. I have one or two fixes/enhancements on my list, in fact.

                              -Dave