Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Documented --no-checkout option in git-svn
From: Sam Vilain @ 2008-10-30 18:20 UTC (permalink / raw)
  To: git, Deskin Miller
  Cc: Sam Vilain,
	_vi@list.ru, git@vger.kernel.org, gitster@pobox.com, Vitaly _Vi Shukela,
	Vitaly "_Vi" Shukela
In-Reply-To: <20081030180736.GA20322@euler>

On Thu, 2008-10-30 at 14:07 -0400, Deskin Miller wrote:
> > +--no-checkout
> > +	Do not checkout latest revision after fetching.
> 
> This isn't quite how the other options are listed in the source; for one, this
> ends up formatted in the final manpage like
> 
> --no-checkout Do not checkout latest revision after fetching.
> 
> Instead of
> 
> --no-checkout
> 	Do not checkout latest revision after fetching.
> 
> Also, the wording seems slightly imprecise; in fact, if the repository already
> has a checkout, git svn fetch would not attempt to check anything out in its
> place, nor will it check anything out if there is a local master branch
> already.  With clone this is not typically a problem, but in fact it is
> possible to clone into a preexisting git repository, so the same concerns
> exist.

I think the wording is close enough; here's a version which looks good
to me and fixes the asciidoc differences.

Subject: git-svn: document --no-checkout option

From: Vitaly "_Vi" Shukela <public_vi@tut.by>

Signed-off-by: Vitaly "_Vi" Shukela <public_vi@tut.by>
Signed-off-by: Sam Vilain <sam@vilain.net>
---
 Documentation/git-svn.txt |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 84c8f3c..2298512 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -91,6 +91,9 @@ COMMANDS
 	tracking.  The name of the [svn-remote "..."] section in the
 	.git/config file may be specified as an optional command-line
 	argument.
+	
+--no-checkout;;
+	Do not checkout the latest revision after fetching.
 
 'clone'::
 	Runs 'init' and 'fetch'.  It will automatically create a
@@ -103,6 +106,9 @@ COMMANDS
 	the working tree; and the 'rebase' command will be able
 	to update the working tree with the latest changes.
 
+--no-checkout;;
+	Do not checkout the latest revision after cloning.
+
 'rebase'::
 	This fetches revisions from the SVN parent of the current HEAD
 	and rebases the current (uncommitted to SVN) work against it.
-- 
debian.1.5.6.1


^ permalink raw reply related

* [PATCH] Documentation: clarify information about 'ident' attribute
From: Jan Krüger @ 2008-10-30 18:14 UTC (permalink / raw)
  To: git; +Cc: gitster

The documentation spoke of the attribute being set "to" a path; this can
mistakenly be interpreted as "the attribute needs to have its value set to
some kind of path". This clarifies things (and also calls the object ID a
hash rather than a name because that might be confusing too).

Signed-off-by: Jan Krüger <jk@jk.gs>
---
Confused me and a few other people on IRC the other day.

 Documentation/gitattributes.txt |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 2694559..b39db6b 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -163,9 +163,9 @@ few exceptions.  Even though...
 `ident`
 ^^^^^^^
 
-When the attribute `ident` is set to a path, git replaces
-`$Id$` in the blob object with `$Id:`, followed by
-40-character hexadecimal blob object name, followed by a dollar
+When the attribute `ident` is set for a path, git replaces
+`$Id$` in the blob object with `$Id:`, followed by the
+40-character hexadecimal blob object hash, followed by a dollar
 sign `$` upon checkout.  Any byte sequence that begins with
 `$Id:` and ends with `$` in the worktree file is replaced
 with `$Id$` upon check-in.
-- 
1.6.0.3.523.g304d0.dirty

^ permalink raw reply related

* Re: [PATCH] Documentation: add a planning document for the next CLI revamp
From: Nicolas Pitre @ 2008-10-30 18:28 UTC (permalink / raw)
  To: Sam Vilain; +Cc: Pierre Habouzit, Mike Hommey, Shawn O. Pearce, git
In-Reply-To: <1225387882.19891.9.camel@maia.lan>

On Thu, 30 Oct 2008, Sam Vilain wrote:

> On Thu, 2008-10-30 at 12:53 -0400, Nicolas Pitre wrote:
> > > Seconded.
> > > 
> > > Having git-checkout $foo being a shorthand for git checkout -b $foo
> > > origin/$foo when origin/$foo exists and $foo doesn't is definitely handy.
> > 
> > No.  This is only the first step towards insanity.
> > 
> > In many cases origin/$foo == origin/master so this can't work in that 
> > case which is, after all, the common case.
> 
> I don't understand that argument at all, can you explain further?

By default, git creates a branch called "master.  Hence, by default, if 
you clone that repository, this branch will be called origin/master.  So 
by default $foo is already ambiguous.

> >   Therefore I think this is 
> > wrong to add magic operations which are not useful for the common case 
> > and actively _hide_ how git actually works.  Not only will you have to 
> > explain how git works anyway for that common origin/master case, but 
> > you'll also have to explain why sometimes the magic works and sometimes 
> > not.  Please keep such convenience shortcuts for your own scripts and/or 
> > aliases.
> 
> It's not about magic, it's about sensible defaults.  Currently this use
> case is an error, and the resultant command is very long to type, and
> involves typing the branch name twice.  I end up writing things like:
> 
>   git checkout -b {,origin/}wr34251-do-something
> 
> For the user who doesn't know to use the ksh-style {} blocks this is
> voodoo.  The longer form is cumbersome.

This is no excuse for promoting semantics only useful in such special 
cases.

> For the case where the thing you type is a resolvable reference, it
> would just check it out, as now.

As long as it checks it out with a detached head if it is a remote 
branch then I have no issue.


Nicolas

^ permalink raw reply

* Re: [PATCH] Introduce receive.denyDeletes
From: Shawn O. Pearce @ 2008-10-30 18:32 UTC (permalink / raw)
  To: Jan Krrrger; +Cc: git, gitster
In-Reply-To: <20081030191134.62455c24@perceptron>

Jan Krrrger <jk@jk.gs> wrote:
> Occasionally, it may be useful to prevent branches from getting deleted from
> a centralized repository, particularly when no administrative access to the
> server is available to undo it via reflog. It also makes
> receive.denyNonFastForwards more useful if it is used for access control, since
> it prevents force-updating refs by deleting and re-creating a ref.
...
>  test_expect_success \
> +	'pushing a delete should be denied with denyDeletes' '
> +	cd victim &&
> +	git config receive.denyDeletes true &&
> +	git branch extra master &&
> +	cd .. &&
> +	test -f victim/.git/refs/heads/extra &&
> +	git send-pack ./victim/.git/ :extra master && return 1

Hmm, that should be:

  test_must_fail git send-pack ./victim/.git/ :extra master

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] Introduce receive.denyDeletes
From: Jan Krüger @ 2008-10-30 18:45 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, gitster
In-Reply-To: <20081030183210.GO14786@spearce.org>

On Thu, 30 Oct 2008 11:32:10 -0700, "Shawn O. Pearce"
<spearce@spearce.org> wrote:
> >  test_expect_success \
> > [snip]
> > +	git send-pack ./victim/.git/ :extra master && return 1
> 
> Hmm, that should be:
> 
>   test_must_fail git send-pack ./victim/.git/ :extra master

Can I then delete the branch afterwards without lots of juggling (in
case the test fails due to a random other reason that the branch
accidentally getting deleted by receive-pack)? I'd expect I'd have to
save the exit code to a temporary variable and that's just as ugly.

Disclaimer: I sort of just hacked the test case together by stringing
together pieces of existing test code, so I don't presume full
understanding of the test framework.

By the way, I love what your mail client did to my name.

-Jan

^ permalink raw reply

* Re: Using the --track option when creating a branch
From: Marc Branchaud @ 2008-10-30 19:13 UTC (permalink / raw)
  To: Bill Lear; +Cc: Andreas Ericsson, Samuel Tardieu, git
In-Reply-To: <18697.54743.601331.133842@lisa.zopyra.com>

Bill Lear wrote:
> 
> the reason being that every manual our users read says "use git push",
> use "git pull", the examples being written for 'master' branch usage,
> and people just assume that 'git push'/'git pull' are smart enough to
> know which branch you are on and do the same logical thing as a bare
> 'git push'/'git pull' does when on master.

I agree that this is a 'gotcha' for git-push.  I'm a new git user, and 
I've been experimenting with git and reading the documentation for the 
last few weeks.  But I would not have known about this behavior if it 
weren't for this thread.

Yes, push's man page is clear about what happens if you push with no 
refspec, and the fetch & pull man pages both have an obscure note to 
"never do your own development on branches that appear
on the right hand side of a <refspec> colon on 'Pull:' lines".  But 
still the behavior is not what I expected.  Before I read this thread, I 
missed the implications of what those parts of the man pages were saying.

One could call this a failure of the documentation (man pages and 
beyond).  Personally, though, I tend to expect minimal commands to do 
minimal things.  So a plain "git push" would do the minimum amount of 
pushing, and if I want it to do more I'd add extra parameters to the 
command.

The current behavior seems fairly harmless if you always follow the 
pattern of creating topic branches for all your work.  But git (rightly) 
doesn't enforce that pattern, and so I think push shouldn't default to 
doing something potentially harmful just because you forgot to create a 
topic branch one day.  (Or maybe you decided to be clever and give one 
of your local branches the same name as a remote's branch...)

		Marc

^ permalink raw reply

* Re: Pull request for sub-tree merge into /contrib/gitstats
From: Junio C Hamano @ 2008-10-30 19:24 UTC (permalink / raw)
  To: sverre; +Cc: Git Mailinglist
In-Reply-To: <bd6139dc0810291606o2efe4254me378335b76861340@mail.gmail.com>

I have a mixed feeling about this.  From a longer-term perspective, do you
really want this to be a part of git.git repository?

I do not mind having notes to endorse and advocate "stats" as one of the
"Third party packages that may make your git life more pleasuable", just
like tig, stgit, guilt and topgit, but I cannot convince myself that
merging it as a subtree is the right thing to do at this point.

The "stats" tool, at least at the conceptual level, shares one important
property with tools like gitk and gitweb: it could be useful to people
whose sources are not in git repositories but in say Hg or Bzr, with some
effort.  The code may need refactoring to make it easier to plug in
different backends and writing actual backends (aka "porting"), but that
is something you can expect people with different backends to help you
with.

However, it would be awkward for the contrib/ area in git.git to carry a
lot of code that are only needed to produce stat data from non-git
repositories, once such a porting effort begins.

It's perfectly fine if you are not interested in any of the other
backends, and tell the people that they are welcome to fork it never to
merge back.  But if this were my brainchild, I'd imagine I'd be wishing to
be able to buy back the improvements to the "core stats" parts that are
done by people with other backends.  I would imagine binding the current
code as part of git.git would make such improvements harder to manage,
both for you (who wants to buy back the changes made by others on
different backends) and for others on different backends (who want to
merge the changes made by you to their forks).

Perhaps pointing at your tree as a submodule would be the right thing to
do; then git.git proper will be just one of the users of "stats" tool.

How about making that as a mid-to-longer term goal?  When we eject git-gui
and gitk from git.git and make them a submodule (wasn't that supposed to
happen in 1.8 or 2.0 timeframe?), we may also add "stats" as a submodule?

^ permalink raw reply

* Re: [PATCH] Documentation: clarify information about 'ident' attribute
From: Junio C Hamano @ 2008-10-30 19:30 UTC (permalink / raw)
  To: Jan Krüger; +Cc: git, gitster
In-Reply-To: <20081030191433.710aff11@perceptron>

"Jan Krüger" <jk@jk.gs> writes:

> The documentation spoke of the attribute being set "to" a path; this can
> mistakenly be interpreted as "the attribute needs to have its value set to
> some kind of path". This clarifies things (and also calls the object ID a
> hash rather than a name because that might be confusing too).

I'd agree with the first change, but not with the second.  "object name"
has been the official name of these hexadecimal thingy for a long time
(see the glossary).

^ permalink raw reply

* Re: [PATCH] git show <tree>: show mode and hash, and handle -r
From: Junio C Hamano @ 2008-10-30 21:15 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, schacon
In-Reply-To: <alpine.DEB.1.00.0810290207330.22125@pacific.mpi-cbg.de.mpi-cbg.de>

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

> Now, git show <tree> shows some more information, and with the -r option,
> it recurses into subdirectories.
>
> Requested by Scott Chacon.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>
> 	The only quirk is the command line parsing for -r: we cannot use 
> 	DIFF_OPT_TST(&rev.diffopt, RECURSIVE), because that is switched 
> 	on not only by cmd_log_init(), but implicitly by 
> 	diff_setup_done(), because FORMAT_PATCH is selected by git-show.

That's a rather large quirk with an ugly workaround if you ask me.

I also notice that there is:

    int cmd_log_reflog(int argc, const char **argv, const char *prefix)
    {
            struct rev_info rev;
            ...
            /*
             * This means that we override whatever commit format the user gave
             * on the cmd line.  Sad, but cmd_log_init() currently doesn't
             * allow us to set a different default.
             */

I wonder if it would help breaking down cmd_log_init() a bit like this.

 builtin-log.c |   27 +++++++++++++++++++++------
 1 files changed, 21 insertions(+), 6 deletions(-)

diff --git c/builtin-log.c w/builtin-log.c
index 2efe593..0fcc28a 100644
--- c/builtin-log.c
+++ w/builtin-log.c
@@ -50,18 +50,23 @@ static int add_ref_decoration(const char *refname, const unsigned char *sha1, in
 	return 0;
 }
 
-static void cmd_log_init(int argc, const char **argv, const char *prefix,
-		      struct rev_info *rev)
+static void cmd_log_init_0(int argc, const char **argv, const char *prefix,
+			   struct rev_info *rev,
+			   int default_abbrev,
+			   int default_commit_format,
+			   int default_verbose_header,
+			   int default_recursive)
 {
 	int i;
 	int decorate = 0;
 
-	rev->abbrev = DEFAULT_ABBREV;
-	rev->commit_format = CMIT_FMT_DEFAULT;
+	rev->abbrev = default_abbrev;
+	rev->commit_format = default_commit_format;
 	if (fmt_pretty)
 		get_commit_format(fmt_pretty, rev);
-	rev->verbose_header = 1;
-	DIFF_OPT_SET(&rev->diffopt, RECURSIVE);
+	rev->verbose_header = default_verbose_header;
+	if (default_recursive)
+		DIFF_OPT_SET(&rev->diffopt, RECURSIVE);
 	rev->show_root_diff = default_show_root;
 	rev->subject_prefix = fmt_patch_subject_prefix;
 
@@ -88,6 +93,16 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 	}
 }
 
+static void cmd_log_init(int argc, const char **argv, const char *prefix,
+			 struct rev_info *rev)
+{
+	cmd_log_init_0(argc, argv, prefix, rev,
+		       DEFAULT_ABBREV,
+		       CMIT_FMT_DEFAULT,
+		       1,
+		       1);
+}
+
 /*
  * This gives a rough estimate for how many commits we
  * will print out in the list.

^ permalink raw reply related

* RE: [RFC] gitweb: add 'historyfollow' view that follows renames
From: Moore, Robert @ 2008-10-30 22:00 UTC (permalink / raw)
  To: Blucher, Guy, Lin, Ming M, jnareb@gmail.com; +Cc: git@vger.kernel.org
In-Reply-To: <054F21930D24A0428E5B4588462C7AED0149B4B8@ednex512.dsto.defence.gov.au>

Thanks. This sounds like what we need.

We will take a look at it.
Bob


>-----Original Message-----
>From: Blucher, Guy [mailto:Guy.Blucher@dsto.defence.gov.au]
>Sent: Sunday, October 26, 2008 8:18 PM
>To: Lin, Ming M; jnareb@gmail.com; Moore, Robert
>Cc: git@vger.kernel.org
>Subject: [RFC] gitweb: add 'historyfollow' view that follows renames
>
>
>Hi Folks,
>
>>>
>>> What should we add to automatically get all file history?
>
>> While the 'commitdiff' view would, in default gitweb configuration,
>> contain information about file renames, currently 'history' view does
>> not support '--follow' option to git-log.  It wouldn't be too hard to
>> add it, but it just wasn't done (well, add to this the fact that
>> --follow works only for simple cases).
>
>We also ran up against this issue because renaming of files in our
>project is a useful bit of information while browsing file history.
>
>I hacked gitweb to add a 'historyfollow' viewing option in addition to
>the 'history' option.  Yes, '--follow' is expensive so I didn't just
>make it the default 'history' behaviour.
>
>The only problem with doing it was that parse_commits in gitweb.perl
>uses git rev-list which doesn't support the '--follow' option like
>git-log does. A bit of hacking to use 'git log --pretty=raw -z' to make
>the output look close to that from rev-list means only minor
>shoe-horning is required (perhaps it would be better to make rev-list
>understand --follow though?).
>
>I wasn't originally prepared to publish the work here because I don't
>really think it's the best solution. But considering it's come up... I
>include a patch inline against gitweb.perl from v1.6.0.3 that implements
>a 'historyfollow' view.
>
>Feel free to do with it what you will. It would need some substantial
>tidying up by someone who knows much more about perl than me :)
>
>We use it such that anywhere a 'history' link is provided a
>'historyfollow' link is also provided next to it - This patch doesn't
>implement that bit though.
>
>Hope it helps.
>
>Cheers,
>Guy.
>
>---
>
>--- a/gitweb/gitweb.perl
>+++ b/gitweb/gitweb.perl
>@@ -478,6 +478,7 @@ my %actions = (
>        "forks" => \&git_forks,
>        "heads" => \&git_heads,
>        "history" => \&git_history,
>+       "historyfollow" => \&git_history_follow,
>        "log" => \&git_log,
>        "rss" => \&git_rss,
>        "atom" => \&git_atom,
>@@ -2311,25 +2312,39 @@ sub parse_commit {
> }
>
> sub parse_commits {
>-       my ($commit_id, $maxcount, $skip, $filename, @args) = @_;
>+       my ($commit_id, $maxcount, $skip, $mode, $filename, @args) = @_;
>        my @cos;
>
>        $maxcount ||= 1;
>        $skip ||= 0;
>
>        local $/ = "\0";
>+        # The '--max-count' argument is not available when doing a
>+        # '--follow' to 'git log'
>+        my $count_arg = ("--max-count=" . $maxcount) ;
>+        if (defined $mode && $mode eq "--follow") {
>+            $count_arg = "--follow" ;
>+        }
>
>-       open my $fd, "-|", git_cmd(), "rev-list",
>-               "--header",
>+
>+       open my $fd, "-|", git_cmd(), "log",
>+               "-z",
>+               "--pretty=raw",
>                @args,
>-               ("--max-count=" . $maxcount),
>+                ($count_arg ? ($count_arg ) : ()),
>                ("--skip=" . $skip),
>                @extra_options,
>                $commit_id,
>                "--",
>                ($filename ? ($filename) : ())
>-               or die_error(500, "Open git-rev-list failed");
>+               or die_error(500, "Open git-log failed");
>        while (my $line = <$fd>) {
>+               # Need to put a delimiter on the end of output
>+                # 'git-log -z' doesn't put one before EOF like rev-list
>does
>+                $line = ($line . '\0');
>+                # Need to strip the word commit from the start so it
>+                # looks like rev-list output
>+                $line =~ s/^commit // ;
>                my %co = parse_commit_text($line);
>                push @cos, \%co;
>        }
>@@ -5363,6 +5378,13 @@ sub git_commitdiff_plain {
> }
>
> sub git_history {
>+        my $mode = shift || '';
>+        my $history_call = "history";
>+
>+       if ($mode eq "--follow") {
>+           $history_call .= "historyfollow" ;
>+       }
>+
>        if (!defined $hash_base) {
>                $hash_base = git_get_head_hash($project);
>        }
>@@ -5377,7 +5399,7 @@ sub git_history {
>        my $limit = sprintf("--max-count=%i", (100 * ($page+1)));
>
>        my @commitlist = parse_commits($hash_base, 101, (100 * $page),
>-                                      $file_name, "--full-history")
>+                                      $mode, $file_name,
>"--full-history")
>            or die_error(404, "No such file or directory on given
>branch");
>
>        if (!defined $hash && defined $file_name) {
>@@ -5398,7 +5420,7 @@ sub git_history {
>        my $paging_nav = '';
>        if ($page > 0) {
>                $paging_nav .=
>-                       $cgi->a({-href => href(action=>"history",
>hash=>$hash, hash_base=>$hash_base,
>+                       $cgi->a({-href => href(action=>"$history_call",
>hash=>$hash, hash_base=>$hash_base,
>                                               file_name=>$file_name)},
>                                "first");
>                $paging_nav .= " &sdot; " .
>@@ -5429,6 +5451,11 @@ sub git_history {
>        git_footer_html();
> }
>
>+sub git_history_follow {
>+       git_history('--follow');
>+}
>+
>+
> sub git_search {
>        gitweb_check_feature('search') or die_error(403, "Search is
>disabled");
>        if (!defined $searchtext) {
>@@ -5469,7 +5496,7 @@ sub git_search {
>                        $greptype = "--committer=";
>                }
>                $greptype .= $searchtext;
>-               my @commitlist = parse_commits($hash, 101, (100 *
>$page), undef,
>+               my @commitlist = parse_commits($hash, 101, (100 *
>$page), undef, undef,
>                                               $greptype,
>'--regexp-ignore-case',
>                                               $search_use_regexp ?
>'--extended-regexp' : '--fixed-strings');
>
>@@ -5737,7 +5764,7 @@ sub git_feed {
>
>        # log/feed of current (HEAD) branch, log of given branch,
>history of file/directory
>        my $head = $hash || 'HEAD';
>-       my @commitlist = parse_commits($head, 150, 0, $file_name);
>+       my @commitlist = parse_commits($head, 150, 0, undef,
>$file_name);
>
>        my %latest_commit;
>        my %latest_date;
>---
>
>Guy.
>____________________________________________________
>Guy Blucher
>Defence Science and Technology Organisation
>AUSTRALIA
>
>IMPORTANT : This email remains the property of the Australian Defence
>Organisation and is subject to the jurisdiction of section 70 of the
>Crimes Act 1914.  If you have received this email in error, you are
>requested to contact the sender and delete the email.

^ permalink raw reply

* [PATCH] t4030: Don't use echo -n
From: Brian Gernhardt @ 2008-10-30 22:12 UTC (permalink / raw)
  To: Git List; +Cc: Shawn O Pearce

Signed-off-by: Brian Gernhardt <benji@silverinsanity.com>
---
 t/t4030-diff-textconv.sh |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh
index 3945731..7ec244f 100755
--- a/t/t4030-diff-textconv.sh
+++ b/t/t4030-diff-textconv.sh
@@ -96,16 +96,15 @@ cat >expect.typechange <<'EOF'
 -1
 diff --git a/file b/file
 new file mode 120000
-index ad8b3d2..67be421
+index ad8b3d2..8e4020b
 --- /dev/null
 +++ b/file
 @@ -0,0 +1 @@
 +frotz
-\ No newline at end of file
 EOF
 # make a symlink the hard way that works on symlink-challenged file systems
 test_expect_success 'textconv does not act on symlinks' '
-	echo -n frotz > file &&
+	echo frotz > file &&
 	git add file &&
 	git ls-files -s | sed -e s/100644/120000/ |
 		git update-index --index-info &&
-- 
1.6.0.3.523.g304d0

^ permalink raw reply related

* [RFC PATCH v2 0/2] Detection of directory renames
From: Yann Dirson @ 2008-10-30 22:16 UTC (permalink / raw)
  To: git

This is an update of my previous patch.  It brings:
* a couple of fixes to annoying segfaults
* a testsuite showing where we're standing.
* getting rid of POSIX dirname
* improvement of debug messages

The code is still full of FIXME's, not ready for inclusin into any
branch whatsoever, and I'd be happy to hear about it if you can make
it break in a way or another - at least in a way not yet shown by the
testsuite :)

-- 
Yann Dirson    <ydirson@altern.org> |
Debian-related: <dirson@debian.org> |   Support Debian GNU/Linux:
                                    |  Freedom, Power, Stability, Gratis
     http://ydirson.free.fr/        | Check <http://www.debian.org/>

^ permalink raw reply

* [PATCH 1/2] Introduce rename factorization in diffcore.
From: Yann Dirson @ 2008-10-30 22:16 UTC (permalink / raw)
  To: git
In-Reply-To: <20081030220532.3325.54457.stgit@gandelf.nowhere.earth>

Rename factorization tries to group together files moving from and to
identical directories - the most common case being directory renames.
This feature is activated by the new --factorize-renames diffcore
flag.

This is only the first step, adding the basic functionnality and
adding support to raw diff output (and it breaks unified-diff output
which does not know how to handle that stuff yet).

Even the output format may not be kept as is.  For now both the result
of "mv a b" and "mv a/* b/" are displayed as "Rnnn a/ b/", which is
probably not what we want.  "Rnnn a/* b/" could be a good choice for
the latter if we want them to be distinguished, and even if we want
them to look the same.

Other future developements to be made on top of this include:
* extension of unified-diff format to express this
* application of such new diffs
* teach git-svn (and others ?) to make use of that flag
* merge correctly in case of addition into a moved dir
* detect "directory splits" so merge can flag a conflict on file adds
* use inexact dir renames to bump score of below-threshold renames
  from/to same locations
* add yours here
---

 diff-lib.c        |    6 +
 diff.c            |    5 +
 diff.h            |    3 +
 diffcore-rename.c |  227 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 diffcore.h        |    1 
 tree-diff.c       |    4 +
 6 files changed, 235 insertions(+), 11 deletions(-)

diff --git a/diff-lib.c b/diff-lib.c
index ae96c64..dcc4c2c 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -179,7 +179,8 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
 		changed = ce_match_stat(ce, &st, ce_option);
 		if (!changed) {
 			ce_mark_uptodate(ce);
-			if (!DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))
+			if (!DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER) &&
+			    !DIFF_OPT_TST(&revs->diffopt, FACTORIZE_RENAMES))
 				continue;
 		}
 		oldmode = ce->ce_mode;
@@ -310,7 +311,8 @@ static int show_modified(struct oneway_unpack_data *cbdata,
 
 	oldmode = old->ce_mode;
 	if (mode == oldmode && !hashcmp(sha1, old->sha1) &&
-	    !DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))
+	    !DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER) &&
+	    !DIFF_OPT_TST(&revs->diffopt, FACTORIZE_RENAMES))
 		return 0;
 
 	diff_change(&revs->diffopt, oldmode, mode,
diff --git a/diff.c b/diff.c
index e368fef..f91fcf6 100644
--- a/diff.c
+++ b/diff.c
@@ -2437,6 +2437,11 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		DIFF_OPT_SET(options, REVERSE_DIFF);
 	else if (!strcmp(arg, "--find-copies-harder"))
 		DIFF_OPT_SET(options, FIND_COPIES_HARDER);
+	else if (!strcmp(arg, "--factorize-renames")) {
+		DIFF_OPT_SET(options, FACTORIZE_RENAMES);
+		if (!options->detect_rename)
+			options->detect_rename = DIFF_DETECT_RENAME;
+	}
 	else if (!strcmp(arg, "--follow"))
 		DIFF_OPT_SET(options, FOLLOW_RENAMES);
 	else if (!strcmp(arg, "--color"))
diff --git a/diff.h b/diff.h
index a49d865..db1658b 100644
--- a/diff.h
+++ b/diff.h
@@ -65,6 +65,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
 #define DIFF_OPT_IGNORE_SUBMODULES   (1 << 18)
 #define DIFF_OPT_DIRSTAT_CUMULATIVE  (1 << 19)
 #define DIFF_OPT_DIRSTAT_BY_FILE     (1 << 20)
+#define DIFF_OPT_FACTORIZE_RENAMES   (1 << 21)
 #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)
 #define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)
 #define DIFF_OPT_CLR(opts, flag)    ((opts)->flags &= ~DIFF_OPT_##flag)
@@ -220,6 +221,8 @@ extern void diffcore_std(struct diff_options *);
 "  -C            detect copies.\n" \
 "  --find-copies-harder\n" \
 "                try unchanged files as candidate for copy detection.\n" \
+"  --factorize-renames\n" \
+"                factorize renames of all files of a directory.\n" \
 "  -l<n>         limit rename attempts up to <n> paths.\n" \
 "  -O<file>      reorder diffs according to the <file>.\n" \
 "  -S<string>    find filepair whose only one side contains the string.\n" \
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 1b2ebb4..fc789bc 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -52,6 +52,32 @@ static struct diff_rename_dst *locate_rename_dst(struct diff_filespec *two,
 	return &(rename_dst[first]);
 }
 
+static struct diff_rename_dst *locate_rename_dst_dir(struct diff_filespec *dir)
+{
+	/* code mostly duplicated from locate_rename_dst - not sure we
+	 * could merge them efficiently,though
+	 */
+	int first, last;
+	int dirlength = strlen(dir->path);
+
+	first = 0;
+	last = rename_dst_nr;
+	while (last > first) {
+		int next = (last + first) >> 1;
+		struct diff_rename_dst *dst = &(rename_dst[next]);
+		int cmp = strncmp(dir->path, dst->two->path, dirlength);
+		if (!cmp)
+			return dst;
+		if (cmp < 0) {
+			last = next;
+			continue;
+		}
+		first = next+1;
+	}
+	/* not found */
+	return NULL;
+}
+
 /* Table of rename/copy src files */
 static struct diff_rename_src {
 	struct diff_filespec *one;
@@ -409,6 +435,165 @@ static void record_if_better(struct diff_score m[], struct diff_score *o)
 		m[worst] = *o;
 }
 
+struct diff_dir_rename {
+	struct diff_filespec *one;
+	struct diff_filespec *two;
+	int discarded;
+	struct diff_dir_rename* next;
+};
+
+/*
+ * Marks as such file_rename if it is made uninteresting by dir_rename.
+ * Returns -1 if the file_rename is outside of the range in which given
+ * renames concerned by dir_rename are to be found (ie. end of loop),
+ * 0 otherwise.
+ */
+static int maybe_mark_uninteresting(struct diff_rename_dst* file_rename,
+				    struct diff_dir_rename* dir_rename,
+				    int one_len, int two_len)
+{
+	if (!file_rename->pair) /* file add */
+		return 0;
+	if (strncmp(file_rename->two->path,
+		    dir_rename->two->path, two_len))
+		return -1;
+	if (strncmp(file_rename->pair->one->path,
+		    dir_rename->one->path, one_len) ||
+	    !basename_same(file_rename->pair->one, file_rename->two) ||
+	    file_rename->pair->score != MAX_SCORE)
+		return 0;
+
+	file_rename->pair->uninteresting_rename = 1;
+	fprintf (stderr, "[DBG] %s* -> %s* makes %s -> %s uninteresting\n",
+		dir_rename->one->path, dir_rename->two->path,
+		file_rename->pair->one->path, file_rename->two->path);
+	return 0;
+}
+
+// FIXME: prevent possible overflow
+/*
+ * Copy dirname of src into dst, with final "/".
+ * Only handles relative paths since there is no relative path in a git repo.
+ * Writes "./" when there is no "/" in src.
+ * May overwrite more chars than really needed, if src ends with a "/".
+ */
+static const char* copy_dirname(char* dst, const char* src)
+{
+	char* lastslash = strrchr(src, '/');
+	if (!lastslash)
+		return strcpy(dst, "./");
+	strncpy(dst, src, lastslash - src + 1);
+	dst[lastslash - src + 1] = '\0';
+
+	// if src ends with a "/" strip the last component
+	if (lastslash[1] == '\0') {
+		lastslash = strrchr(dst, '/');
+		if (!lastslash)
+			return strcpy(dst, ".");
+		lastslash[1] = '\0';
+	}
+
+	return dst;
+}
+
+/*
+ * FIXME: we could optimize the 100%-rename case by preventing
+ * recursion to unfold what we know we would refold here.
+ * FIXME: do we want to replace linked list with sorted array ?
+ * FIXME: this prototype only handles renaming of dirs without
+ * a subdir.
+ * FIXME: leaks like hell.
+ * FIXME: ideas to evaluate a similarity score, anyone ?
+ *  10% * tree similarity + 90% * moved files similarity ?
+ */
+static struct diff_dir_rename* factorization_candidates = NULL;
+static void diffcore_factorize_renames(void)
+{
+	char one_parent_path[PATH_MAX], two_parent_path[PATH_MAX];
+	int i;
+
+	for (i = 0; i < rename_dst_nr; i++) {
+		// FIXME: what are those ?
+		if (!rename_dst[i].pair)
+			continue;
+		// dummy renames used by copy detection
+		if (!strcmp(rename_dst[i].pair->one->path, rename_dst[i].pair->two->path))
+			continue;
+
+		copy_dirname(one_parent_path, rename_dst[i].pair->one->path);
+
+		struct diff_filespec* one_parent = alloc_filespec(one_parent_path);
+		fill_filespec(one_parent, null_sha1 /*FIXME*/, S_IFDIR);
+
+		if (!locate_rename_dst_dir(one_parent)) {
+			// one_parent_path is empty in result tree
+
+			// already considered ?
+			struct diff_dir_rename* seen;
+			for (seen=factorization_candidates; seen; seen = seen->next)
+				if (!strcmp(seen->one->path, one_parent_path)) break;
+			if (!seen) {
+				// record potential dir rename
+				copy_dirname(two_parent_path, rename_dst[i].pair->two->path);
+
+				seen = xmalloc(sizeof(*seen));
+				seen->one = one_parent;
+				seen->two = alloc_filespec(two_parent_path);
+				fill_filespec(seen->two, null_sha1 /*FIXME*/, S_IFDIR);
+				seen->discarded = 0;
+				seen->next = factorization_candidates;
+				factorization_candidates = seen;
+				fprintf (stderr, "[DBG] %s -> %s suggests possible rename from %s to %s\n",
+				       rename_dst[i].pair->one->path,
+				       rename_dst[i].pair->two->path,
+				       one_parent_path, two_parent_path);
+				fflush(stdout);
+				continue;
+			}
+			if (seen->discarded)
+				continue;
+			// check that seen entry matches this rename
+			copy_dirname(two_parent_path, rename_dst[i].pair->two->path);
+			if (strcmp(two_parent_path, seen->two->path)) {
+				fprintf (stderr, "[DBG] discarding dir split of %s from renames (into %s and %s)\n",
+				       one_parent_path, two_parent_path, seen->two->path);
+				seen->discarded = 1;
+			}
+
+			/* all checks ok, we keep that entry */
+		}
+	}
+
+	// turn candidates into real renames
+	struct diff_dir_rename* candidate;
+	for (candidate=factorization_candidates; candidate; candidate = candidate->next) {
+		int two_idx, i, one_len, two_len;
+		if (candidate->discarded)
+			continue;
+
+		if (!locate_rename_dst_dir(candidate->two)) {
+			fprintf (stderr, "PANIC: %s candidate of rename not in target tree (from %s)\n",
+				candidate->two->path, candidate->one->path);
+		}
+		// bisect to an entry within candidate dst dir
+		two_idx = locate_rename_dst_dir(candidate->two) - rename_dst;
+
+		// now remove extraneous 100% files inside.
+		one_len = strlen(candidate->one->path);
+		two_len = strlen(candidate->two->path);
+		for (i = two_idx; i < rename_dst_nr; i++)
+			if (maybe_mark_uninteresting (rename_dst+i, candidate,
+						      one_len, two_len) < 0)
+				break;
+		for (i = two_idx-1; i >= 0; i--)
+			if (maybe_mark_uninteresting (rename_dst+i, candidate,
+						      one_len, two_len) < 0)
+				break;
+	}
+
+	return;
+}
+
 void diffcore_rename(struct diff_options *options)
 {
 	int detect_rename = options->detect_rename;
@@ -446,13 +631,22 @@ void diffcore_rename(struct diff_options *options)
 				p->one->rename_used++;
 			register_rename_src(p->one, p->score);
 		}
-		else if (detect_rename == DIFF_DETECT_COPY) {
-			/*
-			 * Increment the "rename_used" score by
-			 * one, to indicate ourselves as a user.
-			 */
-			p->one->rename_used++;
-			register_rename_src(p->one, p->score);
+		else {
+			if (detect_rename == DIFF_DETECT_COPY) {
+				/*
+				 * Increment the "rename_used" score by
+				 * one, to indicate ourselves as a user.
+				 */
+				p->one->rename_used++;
+				register_rename_src(p->one, p->score);
+			}
+			if (DIFF_OPT_TST(options, FACTORIZE_RENAMES)) {
+				/* similarly, rename factorization needs to
+				 * see all files from second tree
+				 */
+				//p->two->rename_used++; // FIXME: would we need that ?
+				locate_rename_dst(p->two, 1);
+			}
 		}
 	}
 	if (rename_dst_nr == 0 || rename_src_nr == 0)
@@ -561,8 +755,24 @@ void diffcore_rename(struct diff_options *options)
 	/* At this point, we have found some renames and copies and they
 	 * are recorded in rename_dst.  The original list is still in *q.
 	 */
+
+	/* Now possibly factorize those renames and copies. */
+	if (DIFF_OPT_TST(options, FACTORIZE_RENAMES))
+		diffcore_factorize_renames();
+
 	outq.queue = NULL;
 	outq.nr = outq.alloc = 0;
+
+	// first, turn factorization_candidates into real renames
+	struct diff_dir_rename* candidate;
+	for (candidate=factorization_candidates; candidate; candidate = candidate->next) {
+		struct diff_filepair* pair;
+		if (candidate->discarded) continue;
+		pair = diff_queue(&outq, candidate->one, candidate->two);
+		pair->score = MAX_SCORE;
+		pair->renamed_pair = 1;
+	}
+
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];
 		struct diff_filepair *pair_to_free = NULL;
@@ -577,7 +787,8 @@ void diffcore_rename(struct diff_options *options)
 			struct diff_rename_dst *dst =
 				locate_rename_dst(p->two, 0);
 			if (dst && dst->pair) {
-				diff_q(&outq, dst->pair);
+				if (!dst->pair->uninteresting_rename)
+					diff_q(&outq, dst->pair);
 				pair_to_free = p;
 			}
 			else
diff --git a/diffcore.h b/diffcore.h
index 713cca7..6d2e65b 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -66,6 +66,7 @@ struct diff_filepair {
 	unsigned broken_pair : 1;
 	unsigned renamed_pair : 1;
 	unsigned is_unmerged : 1;
+	unsigned uninteresting_rename : 1;
 };
 #define DIFF_PAIR_UNMERGED(p) ((p)->is_unmerged)
 
diff --git a/tree-diff.c b/tree-diff.c
index 9f67af6..872f757 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -49,7 +49,9 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, const
 		show_entry(opt, "+", t2, base, baselen);
 		return 1;
 	}
-	if (!DIFF_OPT_TST(opt, FIND_COPIES_HARDER) && !hashcmp(sha1, sha2) && mode1 == mode2)
+	if (!DIFF_OPT_TST(opt, FIND_COPIES_HARDER) &&
+	    !DIFF_OPT_TST(opt, FACTORIZE_RENAMES) &&
+	    !hashcmp(sha1, sha2) && mode1 == mode2)
 		return 0;
 
 	/*

^ permalink raw reply related

* [PATCH 2/2] Add testcases for the --factorize-renames diffcore flag.
From: Yann Dirson @ 2008-10-30 22:16 UTC (permalink / raw)
  To: git
In-Reply-To: <20081030220532.3325.54457.stgit@gandelf.nowhere.earth>

This notably includes a couple of tests for cases known not to be
working correctly yet.
---

 t/t4030-diff-rename-factorize.sh |  209 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 209 insertions(+), 0 deletions(-)
 create mode 100755 t/t4030-diff-rename-factorize.sh

diff --git a/t/t4030-diff-rename-factorize.sh b/t/t4030-diff-rename-factorize.sh
new file mode 100755
index 0000000..fcf8fb6
--- /dev/null
+++ b/t/t4030-diff-rename-factorize.sh
@@ -0,0 +1,209 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Yann Dirson
+# Copyright (c) 2005 Junio C Hamano
+#
+
+test_description='Test rename factorization in diff engine.
+
+'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/diff-lib.sh
+
+test_expect_success \
+    'commit the index.'  \
+    'git update-ref HEAD $(echo "original empty commit" | git commit-tree $(git write-tree))'
+
+mkdir a
+echo >a/path0 'Line 1
+Line 2
+Line 3
+Line 4
+Line 5
+Line 6
+Line 7
+Line 8
+Line 9
+Line 10
+line 11
+Line 12
+Line 13
+Line 14
+Line 15
+'
+sed <a/path0 >a/path1 s/Line/Record/
+sed <a/path0 >a/path2 s/Line/Stuff/
+sed <a/path0 >a/path3 s/Line/Blurb/
+
+test_expect_success \
+    'update-index --add file inside a directory.' \
+    'git update-index --add a/path*'
+
+test_expect_success \
+    'write that tree.' \
+    'tree=$(git write-tree) && test -n "$tree"'
+
+test_expect_success \
+    'commit the index.'  \
+    'git update-ref HEAD $(echo "original set of files" | git commit-tree $tree)'
+
+mv a b
+test_expect_success \
+    'renamed the directory.' \
+    'git update-index --add --remove a/path0 a/path1 a/path2 a/path3 b/path*'
+
+test_expect_success \
+    'git diff-index --factorize-renames after directory move.' \
+    'git diff-index --factorize-renames $tree >current'
+grep -v "^\[DBG\] " <current >current.filtered
+cat >expected <<\EOF
+:040000 040000 0000000000000000000000000000000000000000 0000000000000000000000000000000000000000 R100	a/	b/
+EOF
+
+test_expect_success \
+    'validate the output for directory move.' \
+    'compare_diff_patch current.filtered expected'
+
+# now test non-100% renames
+
+echo 'Line 16' >> b/path0
+mv b/path2 b/2path
+rm b/path3
+echo anything > b/path100
+test_expect_success \
+    'edited dir contents.' \
+    'git update-index --add --remove b/* b/path2 b/path3'
+
+test_expect_success \
+    'git diff-index --factorize-renames after directory move and content changes.' \
+    'git diff-index --factorize-renames $tree >current'
+grep -v "^\[DBG\] " <current >current.filtered
+cat >expected <<\EOF
+:040000 040000 0000000000000000000000000000000000000000 0000000000000000000000000000000000000000 R100	a/	b/
+:100644 000000 c6971ab9f08a6cd9c89a0f87d94ae347aad6144a 0000000000000000000000000000000000000000 D	a/path3
+:100644 100644 dbde7141d737c8aa0003672c1bc21ded48c6c3b9 dbde7141d737c8aa0003672c1bc21ded48c6c3b9 R100	a/path2	b/2path
+:100644 100644 fdbec444a77953b1bcc899d9fabfa202e5e68f08 4db595d12886f90e36765fc1732c17bccb836663 R093	a/path0	b/path0
+:000000 100644 0000000000000000000000000000000000000000 1ba4650885513e62386fd3e23aeb45beeb67d3bb A	b/path100
+EOF
+
+test_expect_success \
+    'validate the output for directory move and content changes.' \
+    'compare_diff_patch current.filtered expected'
+
+git reset --hard
+
+# now test bulk moves that are not directory moves (get consensus before going further ?)
+
+mkdir c
+for i in 0 1 2; do cp a/path$i c/apath$i; done
+test_expect_success \
+    'add files into a new directory.' \
+    'git update-index --add c/apath*'
+
+test_expect_success \
+    'commit all this.'  \
+    'git commit -m "first set of changes"'
+
+mv c/* a/
+test_expect_success \
+    'move all of the new dir contents into a preexisting dir.' \
+    'git update-index --add --remove a/* c/apath0 c/apath1 c/apath2'
+
+test_expect_success \
+    'git diff-index --factorize-renames without full-dir rename.' \
+    'git diff-index --factorize-renames HEAD >current'
+grep -v "^\[DBG\] " <current >current.filtered
+cat >expected <<\EOF
+:040000 040000 0000000000000000000000000000000000000000 0000000000000000000000000000000000000000 R100	c/*	a/
+EOF
+
+test_expect_failure \
+    'validate the output for bulk rename without full-dir rename.' \
+    'compare_diff_patch current.filtered expected'
+
+git reset --hard
+
+# now test moves to toplevel (seriously broken)
+
+mv c/* .
+test_expect_success \
+    'move all of the new dir contents into toplevel.' \
+    'git update-index --add --remove apath* c/apath0 c/apath1 c/apath2'
+
+test_expect_success \
+    'git diff-index --factorize-renames files bulk-moved to toplevel.' \
+    'git diff-index --factorize-renames HEAD >current'
+grep -v "^\[DBG\] " <current >current.filtered
+cat >expected <<\EOF
+:040000 040000 0000000000000000000000000000000000000000 0000000000000000000000000000000000000000 R100	c/*	./
+EOF
+
+test_expect_failure \
+    'validate the output for files bulk-moved to toplevel.' \
+    'compare_diff_patch current.filtered expected'
+
+git reset --hard
+
+# now test renaming with subdirs (lacks hiding of renamed subdirs)
+
+mv c a/
+test_expect_success \
+    'move the new dir as subdir of another.' \
+    'git update-index --add --remove a/c/* c/apath0 c/apath1 c/apath2'
+
+test_expect_success \
+    'commit all this.'  \
+    'git commit -m "move as subdir"'
+
+mv a b
+echo foo >> b/c/apath0
+test_expect_success \
+    'rename the directory with some changes.' \
+    'git update-index --add --remove a/path0 a/path1 a/path2 a/path3 a/c/apath0 a/c/apath1 a/c/apath2 b/path* b/c/apath*'
+
+test_expect_success \
+    'git diff-index --factorize-renames on a move including a subdir.' \
+    'git diff-index --factorize-renames HEAD >current'
+grep -v "^\[DBG\] " <current >current.filtered
+cat >expected <<\EOF
+:040000 040000 0000000000000000000000000000000000000000 0000000000000000000000000000000000000000 R100	a/	b/
+:100644 100644 fdbec444a77953b1bcc899d9fabfa202e5e68f08 00084e5ea68b5ae339b7c4b429e4a70fe25d069b R096	a/c/apath0	b/c/apath0
+EOF
+
+test_expect_failure \
+    'validate the output for a move including a subdir.' \
+    'compare_diff_patch current.filtered expected'
+
+# now test moving all files from toplevel into subdir (does not hides file moves) (needs consensus on syntax)
+#FIXME: maybe handle this as special case of move of a dir into one of its own subdirs ?
+
+git reset --hard HEAD~2
+
+mv a/* .
+test_expect_success \
+    'rename the directory with some changes.' \
+    'git update-index --add --remove a/path0 a/path1 a/path2 a/path3 path*'
+
+test_expect_success \
+    'commit all this.'  \
+    'git commit -m "move all files to toplevel"'
+
+mkdir z
+mv path* z/
+test_expect_success \
+    'rename the directory with some changes.' \
+    'git update-index --add --remove path0 path1 path2 path3 z/path*'
+
+test_expect_success \
+    'git diff-index --factorize-renames everything from toplevel.' \
+    'git diff-index --factorize-renames HEAD >current'
+grep -v "^\[DBG\] " <current >current.filtered
+cat >expected <<\EOF
+:040000 040000 0000000000000000000000000000000000000000 0000000000000000000000000000000000000000 R100	./*	z/
+EOF
+
+test_expect_failure \
+    'validate the output for a move of everything from toplevel.' \
+    'compare_diff_patch current.filtered expected'
+
+test_done

^ permalink raw reply related

* Re: [PATCH] Documentation: add a planning document for the next CLI revamp
From: Yann Dirson @ 2008-10-30 22:46 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Sam Vilain, Pierre Habouzit, Mike Hommey, Shawn O. Pearce, git
In-Reply-To: <alpine.LFD.2.00.0810301423520.13034@xanadu.home>

On Thu, Oct 30, 2008 at 02:28:35PM -0400, Nicolas Pitre wrote:
> > It's not about magic, it's about sensible defaults.  Currently this use
> > case is an error, and the resultant command is very long to type, and
> > involves typing the branch name twice.  I end up writing things like:
> > 
> >   git checkout -b {,origin/}wr34251-do-something
> > 
> > For the user who doesn't know to use the ksh-style {} blocks this is
> > voodoo.  The longer form is cumbersome.
> 
> This is no excuse for promoting semantics only useful in such special 
> cases.

It is really not so rare to have an upstream repo with branches such
as "stable", "next" and the like.  This syntax extension would make is
as straightforward to work on "stable" as it is on remote HEAD
(usually master, which has already been magically setup for you).


BTW this use case reminds me that the remote HEAD has its own special
treatment for "clone", which AFAIK cannot be overriden from
command-line (I still sometimes lack what cogito provided as "cg clone
URL#branch").


> As long as it checks it out with a detached head if it is a remote 
> branch then I have no issue.

Yes it is possible, but that does not necessarily make a UI
improvement worthless.

Best regards,
-- 
Yann

^ permalink raw reply

* Re: [PATCH 1/2] Add --verbose|-v to test-chmtime
From: Jeff King @ 2008-10-30 22:58 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Git Mailing List, Junio C Hamano, René Scharfe
In-Reply-To: <81b0412b0810300426u2ccbe51at1bf5f989b6333ed1@mail.gmail.com>

On Thu, Oct 30, 2008 at 12:26:24PM +0100, Alex Riesen wrote:

> This allows us replace perl when getting the mtime of a file because
> of time zone conversions, though at the moment only one platform which
> does this has been identified: Cygwin when used with ActiveState Perl
> (as usual).
> [...]
>     test-chmtime -v +0 filename1 | cut -f 1

Personally, I would have:

  - split the argument refactoring and the addition of the "-v" argument
    into two patches to make reviewers lives easier

  - just used a special timespec that means "don't change anything, but
    show show"

but I think those are mostly nitpicks, so I am OK with the series as-is.

-Peff

^ permalink raw reply

* Re: Using the --track option when creating a branch
From: Jakub Narebski @ 2008-10-30 23:24 UTC (permalink / raw)
  To: git
In-Reply-To: <2008-10-30-15-23-16+trackit+sam@rfc1149.net>

[Cc: Samuel Tardieu <sam@rfc1149.net>, Andreas Ericsson <ae@op5.se>,
     git@vger.kernel.org]

Samuel Tardieu wrote:
> * Andreas Ericsson <ae@op5.se> [2008-10-30 15:06:16 +0100]
> 
>> --all pushes all refs, even the non-matching ones, which is very
>> rarely desirable and only accidentally sometimes the same as "push all
>> matching refs".
>>
>>> I know that I've never had the intent to push all the refs without
>>> thinking about it first. Most of the time, I intend to push only
>>> the current branch I am in.
>>
>> Then say so. There's a very simple command syntax for it:
>> "git push <remote> <current-branch>"
> 
> I update the branches I'm working in maybe 20 times a day, sometimes
> more. When I make a change and all the tests pass, I prefer to call
> 
>   git push
> 
> rather than
> 
>   git push origin 2.0-beta1
> 
> (and "2.0-beta1" is a short name here, some branches have much longer
> names)

You can use

  $ git push origin HEAD

and I think (but I am not sure) that there is DWIM-mery allowing
to simply say

  $ git push HEAD

and it would use configured branch.$(git symbolic-ref HEAD).remote


And if it is not as I said, the patches would better made it so, instead of
changing default behavior from push matching refspecs to push current
branch only.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [PATCH] Documentation: add a planning document for the next CLI revamp
From: Theodore Tso @ 2008-10-30 23:27 UTC (permalink / raw)
  To: Sam Vilain; +Cc: git, Sam Vilain
In-Reply-To: <1225389068.19891.28.camel@maia.lan>

On Thu, Oct 30, 2008 at 10:51:08AM -0700, Sam Vilain wrote:
> 
> Well, I don't have strong feelings on the exact command name used; I
> suggested "undo", probably also ambiguous.  But still, a significant
> number of users are surprised when they type 'git revert' and they get a
> backed out patch.

Yeah, that's why I suggested "git revert-file".  It's less ambiguous
than "undo", and it's easier for people used to "hg revert" and "svn
revert" to find "git revert-file".  And, it won't be run accidentally
by old-timers who are used to the old (to be deprecated) "git revert".
But I'm not that picky about the name; I just missed the "git undo"
proposal in your patch.

> Making it plain "revert" would violate expectations of existing users;
> it seems a better idea to just deprecate it, and point the users to the
> new method - cherry-pick --revert - or the command they might have meant
> - whatever that becomes.

Yup, I agree; that's why I suggested "git revert-file".

							- Ted

^ permalink raw reply

* Re: [PATCH] Documentation: add a planning document for the next CLI revamp
From: Sam Vilain @ 2008-10-30 23:28 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Pierre Habouzit, Mike Hommey, Shawn O. Pearce, git
In-Reply-To: <alpine.LFD.2.00.0810301423520.13034@xanadu.home>

On Thu, 2008-10-30 at 14:28 -0400, Nicolas Pitre wrote:
> > For the case where the thing you type is a resolvable reference, it
> > would just check it out, as now.
> As long as it checks it out with a detached head if it is a remote 
> branch then I have no issue.

Absolutely - if you've already got a branch "master", then
"git checkout master" should definitely give it to you.  If you go
"git checkout origin/master", you get a floating head.  But I quite often
find myself wanting to check out a remote branch, and give it a name just
like on the remote.  I want "git checkout blah" to assume that's
what I mean, until I make a local branch "blah".

> By default, git creates a branch called "master.  Hence, by default, if 
> you clone that repository, this branch will be called origin/master.  So 
> by default $foo is already ambiguous.

Right - 'master' in this case resolves to something.  The ambiguity is
resolved by defaulting to the thing that resolves.  The fall-back
behaviour is only triggered if you asked for something that is currently
an error.  Because breaking expectations sucks.

Sam.

^ permalink raw reply

* Re: [PATCH] Documentation: add a planning document for the next CLI revamp
From: Jakub Narebski @ 2008-10-30 23:55 UTC (permalink / raw)
  To: git
In-Reply-To: <1225387882.19891.9.camel@maia.lan>

Sam Vilain wrote:

> It's not about magic, it's about sensible defaults.  Currently this use
> case is an error, and the resultant command is very long to type, and
> involves typing the branch name twice.  I end up writing things like:
> 
>   git checkout -b {,origin/}wr34251-do-something

Can't you use currently

    git checkout --track origin/wr34251-do-something


P.S. Somehow I don't see first message in this thread on GMane...
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [PATCH] t4030: Don't use echo -n
From: Ian Hilt @ 2008-10-30 23:52 UTC (permalink / raw)
  To: Brian Gernhardt; +Cc: Git Mailing List
In-Reply-To: <1225404776-51748-1-git-send-email-benji@silverinsanity.com>

On Thu, Oct 30, 2008 at 06:12:56PM -0400, Brian Gernhardt wrote:
> Signed-off-by: Brian Gernhardt <benji@silverinsanity.com>

No commit message???

> ---
>  t/t4030-diff-textconv.sh |    5 ++---
>  1 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh
> index 3945731..7ec244f 100755
> --- a/t/t4030-diff-textconv.sh
> +++ b/t/t4030-diff-textconv.sh
> @@ -96,16 +96,15 @@ cat >expect.typechange <<'EOF'
>  -1
>  diff --git a/file b/file
>  new file mode 120000
> -index ad8b3d2..67be421
> +index ad8b3d2..8e4020b
>  --- /dev/null
>  +++ b/file
>  @@ -0,0 +1 @@
>  +frotz
> -\ No newline at end of file
>  EOF
>  # make a symlink the hard way that works on symlink-challenged file systems
>  test_expect_success 'textconv does not act on symlinks' '
> -	echo -n frotz > file &&
> +	echo frotz > file &&
>  	git add file &&
>  	git ls-files -s | sed -e s/100644/120000/ |
>  		git update-index --index-info &&
> -- 
> 1.6.0.3.523.g304d0

This doesn't apply for me.  My tip is up-to-date, but it doesn't even have
this file it t/.

A little bewildered,

	Ian

^ permalink raw reply

* [PATCH] bash completion: add doubledash to "git show"
From: Markus Heidelberg @ 2008-10-31  0:04 UTC (permalink / raw)
  To: gitster; +Cc: git

Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---
 contrib/completion/git-completion.bash |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index eebe734..de193ba 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1398,6 +1398,8 @@ _git_shortlog ()
 
 _git_show ()
 {
+	__git_has_doubledash && return
+
 	local cur="${COMP_WORDS[COMP_CWORD]}"
 	case "$cur" in
 	--pretty=*)
-- 
1.6.0.3.524.g3adb7d

^ permalink raw reply related

* Re: [JGIT PATCH 0/3] Improved object validation
From: Robin Rosenberg @ 2008-10-31  0:01 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <1225388785-26818-1-git-send-email-spearce@spearce.org>

torsdagen den 30 oktober 2008 18.46.22 skrev Shawn O. Pearce:
> This is mostly a resend as I haven't heard anything on the series.
> One new patch at the end, to handle '.' and '..' cases.
Ack, pushed now.  I never got to the push action, because I wanted
to do some testing on the bundlewriter (which you did not resubmit) and
the I went to a warmer place for a few days.

These patches and the bundlewrite + a small unit test for it now pushed.

-- robin

^ permalink raw reply

* Re: [JGIT RFC PATCH 3/3] Rate limit warnings spewed by RepositoryTestCase.recursiveDelete
From: Robin Rosenberg @ 2008-10-31  0:16 UTC (permalink / raw)
  To: Jonas Fonseca; +Cc: Shawn O. Pearce, git
In-Reply-To: <20081030104620.GB17131@diku.dk>

torsdagen den 30 oktober 2008 11.46.20 skrev Jonas Fonseca:
> Robin Rosenberg <robin.rosenberg@dewire.com> wrote Fri, Oct 24, 2008:
> > onsdagen den 22 oktober 2008 10.34.20 skrev Jonas Fonseca:
> > > On Windows XP / NTFS / NetBeans 6.1 / Java 5 a lot of warnings are
> > > printed. In most cases the path is in fact deleted and it seems to
> > > just be a timing bug or something Windows or NTFS specific.
> >
> > The problem is actually flaws in the unit tests and in the supporting
> > RepositoryTestCase.  I think I'll fix that instead.
> 
> Thanks. Any progress on this? Can I help in any way?

Ok here is a W-I-P. One test case is not cleaning up properly. You may note that I
still give some room for gc not to unmap immediately, but at the end of the program
we hope that will be the case and I think the missing case is just a bug and not GC
being delayed.  The counter testcount is only here for debugging this problem.

-- robin

>From b3907a11bec0158a99bc77a7655379725203da9c Mon Sep 17 00:00:00 2001
From: Robin Rosenberg <robin.rosnberg@dewire.com>
Date: Thu, 23 Oct 2008 01:27:50 +0200
Subject: [EGIT PATCH] Make the cleanup less verbose when it fails to delete temporary stuff.

---
 .../org/spearce/jgit/lib/RepositoryTestCase.java   |   57 +++++++++++++++-----
 1 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
index 9d7d133..8015a18 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
@@ -47,6 +47,7 @@
 import java.io.Reader;
 
 import junit.framework.TestCase;
+
 import org.spearce.jgit.util.JGitTestUtil;
 
 public abstract class RepositoryTestCase extends TestCase {
@@ -66,22 +67,34 @@
 		jcommitter = new PersonIdent("J. Committer", "jcommitter@example.com");
 	}
 
-	protected static void recursiveDelete(final File dir) {
+	protected static boolean recursiveDelete(final File dir) {
+		return recursiveDelete(dir, false);
+	}
+
+	protected static boolean recursiveDelete(final File dir, boolean silent) {
+		if (!dir.exists())
+			return silent;
 		final File[] ls = dir.listFiles();
 		if (ls != null) {
 			for (int k = 0; k < ls.length; k++) {
 				final File e = ls[k];
 				if (e.isDirectory()) {
-					recursiveDelete(e);
+					silent = recursiveDelete(e, silent);
 				} else {
-					e.delete();
+					if (!e.delete()) {
+						if (!silent)
+							System.out.println("Warning: Failed to delete " + e);
+						silent = true;
+					}
 				}
 			}
 		}
-		dir.delete();
-		if (dir.exists()) {
-			System.out.println("Warning: Failed to delete " + dir);
+		if (!dir.delete()) {
+			if (!silent)
+				System.out.println("Warning: Failed to delete " + dir);
+			silent = true;
 		}
+		return silent;
 	}
 
 	protected static void copyFile(final File src, final File dst)
@@ -121,19 +134,31 @@ protected static void checkFile(File f, final String checkData)
 
 	protected Repository db;
 
+	private static int testcount;
+	private static Thread shutdownhook;
+
 	public void setUp() throws Exception {
 		super.setUp();
-		recursiveDelete(trashParent);
-		trash = new File(trashParent,"trash"+System.currentTimeMillis());
+		System.gc();
+		trash = new File(trashParent,"xtrash"+System.currentTimeMillis()+"."+(testcount++));
+		recursiveDelete(trashParent, true);
 		trash_git = new File(trash, ".git");
-
+		if (shutdownhook == null) {
+			shutdownhook = new Thread() {
+				@Override
+				public void run() {
+					System.gc();
+					recursiveDelete(trashParent, false);
+				}
+			};
+			Runtime.getRuntime().addShutdownHook(shutdownhook);
+		}
 		Runtime.getRuntime().addShutdownHook(new Thread() {
 			@Override
 			public void run() {
-				recursiveDelete(trashParent);
+				recursiveDelete(trash);
 			}
 		});
-
 		db = new Repository(trash_git);
 		db.create();
 
@@ -170,12 +195,18 @@ protected void tearDown() throws Exception {
 	 * @throws IOException
 	 */
 	protected Repository createNewEmptyRepo() throws IOException {
-		File newTestRepo = new File(trashParent, "new"+System.currentTimeMillis()+"/.git");
+		final File newTestRepo = new File(trashParent, "new"+System.currentTimeMillis()+"."+(testcount++)+"/.git");
 		assertFalse(newTestRepo.exists());
-		File unusedDir = new File(trashParent, "tmp"+System.currentTimeMillis());
+		File unusedDir = new File(trashParent, "tmp"+System.currentTimeMillis()+"."+(testcount++));
 		assertTrue(unusedDir.mkdirs());
 		final Repository newRepo = new Repository(newTestRepo);
 		newRepo.create();
+		Runtime.getRuntime().addShutdownHook(new Thread() {
+			@Override
+			public void run() {
+				recursiveDelete(newTestRepo);
+			}
+		});
 		return newRepo;
 	}
 
-- 
1.6.0.2.308.gef4a

^ permalink raw reply related

* Re: git adds some text file as binary file by mistake
From: Robin Rosenberg @ 2008-10-31  0:29 UTC (permalink / raw)
  To: Ping Yin; +Cc: Michael J Gruber, Git Mailing List
In-Reply-To: <46dff0320810292332y34ea0daemde05b58572946497@mail.gmail.com>

torsdagen den 30 oktober 2008 07.32.47 skrev Ping Yin:
> On Fri, Oct 24, 2008 at 9:30 PM, Ping Yin <pkufranky@gmail.com> wrote:
> > On Fri, Oct 24, 2008 at 8:54 PM, Michael J Gruber
> > <git@drmicha.warpmail.net> wrote:
> >> Ping Yin venit, vidit, dixit 24.10.2008 14:37:
> >>> So what should i do if i want it added as text file?
> >>
> >> You should give us more detail on the file ;)
> >> What's the extension, what's the typical content? It may be a simple
> >> matter of specifying attributes.
> >> Do ordinary diff and grep recognize your files as text?
> >>
> >> Michael
> >>
> >
> > It's just an xml file. I guess maybe there are some hidden characters
> > at the beginning. I will figure it out later because i have no access
> > to that file right now.
> >
> > Ping Yin
> >
> 
> I have figured it out. It's just because the BOM characters feff00 in
> the beginning of the utf-8 xml file.

The UTF-8 BOM is EF BB BF, (no NUL.). FEFF is UTF-16 BE (and the third
byte would be 00 for an XML file). I think git currently regards just about
any UTF-16 file as binary becuase of the NUL bytes.

-- robin

^ 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