Git development
 help / color / mirror / Atom feed
* Re: [PATCH/RFD] builtin-revert.c: release index lock when cherry-picking an empty commit
From: Jeff King @ 2009-03-08 14:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Chris Johnsen, Miklos Vajna
In-Reply-To: <1236418251-16947-1-git-send-email-chris_johnsen@pobox.com>

On Sat, Mar 07, 2009 at 03:30:51AM -0600, Chris Johnsen wrote:

> +test_expect_code 1 'cherry-pick an empty commit' '
> +
> +	git checkout master &&
> +	git cherry-pick empty-branch
> +
> +'

Hmm. This test fails for me on FreeBSD. However, it seems to be related
to a shell portability issue with the test suite. The extra newline
inside the shell snippet seems to lose the last status. The following
works fine for me with bash or dash:

-- >8 --
#!/bin/sh

test_description='test that shell handles whitespace in eval'
. ./test-lib.sh

test_expect_code 1 'no newline' 'false'
test_expect_code 1 'one newline' 'false
'
test_expect_code 1 'extra newline' 'false

'

test_done
-- 8< --

but on a FreeBSD 6.1 box generates:

  *   ok 1: no newline
  *   ok 2: one newline
  * FAIL 3: 1
          extra newline false

With this minimal example, you can see that the problem is not some
subtle bug in the test suite:

-- >8 --
#!/bin/sh

eval 'false'
echo status is $?
eval 'false
'
echo status is $?
eval 'false

'
echo status is $?
-- 8< --

generates:

  status is 1
  status is 1
  status is 0

which means that any tests of the form

  test_expect_success description '

      contents

  '

are not actually having their exit code checked properly, and are just
claiming success regardless of what happened. So this definitely needs
to be addressed, I think. I'm not sure of the best course of action,
though. Our options include:

  1. Declare appended newline a forbidden style, fix all existing cases
     in the test suite, and be on the lookout for new ones.

     The biggest problem with this option is that we have no automated
     way of policing. Such tests will just silently pass on the broken
     platform.

  2. Have test_run_ canonicalize the snippet by removing trailing
     newlines.

  3. Declare FreeBSD's /bin/sh unfit for git consumption, and require
     bash for the test suite.

I think (2) is the most reasonable option of those choices.

We could also try to convince FreeBSD that it's a bug, but that doesn't
change the fact that the tests are broken on every existing version.

-Peff

^ permalink raw reply

* Re: Git for Windows 1.6.2-preview20090308
From: Jakub Narebski @ 2009-03-08 13:26 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, msysgit
In-Reply-To: <alpine.DEB.1.00.0903080132470.10279@pacific.mpi-cbg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Known issues

> - The Logitec QuickCam software can cause spurious crashes. See "Why does 
>   make often crash creating a sh.exe.stackdump file when I try to compile 
>   my source code?" in the MinGW FAQs 
>   (http://www.mingw.org/MinGWiki/index.php/FAQ).

You meant issue described here?
  http://www.mingw.org/wiki/Environment_issues

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: Gitk - Seeing just a specific remote ?
From: Aneesh Bhasin @ 2009-03-08 13:01 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git
In-Reply-To: <49B0D8FD.6070807@viscovery.net>

On Fri, Mar 6, 2009 at 1:34 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Aneesh Bhasin schrieb:
>> I have a git repository (say, new_develop) in which a remote
>> repository (say old_develop) is also there of some older work with the
>> associated remote branches. Is there a way to see all  the branches of
>> only this remote older_develop repository graphically in gitk -
>> something that shows me the same thing that doing a 'gitk --all' would
>> have shown had I done it from the older_develop repository itself ? If
>> I say 'gitk --all' (in new_develop) it shows me all the branches
>> (local as well as remote). Specifying 'gitk --remotes' also shows all
>> the remote branches (not just from the old_develop remote repo) ? Is
>> there some other way that I am missing ? I have seen the man page of
>> git-rev-list too but there doesn't sem to be a way to do it.
>
> gitk has an option, --argscmd, that you can use:
>
>  gitk --argscmd="git for-each-ref --format='%(refname)'
> refs/remotes/old_develop/*"
>

Thanks. That was exactly what I was looking for.

^ permalink raw reply

* Re: Git for Windows 1.6.2-preview20090308
From: Lee Henson @ 2009-03-08 12:59 UTC (permalink / raw)
  To: Johannes.Schindelin; +Cc: git, msysgit
In-Reply-To: <alpine.DEB.1.00.0903080132470.10279@pacific.mpi-cbg.de>

[-- Attachment #1: Type: text/plain, Size: 2857 bytes --]

Thanks to everyone involved in the production of this installer!

2009/3/8 Johannes Schindelin <Johannes.Schindelin@gmx.de>

>
> Hi,
>
> I just released a new version of Git for Windows (TAFKA WinGit).  It is
> basically Git 1.6.2 plus a few patches.  Please find the installer here:
>
>        http://msysgit.googlecode.com/
>
> Disclaimer: Git for Windows is still in a state where I do _not_ recommend
> using it unless you have the means to fix issues.  Unlike git.git
> developer community, the msysGit team is heavily undermanned.
>
> Known issues
>
> - Some commands are not yet supported on Windows and excluded from the
>  installation; namely: git archimport, git cvsexportcommit, git
>  cvsimport, git cvsserver, git filter-branch, git instaweb, git
>  send-email, git shell.
>
> - The Logitec QuickCam software can cause spurious crashes. See "Why does
>  make often crash creating a sh.exe.stackdump file when I try to compile
>  my source code?" in the MinGW FAQs
>  (http://www.mingw.org/MinGWiki/index.php/FAQ).
>
> - The Quick Launch icon will only be installed for the user running setup
>  (typically the Administrator). This is a technical restriction and will
>  not change.
>
> - Git Bash launched through the Explorer shell extension does not have the
>  git icon in its taskbar. This is a technical restriction and will not
>  change.
>
> - git send-mail does not work properly (Issue 27).
>
> - curl uses $HOME/_netrc instead of $HOME/.netrc.
>
> - If you want to specify a different location for --upload-pack, you have
>  to start the absolute path with two slashes. Otherwise MSys will mangle
>  the path.
>
> - git clone fails when the repository contains UTF-8 filepaths (Issue 80).
>
> Changes since Git-1.6.1-preview20081225
>
> New Features
> - Comes with official git 1.6.2.
> - Comes with upgraded vim 7.2.
> - Compiled with GCC 4.3.3.
> - The user can choose the preferred CR/LF behavior in the installer now.
> - Peter Kodl contributed support for hardlinks on Windows.
> - The bash prompt now shows information about the current repository.
>
> Bugfixes
> - If supported by the file system, pack files can grow larger than 2gb.
> - Comes with updated msys-1.0.dll (should fix some Vista issues).
> - Assorted fixes to support the new libexec/git-core/ layout better.
> - Read-only files can be properly replaced now.
> - git-svn is included again (original caveats still apply).
> - Obsolete programs from previous installations are cleaned up.
>
>
> So what are the next steps?
>
> Hannes is busy sorting out the differences between the test suites in
> git.git and mingw.git.
>
> Thanks to the awesome efforts of both Hannes and Steffen, the rest of the
> differences are really small (the biggest being Peter's hard link patch).
> I'll try to put together a patch series in the next few weeks.
>
> Ciao,
> Dscho
>
>

[-- Attachment #2: Type: text/html, Size: 3528 bytes --]

^ permalink raw reply

* Re: Git for Windows 1.6.2-preview20090308
From: Johannes Schindelin @ 2009-03-08 11:54 UTC (permalink / raw)
  To: Alexander Gavrilov; +Cc: msysgit, git, Junio C Hamano
In-Reply-To: <200903081413.28354.angavrilov@gmail.com>


Hi,

On Sun, 8 Mar 2009, Alexander Gavrilov wrote:

> On Sunday 08 March 2009 04:10:21 Johannes Schindelin wrote:
> > I just released a new version of Git for Windows (TAFKA WinGit).  It is 
> > basically Git 1.6.2 plus a few patches.  Please find the installer here:
> > 
> > 	http://msysgit.googlecode.com/
> > 
> > Disclaimer: Git for Windows is still in a state where I do _not_ recommend 
> > using it unless you have the means to fix issues.  Unlike git.git 
> > developer community, the msysGit team is heavily undermanned.
> > 
> > Known issues
> 
> I've just noticed that the following git-gui fixes haven't been merged in 1.6.2:
> 
>   git-gui: Support more git version notations.
>   git-gui: Avoid an infinite rescan loop in handle_empty_diff.
>   git-gui: Fix post-commit status with subject in non-locale encoding
> 
> The second one is a fix for a rather important problem reported on the msysgit list.
> The last one is msysgit bug #181.

Thanks for keeping an eye on things.  Would you agree that we can wait a 
few days, maybe this whole week, to collect issues like these, and then 
release another installer with the issues fixed?

Ciao,
Dscho

^ permalink raw reply

* Re: Git for Windows 1.6.2-preview20090308
From: Alexander Gavrilov @ 2009-03-08 11:13 UTC (permalink / raw)
  To: msysgit, Johannes.Schindelin; +Cc: git, Junio C Hamano
In-Reply-To: <alpine.DEB.1.00.0903080132470.10279@pacific.mpi-cbg.de>


On Sunday 08 March 2009 04:10:21 Johannes Schindelin wrote:
> I just released a new version of Git for Windows (TAFKA WinGit).  It is 
> basically Git 1.6.2 plus a few patches.  Please find the installer here:
> 
> 	http://msysgit.googlecode.com/
> 
> Disclaimer: Git for Windows is still in a state where I do _not_ recommend 
> using it unless you have the means to fix issues.  Unlike git.git 
> developer community, the msysGit team is heavily undermanned.
> 
> Known issues

I've just noticed that the following git-gui fixes haven't been merged in 1.6.2:

  git-gui: Support more git version notations.
  git-gui: Avoid an infinite rescan loop in handle_empty_diff.
  git-gui: Fix post-commit status with subject in non-locale encoding

The second one is a fix for a rather important problem reported on the msysgit list.
The last one is msysgit bug #181.

Alexander

^ permalink raw reply

* Re: [PATCH] http: add_fill_function checks if function has been added
From: Tay Ray Chuan @ 2009-03-08 10:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd4ctm29m.fsf@gitster.siamese.dyndns.org>

Hi,

On Sun, Mar 8, 2009 at 5:49 AM, Junio C Hamano <gitster@pobox.com> wrote:
> I didn't look at the callers of add_fill_function(), but "fill" takes a
> callback data and different invocation of add_fill_function() could be
> passing different callback data.  In such a case, doesn't it feel wrong to
> omit the "duplicated" calls to register the fill callback?  Your patch
> makes me suspect that it _might_ be better to fix the callers not to call
> the function repeatedly when they know they only want one-shot invocation.

Omitting duplicate calls in fill_active_slots does not mean that
repeated calls of the fill functions won't take place.

In the current use instances in http-push and http-walker,
run_active_slot and finish_all_active_slots (which calls
run_active_slot) will always be called, because they are the functions
that actually start the http requests[1], and that means the fill
functions will be called repeated, because

run_active_slot, in its while loop, repeatedly calls
  step_active_slots calls
    fill_active_slots calls
      our fill functions.

fill_active_slots is the only direct caller of fill functions
(duplicate ones included).

So we can be sure that the fill functions are called repeatedly (at
least for the slot's active duration).

[1] Actually calling step_active_slots will start http requests,
without you having to call run_active_slot.

-- 
Cheers,
Ray Chuan

^ permalink raw reply

* Re: [PATCH 0/5] Extend pattern refspecs
From: Daniel Barkalow @ 2009-03-08  8:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jay Soffian, git
In-Reply-To: <7vocwcl8ku.fsf@gitster.siamese.dyndns.org>

On Sun, 8 Mar 2009, Junio C Hamano wrote:

> Jay Soffian <jaysoffian@gmail.com> writes:
> 
> > This series and js/remote-improvements (e5dcbfd) in pu may not get
> > along completely. "git remote show" tries to show how the refspecs
> > expand out.
> 
> I've created an "early semantic conflict resolution" topic branch that is
> a cross between this series and js/remote-improvements, like so:
> 
>     $ git checkout -b xx/db-refspec-vs-js-remote db/refspec-wildcard-in-the-middle
>     $ git merge js/remote-improvements
>     $ git apply evil-fixup.diff
>     $ git commit --amend -a -m "Evil merge."
> 
> with the following "fixup-as-an-evil-merge patch", which I'd appreciate if
> you two can sanity check.

That looks like what I'd come up with as a resolution, too, so I think 
it's sane unless Jay knows of another way to get remote to care about 
patterns.

> The result will be queued at the tip of 'pu', and hopefully I can merge it
> to 'next' when this series goes to 'next'.  Similarly, whichever one goes
> first to 'master' can be merged straight, but the merge of the other one
> needs to merge this branch as well.
> 
> diff --git a/builtin-remote.c b/builtin-remote.c
> index 7e82a52..3e4a41b 100644
> --- a/builtin-remote.c
> +++ b/builtin-remote.c
> @@ -359,14 +358,9 @@ static int get_push_ref_states_noquery(struct ref_states *states)
>  	}
>  	for (i = 0; i < remote->push_refspec_nr; i++) {
>  		struct refspec *spec = remote->push + i;
> -		char buf[PATH_MAX];
>  		if (spec->matching)
>  			item = string_list_append("(matching)", &states->push);
> -		else if (spec->pattern) {
> -			snprintf(buf, (sizeof(buf)), "%s*", spec->src);
> -			item = string_list_append(buf, &states->push);
> -			snprintf(buf, (sizeof(buf)), "%s*", spec->dst);
> -		} else if (strlen(spec->src))
> +		else if (strlen(spec->src))
>  			item = string_list_append(spec->src, &states->push);
>  		else
>  			item = string_list_append("(delete)", &states->push);
> @@ -374,10 +368,7 @@ static int get_push_ref_states_noquery(struct ref_states *states)
>  		info = item->util = xcalloc(sizeof(struct push_info), 1);
>  		info->forced = spec->force;
>  		info->status = PUSH_STATUS_NOTQUERIED;
> -		if (spec->pattern)
> -			info->dest = xstrdup(buf);
> -		else
> -			info->dest = xstrdup(spec->dst ? spec->dst : item->string);
> +		info->dest = xstrdup(spec->dst ? spec->dst : item->string);
>  	}
>  	return 0;
>  }
> @@ -390,7 +381,7 @@ static int get_head_names(const struct ref *remote_refs, struct ref_states *stat
>  
>  	refspec.force = 0;
>  	refspec.pattern = 1;
> -	refspec.src = refspec.dst = "refs/heads/";
> +	refspec.src = refspec.dst = "refs/heads/*";
>  	states->heads.strdup_strings = 1;
>  	get_fetch_map(remote_refs, &refspec, &fetch_map_tail, 0);
>  	matches = guess_remote_head(find_ref_by_name(remote_refs, "HEAD"),
> 

^ permalink raw reply

* Re: [PATCH 0/5] Extend pattern refspecs
From: Junio C Hamano @ 2009-03-08  8:31 UTC (permalink / raw)
  To: Jay Soffian; +Cc: Daniel Barkalow, git
In-Reply-To: <76718490903052119y4d6a7e0ck24bfeb1c0964e413@mail.gmail.com>

Jay Soffian <jaysoffian@gmail.com> writes:

> This series and js/remote-improvements (e5dcbfd) in pu may not get
> along completely. "git remote show" tries to show how the refspecs
> expand out.

I've created an "early semantic conflict resolution" topic branch that is
a cross between this series and js/remote-improvements, like so:

    $ git checkout -b xx/db-refspec-vs-js-remote db/refspec-wildcard-in-the-middle
    $ git merge js/remote-improvements
    $ git apply evil-fixup.diff
    $ git commit --amend -a -m "Evil merge."

with the following "fixup-as-an-evil-merge patch", which I'd appreciate if
you two can sanity check.

The result will be queued at the tip of 'pu', and hopefully I can merge it
to 'next' when this series goes to 'next'.  Similarly, whichever one goes
first to 'master' can be merged straight, but the merge of the other one
needs to merge this branch as well.

diff --git a/builtin-remote.c b/builtin-remote.c
index 7e82a52..3e4a41b 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -359,14 +358,9 @@ static int get_push_ref_states_noquery(struct ref_states *states)
 	}
 	for (i = 0; i < remote->push_refspec_nr; i++) {
 		struct refspec *spec = remote->push + i;
-		char buf[PATH_MAX];
 		if (spec->matching)
 			item = string_list_append("(matching)", &states->push);
-		else if (spec->pattern) {
-			snprintf(buf, (sizeof(buf)), "%s*", spec->src);
-			item = string_list_append(buf, &states->push);
-			snprintf(buf, (sizeof(buf)), "%s*", spec->dst);
-		} else if (strlen(spec->src))
+		else if (strlen(spec->src))
 			item = string_list_append(spec->src, &states->push);
 		else
 			item = string_list_append("(delete)", &states->push);
@@ -374,10 +368,7 @@ static int get_push_ref_states_noquery(struct ref_states *states)
 		info = item->util = xcalloc(sizeof(struct push_info), 1);
 		info->forced = spec->force;
 		info->status = PUSH_STATUS_NOTQUERIED;
-		if (spec->pattern)
-			info->dest = xstrdup(buf);
-		else
-			info->dest = xstrdup(spec->dst ? spec->dst : item->string);
+		info->dest = xstrdup(spec->dst ? spec->dst : item->string);
 	}
 	return 0;
 }
@@ -390,7 +381,7 @@ static int get_head_names(const struct ref *remote_refs, struct ref_states *stat
 
 	refspec.force = 0;
 	refspec.pattern = 1;
-	refspec.src = refspec.dst = "refs/heads/";
+	refspec.src = refspec.dst = "refs/heads/*";
 	states->heads.strdup_strings = 1;
 	get_fetch_map(remote_refs, &refspec, &fetch_map_tail, 0);
 	matches = guess_remote_head(find_ref_by_name(remote_refs, "HEAD"),

^ permalink raw reply related

* Re: [RFC PATCH] git-svn does not support intermediate directories?
From: Eric Wong @ 2009-03-08  4:43 UTC (permalink / raw)
  To: Michael Lai; +Cc: Tim Stoakes, git
In-Reply-To: <21fc26450903051612u1400b2b4gd71c3eafa4418e37@mail.gmail.com>

Michael Lai <myllai@gmail.com> wrote:
> I did some additional hacking and may have found a slightly cleaner
> way of at least fixing the problems with "git svn fetch".  The problem
> with the wrong paths being initialized for branches and tags is fairly
> minor (since you can just edit the config by hand), so I'll probably
> address that later, if I have time.  Here's the patch (I hope I'm
> doing this right):

Hi Michael,

Your patch was whitespace damaged and lacked a proposed commit message.
Please read Documentation/SubmittingPatches next time.

Anyhow, I fixed your patch up a bit.  Can you sign-off on it
if its right to you or let me know if it's broken?  Thanks.

From cddc7e5bde060eb963534156ae0daaf41c87c21a Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sat, 7 Mar 2009 20:22:29 -0800
Subject: [PATCH] git-svn: support intermediate paths when matching tags/branches
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

For repositories laid out like the following:

> [svn-remote "svn"]
>       url = http://foo.com/svn/repos/bar
>       fetch = myproject/trunk:refs/remotes/trunk
>       branches = bar/myproject/branches/*:refs/remotes/*
>       tags = bar/myproject/tags/*:refs/remotes/tags/*

The "bar" component above is considered the intermediate path
and was not handled correctly.

This patch was originally by Michael Lai (without a commit
message) with some minor fixes:

  * extraneous slash removed from $intermediate_path,
    this was causing tests to fail.

  * fixed a case where $intermediate_path could be "0" and
    considered false by Perl, preventing the necessary
    slash from being appended.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-svn.perl |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 959eb52..745dd03 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2351,7 +2351,11 @@ sub match_paths {
 	if (my $path = $paths->{"/$self->{path}"}) {
 		return ($path->{action} eq 'D') ? 0 : 1;
 	}
-	$self->{path_regex} ||= qr/^\/\Q$self->{path}\E\//;
+	my $repos_root = $self->ra->{repos_root};
+	my $intermediate_path = $self->{url};
+	$intermediate_path =~ s#^\Q$repos_root\E(/|$)##;
+	$intermediate_path .= '/' if length($intermediate_path) > 0;
+	$self->{path_regex} ||= qr/^\/\Q$intermediate_path$self->{path}\E\//;
 	if (grep /$self->{path_regex}/, keys %$paths) {
 		return 1;
 	}
-- 
Eric Wong

^ permalink raw reply related

* Re: [PATCH/RFD] builtin-revert.c: release index lock when cherry-picking an empty commit
From: Junio C Hamano @ 2009-03-08  4:14 UTC (permalink / raw)
  To: Chris Johnsen; +Cc: git, Miklos Vajna
In-Reply-To: <1236418251-16947-1-git-send-email-chris_johnsen@pobox.com>

Chris Johnsen <chris_johnsen@pobox.com> writes:

> When a cherry-pick of an empty commit is done, release the lock
> held on the index.
>
> The fix is the same as was applied to similar code in 4271666046.
>
> Signed-off-by: Chris Johnsen <chris_johnsen@pobox.com>

Thanks.  Will apply.  We should handle possible refactoring as a separate
topic.

> UNEVEN TREATMENT OF EMPTY CHANGES
>
> It seems that empty commits suffer uneven treatment under various
> patch-transport/history-rewriting mechanisms. They seem to be
> handled okay in the most of the core (fetch, push, bundle all
> seem to preserve empty commits, though I have not done rigorous
> testing).

They just transfer an existing history from one place to another without
modifying, so it is unfortunately true that they preserve such a broken
history with empty commits.

> 'format-patch', 'send-email', 'apply', 'am', 'rebase' (automatic,
> non-fast-forward; and --interactive).

These are all about creating history afresh, and they actively discourage
empty commits to be (re)created.

There is no "uneven treatment".

> 36863af16e (git-commit --allow-empty) says "This is primarily for
> use by foreign scm interface scripts.". Is this the only case
> where empty commits _should_ be used?

If foreign scm recorded an empty commit, it would be nice to be able to
recreate such a broken history _if the user wanted to_, and that is where
the --allow-empty option can be used.

^ permalink raw reply

* JGit server performance
From: Shawn O. Pearce @ 2009-03-08  3:22 UTC (permalink / raw)
  To: git

As Gerrit Code Review provides Gitosis-like functionality, but
that is implemented through JGit's pure Java implementation, the
performance of JGit's UploadPack matters to me.

JGit is about 66% slower on my hardware when cloning the linux-2.6
repository, compared to git-core (jgit 2m21s, git-core 1m23s).

The bottlenecks are:

  ~41.2% in ObjectLoader.getCachedBytes()

    This is the tree objects being parsed out of the pack file.
    The problem here (I believe) is we have horrible locality
    when reading.  The delta base for a tree isn't in memory most
    of the time, because its been evicted by other trees accessed
    since the last time that tree was touched.

    Conceptually this makes some sense, as ObjectWalk does a depth
    first traversal through the tree of each commit, in most-recent
    to least-recent commit order.  On a larger project like the
    kernel we'll touch a lot more objects between two root trees,
    and there isn't even any guarantee that two root trees that
    appear near each other in the commit sequence have a delta
    base relationship.

  ~20.5% in AbstractTreeIterator.getEntryObjectId()

    The bulk of the time here is really down in NB.decodeInt32().
    We spend a lot of time converting an object id in a tree data
    stream into an AnyObjectId (really a reused MutableObjectId)
    so that we can probe the ObjectIdSubclassMap to see if we have
    seen this object before.

    The sad fact is, we need all 20 bytes to be converted into the 5
    words, because the majority of the time, we have actually seen
    the object before, and it exists in our hash table.  The only
    way to know is to convert and compare all 5 words.  Any attempt
    to lazily convert the 5 words would just make it slower.

... and it falls off from there.

I'm at a loss on how to improve the performance.  I don't think that
we can do anything about the 20% in getEntryObjectId() due to the
way our data structures are organized around the 5-word ObjectId,
and not a byte[].  That 20% is a penalty git-core doesn't have to
pay, and is most certainly one reason why JGit is so much slower.

The only thing that may work is to modify ObjectWalk to try and
deduce some delta-chain locality from the pack.  Buffer up objects
that it needs to parse in a queue, rank them by the delta base
they would need to use, and then try to unfold the base first,
and then the children.

That is, do something like what IndexPack does, where we try to
unpack each object exactly once, and recursively process the delta
chain children after unpacking the parent.

We _might_ get better locality if we can queue up all root trees,
process all of them, then process the first level children, etc,
so go breadth first.

But that seems like a lot of code, and it probably wrecks the simple
recency ordering produced natively by ObjectWalk.  :-\

-- 
Shawn.

^ permalink raw reply

* Re: Git for Windows 1.6.2-preview20090308
From: Sverre Rabbelier @ 2009-03-08  2:07 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, msysgit
In-Reply-To: <alpine.DEB.1.00.0903080132470.10279@pacific.mpi-cbg.de>

Heya,

On Sun, Mar 8, 2009 at 03:10, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> I just released a new version of Git for Windows (TAFKA WinGit).  It is
> basically Git 1.6.2 plus a few patches.  Please find the installer here:

Good to see you're carrying MSysGit forward again, both you and J6t, thanks!

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: t9500-gitweb-standalone-no-errors.sh.prb 4 errors
From: Boyd Lynn Gerber @ 2009-03-08  1:29 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Git List
In-Reply-To: <m3ab7wizz5.fsf@localhost.localdomain>

On Sat, 7 Mar 2009, Jakub Narebski wrote:
> Boyd Lynn Gerber <gerberb@zenez.com> writes:
>> Test fails for SCO OpenServer 6.0 MP4 with 1.6.2
>>
>> Below is the set -x output of the tests.
>
> Could you run it without -x, but with --debug (as option to test
> itself) instead?  Because what matters here not on which command the
> test failed (we know it is on gitweb_run), but what error message
> gitweb returned.

Yes,

> Also the problem might with in Perl modules gitweb uses (CGI, Encode,
> ...), and with Perl version used, although it is less likely...

It is a perl problem...

* FAIL 18: .git blob (file)
         gitweb_run "p=.git;a=blob;f=file"
[Sun Mar  8 01:11:44 2009] gitweb.perl: -T and -B not implemented on 
filehandles at /tmp/git-1.6.2/t/../gitweb/gitweb.perl line 2853.
* FAIL 19: .git blob_plain (file)
         gitweb_run "p=.git;a=blob_plain;f=file"
[Sun Mar  8 01:11:44 2009] gitweb.perl: -T and -B not implemented on 
filehandles at /tmp/git-1.6.2/t/../gitweb/gitweb.perl line 2853.
* FAIL 62: path_info: project/branch:file
         gitweb_run "" "/.git/master:file"
[Sun Mar  8 01:12:15 2009] gitweb.perl: -T and -B not implemented on 
filehandles at /tmp/git-1.6.2/t/../gitweb/gitweb.perl line 2853.
* FAIL 66: path_info: project/branch:/file
         gitweb_run "" "/.git/master:/file"
[Sun Mar  8 01:12:17 2009] gitweb.perl: -T and -B not implemented on 
filehandles at /tmp/git-1.6.2/t/../gitweb/gitweb.perl line 2853.

I am not really sure what to do about it.  I contacted SCO on a 
different problem with the same type of filehandle problem and they told 
me they have not implemented any thing is the OS that could do this test.

This is what I traced it to...

This is what Tim Rice and I received from SCO...

  Re: __fpending()

Tim Rice wrote:
> It looks like it needs __fpending, __fpurge, & __freading.
> But as mentioned before, we can use funflush for __fpurge.

And for __freading() you could use

#define __freading(p) ((p)->__flag & _IOREAD)

or if we were to have had it in <stdio.h>, it'd look
more like the following (for C) for various reasons:

#define __freading(p) ((void)sizeof(__filbuf(p)), ((FILE *)(p))->__flag &
_IOREAD)

> I strongly suspect these missing APIs are responsible for the
For C++, it's be an inline routine checking the flag.

> I strongly suspect these missing APIs are responsible for the
> problem I found with Perl too. See "uw714 MP4 perl" and "osr600 
MP4 perl" thread.

This hadn't been raised as a possibility as far as I've
read in that thread -- did I miss something?  Does this
mean you've built your own perl for uw7 and found that
you needed one or more of these __f...() routines?

And Boyd Gerber wrote:
Ah yes, we're doing
#if defined(__USLC__)
   return (fp->__flag & _IOREAD) != 0;
#else
   return (fp->_flag & _IOREAD) != 0;
#endif

Would you consider this test case.
......
cat > tst.pl << 'EOF'
open (FH, $ARGV[0]) || die ("Can't open $ARGV[0] -- $!\n");
print "Text\n" if (-T FH);
print "Binary\n" if (-B FH);
close (FH);
EOF
......

On OpenServer 5 if you run "perl tst.pl tst.pl" it will say
Text

On UnixWare 7.1.4 MP4 and OpenSever 6.0 MP4 if you run "perl tst.pl 
tst.pl" it will say -T and -B not implemented on filehandles at tst.pl 
line 2.

While googling I found according to a post by Randal Schwartz that this 
means perl doesn't know enough about stdio on that platform.

Looking at /opt/lib/perl5/5.8.8/i386-unixware-thread-multi/Config_heavy.pl
we see things like stdio_base='((fp)->_base)' but then we have
d_stdiobase='undef' so it is not used.

And this wouldn't work but probably isn't used because of
d_stdiobase='undef'
stdio_bufsiz='((fp)->_cnt + (fp)->_ptr - (fp)->_base)'

If we had  __fbufsiz() it might be
stdio_bufsiz='__fbufsiz(fp)'

As the test program shows, there is something missing in UnixWare's
and OpenServer 6 perl's implementation.

-- 
Boyd Gerber <gerberb@zenez.com> 801 849-0213
ZENEZ	1042 East Fort Union #135, Midvale Utah  84047

^ permalink raw reply

* [PATCH 2/2] ls-files: fix broken --no-empty-directory
From: Jeff King @ 2009-03-08  1:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Miklos Vajna, git
In-Reply-To: <20090308012049.GA18616@coredump.intra.peff.net>

Commit ce8e880 converted ls-files to use parseopt; the
--no-empty-directory option was converted as an
OPT_BIT for "empty-directory" to set the
DIR_HIDE_EMPTY_DIRECTORY flag. However, that makes it do the
opposite of what it should: --empty-directory would hide,
but --no-empty-directory would turn off hiding.

Signed-off-by: Jeff King <peff@peff.net>
---
Two comments on this patch:

  1. The original conversion gave us --empty-directory and
     --no-empty-directory. This one gives us --no-empty-directory and
     --no-no-empty-directory. If we really want to expose a negatable
     flag, then either:

       - the bit needs to change to DIR_SHOW_EMPTY_DIRECTORY; however,
         that changes the behavior for callers who zero the flag field

       - we need a custom option which implements the negation with
         respect to the bit, instead of a vanilla OPT_BIT

  2. I tried to follow the existing style of the t3000 test script
     rather than overhauling it. However, I think the result is a little
     hard to read. We set up expectations for many cases at the
     beginning, and then do all the tests. I think it might read better
     as "set up a case, create expectat, run test".

 builtin-ls-files.c         |    4 ++--
 t/t3000-ls-files-others.sh |   14 +++++++++++++-
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index 1742c0f..437c366 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -454,8 +454,8 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 		OPT_BIT(0, "directory", &dir.flags,
 			"show 'other' directories' name only",
 			DIR_SHOW_OTHER_DIRECTORIES),
-		OPT_BIT(0, "empty-directory", &dir.flags,
-			"list empty directories",
+		OPT_BIT(0, "no-empty-directory", &dir.flags,
+			"don't show empty directories",
 			DIR_HIDE_EMPTY_DIRECTORIES),
 		OPT_BOOLEAN('u', "unmerged", &show_unmerged,
 			"show unmerged files in the output"),
diff --git a/t/t3000-ls-files-others.sh b/t/t3000-ls-files-others.sh
index 36eee0f..379d963 100755
--- a/t/t3000-ls-files-others.sh
+++ b/t/t3000-ls-files-others.sh
@@ -13,12 +13,13 @@ filesystem.
     path2/file2 - a file in a directory
     path3-junk  - a file to confuse things
     path3/file3 - a file in a directory
+    path4       - an empty directory
 '
 . ./test-lib.sh
 
 date >path0
 ln -s xyzzy path1
-mkdir path2 path3
+mkdir path2 path3 path4
 date >path2/file2
 date >path2-junk
 date >path3/file3
@@ -28,6 +29,7 @@ git update-index --add path3-junk path3/file3
 cat >expected1 <<EOF
 expected1
 expected2
+expected3
 output
 path0
 path1
@@ -35,6 +37,8 @@ path2-junk
 path2/file2
 EOF
 sed -e 's|path2/file2|path2/|' <expected1 >expected2
+cat <expected2 >expected3
+echo path4/ >>expected2
 
 test_expect_success \
     'git ls-files --others to show output.' \
@@ -53,4 +57,12 @@ test_expect_success \
     'git ls-files --others --directory should not get confused.' \
     'test_cmp expected2 output'
 
+test_expect_success \
+    'git ls-files --others --directory --no-empty-directory to show output.' \
+    'git ls-files --others --directory --no-empty-directory >output'
+
+test_expect_success \
+    '--no-empty-directory hides empty directory' \
+    'test_cmp expected3 output'
+
 test_done
-- 
1.6.2.198.ge2a58

^ permalink raw reply related

* Re: [JGIT PATCH v2] Fix a race condition when loading non-mmapd byte windows
From: Shawn O. Pearce @ 2009-03-08  1:24 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1236471435-21888-1-git-send-email-spearce@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> wrote:
>  Oh hell, I missed the fact that markLoaded(ByteWindow) may be
>  dropping the last reference and need to close the file.

*and* you need to squash this in to prevent a deadlock:

Frell.  What a Saturday.

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteArrayWindow.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteArrayWindow.java
index 055a329..c07a260 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteArrayWindow.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteArrayWindow.java
@@ -100,16 +100,25 @@ void inflateVerify(final byte[] array, final int pos, final Inflater inf)
 			inf.inflate(verifyGarbageBuffer, 0, verifyGarbageBuffer.length);
 	}
 
-	synchronized void ensureLoaded(final byte[] array) {
-		if (!loaded) {
-			try {
-				provider.fd.getChannel().read(ByteBuffer.wrap(array), start);
-			} catch (IOException e) {
-				throw new RuntimeException("Cannot fault in window", e);
-			} finally {
+	void ensureLoaded(final byte[] array) {
+		boolean release = false;
+		try {
+			synchronized (this) {
+				if (!loaded) {
+					release = true;
+					try {
+						provider.fd.getChannel().read(ByteBuffer.wrap(array),
+								start);
+					} catch (IOException e) {
+						throw new RuntimeException("Cannot fault in window", e);
+					}
+					loaded = true;
+				}
+			}
+		} finally {
+			if (release) {
 				WindowCache.markLoaded(this);
 			}
-			loaded = true;
 		}
 	}
 }
\ No newline at end of file

-- 
Shawn.

^ permalink raw reply related

* [PATCH 1/2] t3000: use test_cmp instead of diff
From: Jeff King @ 2009-03-08  1:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Miklos Vajna, git
In-Reply-To: <20090308012049.GA18616@coredump.intra.peff.net>

These ancient tests predate test_cmp.

While we're at it, let's switch to our usual "expected
before actual" order of arguments; this makes the diff
output "here's what is changed from expected" instead of the
reverse.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t3000-ls-files-others.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t3000-ls-files-others.sh b/t/t3000-ls-files-others.sh
index bc0a351..36eee0f 100755
--- a/t/t3000-ls-files-others.sh
+++ b/t/t3000-ls-files-others.sh
@@ -42,7 +42,7 @@ test_expect_success \
 
 test_expect_success \
     'git ls-files --others should pick up symlinks.' \
-    'diff output expected1'
+    'test_cmp expected1 output'
 
 test_expect_success \
     'git ls-files --others --directory to show output.' \
@@ -51,6 +51,6 @@ test_expect_success \
 
 test_expect_success \
     'git ls-files --others --directory should not get confused.' \
-    'diff output expected2'
+    'test_cmp expected2 output'
 
 test_done
-- 
1.6.2.198.ge2a58

^ permalink raw reply related

* [PATCH 0/2] fix ls-files parseopt regression
From: Jeff King @ 2009-03-08  1:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Miklos Vajna, git

These patches are on top of the mv/parseopt-ls-files topic in next. The
first is a related cleanup, the second is the fix.

 1/2: t3000: use test_cmp instead of diff
 2/2: ls-files: fix broken --no-empty-directory

-Peff

^ permalink raw reply

* Re: [PATCH/RFD] builtin-revert.c: release index lock when cherry-picking an empty commit
From: Johannes Schindelin @ 2009-03-08  1:19 UTC (permalink / raw)
  To: Chris Johnsen; +Cc: git, Junio C Hamano, Miklos Vajna
In-Reply-To: <EEE56CB5-BBC7-45F4-801B-062349E07730@pobox.com>

Hi,

On Sat, 7 Mar 2009, Chris Johnsen wrote:

> On 2009 Mar 7, Johannes Schindelin wrote:
> >I wonder, though, if the real root of the problem is that there is
> >copied code.
> 
> Agreed.
> 
> >IOW I think it would be better to introduce a global
> >function that writes the index to a tree.
> 
> I am not quite sure I follow your meaning here.

Well, my thinking was that instead of imitating what merge-recursive does, 
you could refactor that very code into a function that gets called from 
both merge-recursive and revert.

Ciao,
Dscho

^ permalink raw reply

* Git for Windows 1.6.2-preview20090308
From: Johannes Schindelin @ 2009-03-08  1:10 UTC (permalink / raw)
  To: git, msysgit

Hi,

I just released a new version of Git for Windows (TAFKA WinGit).  It is 
basically Git 1.6.2 plus a few patches.  Please find the installer here:

	http://msysgit.googlecode.com/

Disclaimer: Git for Windows is still in a state where I do _not_ recommend 
using it unless you have the means to fix issues.  Unlike git.git 
developer community, the msysGit team is heavily undermanned.

Known issues

- Some commands are not yet supported on Windows and excluded from the 
  installation; namely: git archimport, git cvsexportcommit, git 
  cvsimport, git cvsserver, git filter-branch, git instaweb, git 
  send-email, git shell.

- The Logitec QuickCam software can cause spurious crashes. See "Why does 
  make often crash creating a sh.exe.stackdump file when I try to compile 
  my source code?" in the MinGW FAQs 
  (http://www.mingw.org/MinGWiki/index.php/FAQ).

- The Quick Launch icon will only be installed for the user running setup 
  (typically the Administrator). This is a technical restriction and will 
  not change.

- Git Bash launched through the Explorer shell extension does not have the 
  git icon in its taskbar. This is a technical restriction and will not 
  change.

- git send-mail does not work properly (Issue 27).

- curl uses $HOME/_netrc instead of $HOME/.netrc.

- If you want to specify a different location for --upload-pack, you have 
  to start the absolute path with two slashes. Otherwise MSys will mangle 
  the path.

- git clone fails when the repository contains UTF-8 filepaths (Issue 80).

Changes since Git-1.6.1-preview20081225

New Features
- Comes with official git 1.6.2.
- Comes with upgraded vim 7.2.
- Compiled with GCC 4.3.3.
- The user can choose the preferred CR/LF behavior in the installer now.
- Peter Kodl contributed support for hardlinks on Windows.
- The bash prompt now shows information about the current repository.

Bugfixes
- If supported by the file system, pack files can grow larger than 2gb.
- Comes with updated msys-1.0.dll (should fix some Vista issues).
- Assorted fixes to support the new libexec/git-core/ layout better.
- Read-only files can be properly replaced now.
- git-svn is included again (original caveats still apply).
- Obsolete programs from previous installations are cleaned up.


So what are the next steps?

Hannes is busy sorting out the differences between the test suites in 
git.git and mingw.git.

Thanks to the awesome efforts of both Hannes and Steffen, the rest of the 
differences are really small (the biggest being Peter's hard link patch).  
I'll try to put together a patch series in the next few weeks.

Ciao,
Dscho

^ permalink raw reply

* Re: t9500-gitweb-standalone-no-errors.sh.prb 4 errors
From: Jakub Narebski @ 2009-03-08  1:05 UTC (permalink / raw)
  To: Boyd Lynn Gerber; +Cc: Git List
In-Reply-To: <alpine.LNX.2.00.0903071645370.20607@suse104.zenez.com>

Boyd Lynn Gerber <gerberb@zenez.com> writes:

> Test fails for SCO OpenServer 6.0 MP4 with 1.6.2
> 
> Below is the set -x output of the tests.

Could you run it without -x, but with --debug (as option to test
itself) instead?  Because what matters here not on which command the
test failed (we know it is on gitweb_run), but what error message
gitweb returned.

Also the problem might with in Perl modules gitweb uses (CGI, Encode,
...), and with Perl version used, although it is less likely...

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* [JGIT PATCH v2] Fix a race condition when loading non-mmapd byte windows
From: Shawn O. Pearce @ 2009-03-08  0:17 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1236471125-21382-1-git-send-email-spearce@spearce.org>

Back in 439d441b0800 ("Change ByteArrayWindow to read content
outside of WindowCache's lock") I introduced a race condition
to WindowCache.

It is possible for a ByteArrayWindow to be allocated and put
into the cache, and then for the calling thread to get put
to sleep before it can actually load the window from disk.
By the time the thread wakes up again, the window may have
already been evicted from the cache, and its underlying file
was closed.  This results in stack traces such as:

java.lang.RuntimeException: Cannot fault in window
        at org.spearce.jgit.lib.ByteArrayWindow.ensureLoaded(ByteArrayWindow.java:111)
...
Caused by: java.nio.channels.ClosedChannelException
        at sun.nio.ch.FileChannelImpl.ensureOpen(FileChannelImpl.java:91)

Now when we allocate a ByteArrayWindow for a specific window reference
we increment the file reference count one additional time, forcing the
file to stay open even if the window was evicted from the cache.  The
reference count is decremented once the window loads successfully, and
this puts us back into the state of 1 reference count value for each
window still in the window cache.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---

 Oh hell, I missed the fact that markLoaded(ByteWindow) may be
 dropping the last reference and need to close the file.

 .../src/org/spearce/jgit/lib/ByteArrayWindow.java  |    7 +++----
 .../src/org/spearce/jgit/lib/ByteBufferWindow.java |    4 ++++
 .../src/org/spearce/jgit/lib/ByteWindow.java       |    2 ++
 .../src/org/spearce/jgit/lib/WindowCache.java      |   14 +++++++++++++-
 .../src/org/spearce/jgit/lib/WindowedFile.java     |    1 +
 5 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteArrayWindow.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteArrayWindow.java
index 7b7c7a4..055a329 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteArrayWindow.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteArrayWindow.java
@@ -68,7 +68,6 @@ ByteArrayWindow(final WindowedFile o, final long p, final int d,
 	}
 
 	int copy(final byte[] array, final int p, final byte[] b, final int o, int n) {
-		ensureLoaded(array);
 		n = Math.min(array.length - p, n);
 		System.arraycopy(array, p, b, o, n);
 		return n;
@@ -76,7 +75,6 @@ int copy(final byte[] array, final int p, final byte[] b, final int o, int n) {
 
 	int inflate(final byte[] array, final int pos, final byte[] b, int o,
 			final Inflater inf) throws DataFormatException {
-		ensureLoaded(array);
 		while (!inf.finished()) {
 			if (inf.needsInput()) {
 				inf.setInput(array, pos, array.length - pos);
@@ -91,7 +89,6 @@ int inflate(final byte[] array, final int pos, final byte[] b, int o,
 
 	void inflateVerify(final byte[] array, final int pos, final Inflater inf)
 			throws DataFormatException {
-		ensureLoaded(array);
 		while (!inf.finished()) {
 			if (inf.needsInput()) {
 				inf.setInput(array, pos, array.length - pos);
@@ -103,12 +100,14 @@ void inflateVerify(final byte[] array, final int pos, final Inflater inf)
 			inf.inflate(verifyGarbageBuffer, 0, verifyGarbageBuffer.length);
 	}
 
-	private synchronized void ensureLoaded(final byte[] array) {
+	synchronized void ensureLoaded(final byte[] array) {
 		if (!loaded) {
 			try {
 				provider.fd.getChannel().read(ByteBuffer.wrap(array), start);
 			} catch (IOException e) {
 				throw new RuntimeException("Cannot fault in window", e);
+			} finally {
+				WindowCache.markLoaded(this);
 			}
 			loaded = true;
 		}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteBufferWindow.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteBufferWindow.java
index a6757e8..5beb79b 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteBufferWindow.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteBufferWindow.java
@@ -107,4 +107,8 @@ void inflateVerify(final ByteBuffer buffer, final int pos,
 		while (!inf.finished() && !inf.needsInput())
 			inf.inflate(verifyGarbageBuffer, 0, verifyGarbageBuffer.length);
 	}
+
+	void ensureLoaded(final ByteBuffer ref) {
+		// Do nothing.
+	}
 }
\ No newline at end of file
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteWindow.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteWindow.java
index 732664b..e957138 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteWindow.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteWindow.java
@@ -217,4 +217,6 @@ final void inflateVerify(T ref, long pos, Inflater inf)
 
 	abstract void inflateVerify(T ref, int pos, Inflater inf)
 			throws DataFormatException;
+
+	abstract void ensureLoaded(T ref);
 }
\ No newline at end of file
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java
index 0b9d20c..4b7e10d 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java
@@ -189,7 +189,13 @@ private static synchronized void reconfigureImpl(final int packedGitLimit,
 	 *             the window was not found in the cache and the given provider
 	 *             was unable to load the window on demand.
 	 */
-	public static synchronized final void get(final WindowCursor curs,
+	public static final void get(final WindowCursor curs,
+			final WindowedFile wp, final long position) throws IOException {
+		getImpl(curs, wp, position);
+		curs.window.ensureLoaded(curs.handle);
+	}
+
+	private static synchronized final void getImpl(final WindowCursor curs,
 			final WindowedFile wp, final long position) throws IOException {
 		final int id = (int) (position >> windowSizeShift);
 		final int idx = hash(wp, id);
@@ -254,6 +260,12 @@ public static synchronized final void get(final WindowCursor curs,
 		insertLRU(e);
 	}
 
+	static synchronized void markLoaded(final ByteWindow w) {
+		if (--w.provider.openCount == 0) {
+			w.provider.cacheClose();
+		}
+	}
+
 	private static void makeMostRecent(ByteWindow<?> e) {
 		if (lruHead != e) {
 			unlinkLRU(e);
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java
index db8ea88..938f44c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java
@@ -348,5 +348,6 @@ void allocWindow(final WindowCursor curs, final int windowId,
 		final byte[] b = new byte[size];
 		curs.window = new ByteArrayWindow(this, pos, windowId, b);
 		curs.handle = b;
+		openCount++; // Until the window loads, we must stay open.
 	}
 }
-- 
1.6.2.185.g8b635

^ permalink raw reply related

* [JGIT PATCH] Fix a race condition when loading non-mmapd byte windows
From: Shawn O. Pearce @ 2009-03-08  0:12 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git

Back in 439d441b0800 ("Change ByteArrayWindow to read content
outside of WindowCache's lock") I introduced a race condition
to WindowCache.

It is possible for a ByteArrayWindow to be allocated and put
into the cache, and then for the calling thread to get put
to sleep before it can actually load the window from disk.
By the time the thread wakes up again, the window may have
already been evicted from the cache, and its underlying file
was closed.  This results in stack traces such as:

java.lang.RuntimeException: Cannot fault in window
        at org.spearce.jgit.lib.ByteArrayWindow.ensureLoaded(ByteArrayWindow.java:111)
...
Caused by: java.nio.channels.ClosedChannelException
        at sun.nio.ch.FileChannelImpl.ensureOpen(FileChannelImpl.java:91)

Now when we allocate a ByteArrayWindow for a specific window reference
we increment the file reference count one additional time, forcing the
file to stay open even if the window was evicted from the cache.  The
reference count is decremented once the window loads successfully, and
this puts us back into the state of 1 reference count value for each
window still in the window cache.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---

 I've decided I hate WindowedFile, ByteWindow, WindowCache.

 The API and implementation is crap.  Its all my own fault.  This
 code is mine, but its impossible to work with, not as concurrent
 as I'd like the cache to be, and there's problems getting it to
 be thread-safe, as this patch just pointed out.  *sigh*

 I think we should apply this, but really, someone needs to come
 back here and overhaul how we do our most low level IO ops.
 Its a crime that this code is in the state it is in.

 Or maybe I'm just pissed off because I'm hunting down a stupid
 concurrency issue on a Saturday.
 
 .../src/org/spearce/jgit/lib/ByteArrayWindow.java  |    7 +++----
 .../src/org/spearce/jgit/lib/ByteBufferWindow.java |    4 ++++
 .../src/org/spearce/jgit/lib/ByteWindow.java       |    2 ++
 .../src/org/spearce/jgit/lib/WindowCache.java      |   12 +++++++++++-
 .../src/org/spearce/jgit/lib/WindowedFile.java     |    1 +
 5 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteArrayWindow.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteArrayWindow.java
index 7b7c7a4..055a329 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteArrayWindow.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteArrayWindow.java
@@ -68,7 +68,6 @@ ByteArrayWindow(final WindowedFile o, final long p, final int d,
 	}
 
 	int copy(final byte[] array, final int p, final byte[] b, final int o, int n) {
-		ensureLoaded(array);
 		n = Math.min(array.length - p, n);
 		System.arraycopy(array, p, b, o, n);
 		return n;
@@ -76,7 +75,6 @@ int copy(final byte[] array, final int p, final byte[] b, final int o, int n) {
 
 	int inflate(final byte[] array, final int pos, final byte[] b, int o,
 			final Inflater inf) throws DataFormatException {
-		ensureLoaded(array);
 		while (!inf.finished()) {
 			if (inf.needsInput()) {
 				inf.setInput(array, pos, array.length - pos);
@@ -91,7 +89,6 @@ int inflate(final byte[] array, final int pos, final byte[] b, int o,
 
 	void inflateVerify(final byte[] array, final int pos, final Inflater inf)
 			throws DataFormatException {
-		ensureLoaded(array);
 		while (!inf.finished()) {
 			if (inf.needsInput()) {
 				inf.setInput(array, pos, array.length - pos);
@@ -103,12 +100,14 @@ void inflateVerify(final byte[] array, final int pos, final Inflater inf)
 			inf.inflate(verifyGarbageBuffer, 0, verifyGarbageBuffer.length);
 	}
 
-	private synchronized void ensureLoaded(final byte[] array) {
+	synchronized void ensureLoaded(final byte[] array) {
 		if (!loaded) {
 			try {
 				provider.fd.getChannel().read(ByteBuffer.wrap(array), start);
 			} catch (IOException e) {
 				throw new RuntimeException("Cannot fault in window", e);
+			} finally {
+				WindowCache.markLoaded(this);
 			}
 			loaded = true;
 		}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteBufferWindow.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteBufferWindow.java
index a6757e8..5beb79b 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteBufferWindow.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteBufferWindow.java
@@ -107,4 +107,8 @@ void inflateVerify(final ByteBuffer buffer, final int pos,
 		while (!inf.finished() && !inf.needsInput())
 			inf.inflate(verifyGarbageBuffer, 0, verifyGarbageBuffer.length);
 	}
+
+	void ensureLoaded(final ByteBuffer ref) {
+		// Do nothing.
+	}
 }
\ No newline at end of file
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteWindow.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteWindow.java
index 732664b..e957138 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ByteWindow.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ByteWindow.java
@@ -217,4 +217,6 @@ final void inflateVerify(T ref, long pos, Inflater inf)
 
 	abstract void inflateVerify(T ref, int pos, Inflater inf)
 			throws DataFormatException;
+
+	abstract void ensureLoaded(T ref);
 }
\ No newline at end of file
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java
index 0b9d20c..fc4bab8 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowCache.java
@@ -189,7 +189,13 @@ private static synchronized void reconfigureImpl(final int packedGitLimit,
 	 *             the window was not found in the cache and the given provider
 	 *             was unable to load the window on demand.
 	 */
-	public static synchronized final void get(final WindowCursor curs,
+	public static final void get(final WindowCursor curs,
+			final WindowedFile wp, final long position) throws IOException {
+		getImpl(curs, wp, position);
+		curs.window.ensureLoaded(curs.handle);
+	}
+
+	private static synchronized final void getImpl(final WindowCursor curs,
 			final WindowedFile wp, final long position) throws IOException {
 		final int id = (int) (position >> windowSizeShift);
 		final int idx = hash(wp, id);
@@ -254,6 +260,10 @@ public static synchronized final void get(final WindowCursor curs,
 		insertLRU(e);
 	}
 
+	static synchronized void markLoaded(final ByteWindow w) {
+		w.provider.openCount--;
+	}
+
 	private static void makeMostRecent(ByteWindow<?> e) {
 		if (lruHead != e) {
 			unlinkLRU(e);
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java
index db8ea88..938f44c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/WindowedFile.java
@@ -348,5 +348,6 @@ void allocWindow(final WindowCursor curs, final int windowId,
 		final byte[] b = new byte[size];
 		curs.window = new ByteArrayWindow(this, pos, windowId, b);
 		curs.handle = b;
+		openCount++; // Until the window loads, we must stay open.
 	}
 }
-- 
1.6.2.185.g8b635

^ permalink raw reply related

* t9500-gitweb-standalone-no-errors.sh.prb 4 errors
From: Boyd Lynn Gerber @ 2009-03-08  0:11 UTC (permalink / raw)
  To: Git List

Test fails for SCO OpenServer 6.0 MP4 with 1.6.2

Below is the set -x output of the tests.

-- 
Boyd Gerber <gerberb@zenez.com> 801 849-0213
ZENEZ	1042 East Fort Union #135, Midvale Utah  84047


+ test_description='gitweb as standalone script (basic tests).

This test runs gitweb (git web interface) as CGI script from
commandline, and checks that it would not write any errors
or warnings to log.'
+ . ./test-lib.sh
++ ORIGINAL_TERM=xterm
++ LANG=C
++ LC_ALL=C
++ PAGER=cat
++ TZ=UTC
++ TERM=dumb
++ export LANG LC_ALL PAGER TERM TZ
++ EDITOR=:
++ VISUAL=:
++ unset GIT_EDITOR
++ unset AUTHOR_DATE
++ unset AUTHOR_EMAIL
++ unset AUTHOR_NAME
++ unset COMMIT_AUTHOR_EMAIL
++ unset COMMIT_AUTHOR_NAME
++ unset EMAIL
++ unset GIT_ALTERNATE_OBJECT_DIRECTORIES
++ unset GIT_AUTHOR_DATE
++ GIT_AUTHOR_EMAIL=author@example.com
++ GIT_AUTHOR_NAME='A U Thor'
++ unset GIT_COMMITTER_DATE
++ GIT_COMMITTER_EMAIL=committer@example.com
++ GIT_COMMITTER_NAME='C O Mitter'
++ unset GIT_DIFF_OPTS
++ unset GIT_DIR
++ unset GIT_WORK_TREE
++ unset GIT_EXTERNAL_DIFF
++ unset GIT_INDEX_FILE
++ unset GIT_OBJECT_DIRECTORY
++ unset GIT_CEILING_DIRECTORIES
++ unset SHA1_FILE_DIRECTORIES
++ unset SHA1_FILE_DIRECTORY
++ GIT_MERGE_VERBOSITY=5
++ export GIT_MERGE_VERBOSITY
++ export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
++ export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
++ export EDITOR VISUAL
++ GIT_TEST_CMP='diff -u'
++ unset CDPATH
++ case $(echo $GIT_TRACE |tr "[A-Z]" "[a-z]") in
+++ echo
+++ tr '[A-Z]' '[a-z]'
++ '[' xxterm '!=' xdumb ']'
++ TERM=xterm
++ export TERM
++ '[' -t 1 ']'
++ test 0 -ne 0
++ test -n ''
++ test 'gitweb as standalone script (basic tests).

This test runs gitweb (git web interface) as CGI script from
commandline, and checks that it would not write any errors
or warnings to log.' '!=' ''
++ test '' = t
++ exec
++ test '' = t
++ exec
++ test_failure=0
++ test_count=0
++ test_fixed=0
++ test_broken=0
++ test_success=0
++ trap die EXIT
+++ pwd
++ TEST_DIRECTORY=/tmp/git-1.6.2/t
++ 
PATH=/tmp/git-1.6.2/t/..:/usr/local/bin:/usr/gnu/bin:/home/gerberb/bin:/usr/bin:/bin:/sbin:/usr/sbin:/etc:/usr/X11R6/bin
+++ pwd
++ GIT_EXEC_PATH=/tmp/git-1.6.2/t/..
+++ pwd
++ GIT_TEMPLATE_DIR=/tmp/git-1.6.2/t/../templates/blt
++ unset GIT_CONFIG
++ GIT_CONFIG_NOSYSTEM=1
++ GIT_CONFIG_NOGLOBAL=1
++ export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG_NOSYSTEM 
GIT_CONFIG_NOGLOBAL
+++ pwd
+++ pwd
++ 
GITPERLLIB=/tmp/git-1.6.2/t/../perl/blib/lib:/tmp/git-1.6.2/t/../perl/blib/arch/auto/Git
++ export GITPERLLIB
++ test -d ../templates/blt
++ test -x ../test-chmtime
++ . ../GIT-BUILD-OPTIONS
+++ SHELL_PATH=/usr/bin/bash
+++ TAR=gtar
+++ basename ./t9500-gitweb-standalone-no-errors.sh .sh
++ test='trash directory.t9500-gitweb-standalone-no-errors'
++ test '!' -z ''
++ remove_trash='/tmp/git-1.6.2/t/trash 
directory.t9500-gitweb-standalone-no-errors'
++ rm -fr 'trash directory.t9500-gitweb-standalone-no-errors'
++ test_create_repo 'trash directory.t9500-gitweb-standalone-no-errors'
++ test 1 = 1
+++ pwd
++ owd=/tmp/git-1.6.2/t
++ repo='trash directory.t9500-gitweb-standalone-no-errors'
++ mkdir -p 'trash directory.t9500-gitweb-standalone-no-errors'
++ cd 'trash directory.t9500-gitweb-standalone-no-errors'
++ /tmp/git-1.6.2/t/../git init --template=/tmp/git-1.6.2/t/../templates/blt/
++ mv .git/hooks .git/hooks-disabled
++ cd /tmp/git-1.6.2/t
++ cd -P 'trash directory.t9500-gitweb-standalone-no-errors'
+++ expr ././t9500-gitweb-standalone-no-errors.sh : '.*/\(t[0-9]*\)-[^/]*$'
++ this_test=t9500
+ perl -MEncode -e 'decode_utf8("", Encode::FB_CROAK)'
+ gitweb_init
++ perl -MPOSIX=getcwd -e 'print quotemeta(getcwd)'
+ safe_pwd='\/tmp\/git\-1\.6\.2\/t\/trash\ 
directory\.t9500\-gitweb\-standalone\-no\-errors'
+ cat
+ cat
+ test_expect_success 'no commits: projects_list (implicit)' gitweb_run
+ test 2 = 2
+ test_skip 'no commits: projects_list (implicit)' gitweb_run
++ expr ././t9500-gitweb-standalone-no-errors.sh : '.*/\(t[0-9]*\)-[^/]*$'
+ this_test=t9500
...
+ echo ''
+ test_debug 'cat gitweb.log'
+ test '' = ''
+ test_expect_success '.git blob (file)' 'gitweb_run "p=.git;a=blob;f=file"'
+ test 2 = 2
+ test_skip '.git blob (file)' 'gitweb_run "p=.git;a=blob;f=file"'
++ expr ././t9500-gitweb-standalone-no-errors.sh : '.*/\(t[0-9]*\)-[^/]*$'
+ this_test=t9500
++ expr 17 + 1
+ this_test=t9500.18
+ to_skip=
+ case "$to_skip" in
+ false
+ say 'expecting success: gitweb_run "p=.git;a=blob;f=file"'
+ say_color info 'expecting success: gitweb_run "p=.git;a=blob;f=file"'
+ test -z info
+ shift
+ echo '* expecting success: gitweb_run "p=.git;a=blob;f=file"'
+ test_run_ 'gitweb_run "p=.git;a=blob;f=file"'
+ eval 'gitweb_run "p=.git;a=blob;f=file"'
+ eval_ret=255
+ return 0
+ '[' 0 = 0 -a 255 = 0 ']'
+ test_failure_ '.git blob (file)' 'gitweb_run "p=.git;a=blob;f=file"'
++ expr 17 + 1
+ test_count=18
++ expr 0 + 1
+ test_failure=1
+ say_color error 'FAIL 18: .git blob (file)'
+ test -z error
+ shift
+ echo '* FAIL 18: .git blob (file)'
* FAIL 18: .git blob (file)
+ shift
+ echo 'gitweb_run "p=.git;a=blob;f=file"'
+ sed -e 's/^/	/'
      	gitweb_run "p=.git;a=blob;f=file"
+ test '' = ''
+ echo ''
+ test_debug 'cat gitweb.log'
+ test '' = ''
+ test_expect_success '.git blob_plain (file)' 'gitweb_run 
"p=.git;a=blob_plain;f=file"'
+ test 2 = 2
+ test_skip '.git blob_plain (file)' 'gitweb_run "p=.git;a=blob_plain;f=file"'
++ expr ././t9500-gitweb-standalone-no-errors.sh : '.*/\(t[0-9]*\)-[^/]*$'
+ this_test=t9500
++ expr 18 + 1
+ this_test=t9500.19
+ to_skip=
+ case "$to_skip" in
+ false
+ say 'expecting success: gitweb_run "p=.git;a=blob_plain;f=file"'
+ say_color info 'expecting success: gitweb_run "p=.git;a=blob_plain;f=file"'
+ test -z info
+ shift
+ echo '* expecting success: gitweb_run "p=.git;a=blob_plain;f=file"'
+ test_run_ 'gitweb_run "p=.git;a=blob_plain;f=file"'
+ eval 'gitweb_run "p=.git;a=blob_plain;f=file"'
+ eval_ret=255
+ return 0
+ '[' 0 = 0 -a 255 = 0 ']'
+ test_failure_ '.git blob_plain (file)' 'gitweb_run 
"p=.git;a=blob_plain;f=file"'
++ expr 18 + 1
+ test_count=19
++ expr 1 + 1
+ test_failure=2
+ say_color error 'FAIL 19: .git blob_plain (file)'
+ test -z error
+ shift
+ echo '* FAIL 19: .git blob_plain (file)'
* FAIL 19: .git blob_plain (file)
+ shift
+ echo 'gitweb_run "p=.git;a=blob_plain;f=file"'
+ sed -e 's/^/	/'
      	gitweb_run "p=.git;a=blob_plain;f=file"
...
+ echo ''
+ test_debug 'cat gitweb.log'
+ test '' = ''
+ test_expect_success 'path_info: project/branch:file' 'gitweb_run "" 
"/.git/master:file"'
+ test 2 = 2
+ test_skip 'path_info: project/branch:file' 'gitweb_run "" 
"/.git/master:file"'
++ expr ././t9500-gitweb-standalone-no-errors.sh : '.*/\(t[0-9]*\)-[^/]*$'
+ this_test=t9500
++ expr 61 + 1
+ this_test=t9500.62
+ to_skip=
+ case "$to_skip" in
+ false
+ say 'expecting success: gitweb_run "" "/.git/master:file"'
+ say_color info 'expecting success: gitweb_run "" "/.git/master:file"'
+ test -z info
+ shift
+ echo '* expecting success: gitweb_run "" "/.git/master:file"'
+ test_run_ 'gitweb_run "" "/.git/master:file"'
+ eval 'gitweb_run "" "/.git/master:file"'
+ eval_ret=2
+ return 0
+ '[' 0 = 0 -a 2 = 0 ']'
+ test_failure_ 'path_info: project/branch:file' 'gitweb_run "" 
"/.git/master:file"'
++ expr 61 + 1
+ test_count=62
++ expr 2 + 1
+ test_failure=3
+ say_color error 'FAIL 62: path_info: project/branch:file'
+ test -z error
+ shift
+ echo '* FAIL 62: path_info: project/branch:file'
* FAIL 62: path_info: project/branch:file
+ shift
+ echo 'gitweb_run "" "/.git/master:file"'
+ sed -e 's/^/	/'
      	gitweb_run "" "/.git/master:file"
...
+ test -z ''
+ test -n ''
+ shift
+ echo '*   ok 65: path_info: project/branch:dir/ (non-existent)'
*   ok 65: path_info: project/branch:dir/ (non-existent)
+ echo ''
+ test_debug 'cat gitweb.log'
+ test '' = ''
+ test_expect_success 'path_info: project/branch:/file' 'gitweb_run "" 
"/.git/master:/file"'
+ test 2 = 2
+ test_skip 'path_info: project/branch:/file' 'gitweb_run "" 
"/.git/master:/file"'
++ expr ././t9500-gitweb-standalone-no-errors.sh : '.*/\(t[0-9]*\)-[^/]*$'
+ this_test=t9500
++ expr 65 + 1
+ this_test=t9500.66
+ to_skip=
+ case "$to_skip" in
+ false
+ say 'expecting success: gitweb_run "" "/.git/master:/file"'
+ say_color info 'expecting success: gitweb_run "" "/.git/master:/file"'
+ test -z info
+ shift
+ echo '* expecting success: gitweb_run "" "/.git/master:/file"'
+ test_run_ 'gitweb_run "" "/.git/master:/file"'
+ eval 'gitweb_run "" "/.git/master:/file"'
+ eval_ret=2
+ return 0
+ '[' 0 = 0 -a 2 = 0 ']'
+ test_failure_ 'path_info: project/branch:/file' 'gitweb_run "" 
"/.git/master:/file"'
++ expr 65 + 1
+ test_count=66
++ expr 3 + 1
+ test_failure=4
+ say_color error 'FAIL 66: path_info: project/branch:/file'
+ test -z error
+ shift
+ echo '* FAIL 66: path_info: project/branch:/file'
* FAIL 66: path_info: project/branch:/file
+ shift
+ echo 'gitweb_run "" "/.git/master:/file"'
+ sed -e 's/^/	/'
      	gitweb_run "" "/.git/master:/file"
...
+ echo ''
+ test_debug 'cat gitweb.log'
+ test '' = ''
+ test_done
+ trap - EXIT
+ test_results_dir=/tmp/git-1.6.2/t/test-results
+ mkdir -p /tmp/git-1.6.2/t/test-results
+ 
test_results_path=/tmp/git-1.6.2/t/test-results/./t9500-gitweb-standalone-no-1287
+ echo 'total 87'
+ echo 'success 83'
+ echo 'fixed 0'
+ echo 'broken 0'
+ echo 'failed 4'
+ echo ''
+ test 0 '!=' 0
+ test 0 '!=' 0
+ msg='87 test(s)'
+ case "$test_failure" in
+ say_color error 'failed 4 among 87 test(s)'
+ test -z error
+ shift
+ echo '* failed 4 among 87 test(s)'
* failed 4 among 87 test(s)
+ exit 1

^ permalink raw reply

* Re: import files w/ history
From: Jeff King @ 2009-03-08  0:10 UTC (permalink / raw)
  To: Csaba Henk; +Cc: git
In-Reply-To: <slrngr299k.1t4t.csaba-ml@beastie.creo.hu>

On Fri, Mar 06, 2009 at 01:29:38PM +0000, Csaba Henk wrote:

> $ git filter-branch --commit-filter '
>    if [ $# -lt 3 ] || git diff --stat $3 $1 | grep -q 'sys/dev/disk/vn/vn\.c'
>    then
>      git commit-tree "$@"
>    else
>      skip_commit "$@"
>    fi' HEAD

Wow, I'll bet that was slow to run. And it's not really what you want.

You are picking commits that changed a particular file, and then
including the _whole_ tree. Remember that commits really record a tree
state; we only think of them as "changes" because they point to a prior
commit with its own tree state. So you are just selecting some subset of
the states, but not cutting down the tree in each state.

What you really want to do is say:

  - for every commit, narrow the tree to _just_ the one file

  - if there were no changes in the narrowed tree, just throw out the
    commit

You can use an --index-filter to do the former, and a --commit-filter to
do the latter (or just use --prune-empty, which is a shorthand).

Another poster had a similar problem, and you can see the right
filter-branch recipe there:

  http://article.gmane.org/gmane.comp.version-control.git/111991

>   *  *  *
> 
> OK, I then tried to do more RTFM and be more clever and efficient, and
> find a way to specify directly those commits which affect vn.c. As "git
> rev-list" can be invoked like "git rev-list <commit> <path>", and the
> synopsis of "git filter-branch" is like
> 
>  git filter-branch [options] [--] [<rev-list options>...]
> 
> I then gave a try to:
> 
> $ git filter-branch --  master sys/dev/disk/vn/vn.c
> 
> but no dice -- I got:
> 
>   fatal: ambiguous argument 'sys/dev/disk/vn/vn.c': unknown revision or
>   path not in the working tree.
>   Use '--' to separate paths from revisions
>   Could not get the commits
> 
> Any idea?

I think you need an extra '--' to separate the paths from the revisions
in the rev-list arguments:

  git filter-branch -- master -- sys/dev/disk/vn/vn.c

but even that doesn't quite do what you want. It limits the commits that
are shown, similar to your first attempt above, but it doesn't cut down
the tree itself (OTOH, limiting by path rather than using --prune-empty
is likely to run faster, since you won't even look at commits that are
uninteresting. However, it may change the shape of your history with
respect to branching and merging).

-Peff

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox