Git development
 help / color / mirror / Atom feed
* Re: [PATCH 2/2] git-gui: Allow "Stage Line" to stage adjacent changes independently
From: Junio C Hamano @ 2008-07-15 21:49 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Shawn O. Pearce, git
In-Reply-To: <1216156261-9687-2-git-send-email-johannes.sixt@telecom.at>

Johannes Sixt <johannes.sixt@telecom.at> writes:

> Consider this hunk:
>
>   @@ -10,4 +10,4 @@
>    context before
>   -old 1
>   -old 2
>   +new 1
>   +new 2
>    context after
>
> [Nomenclature: to "stage change 2" means to stage lines "-old 1" and
> "+new 1", in any order; likewise for "unstage" and "change 2".]

You lost me.

Do you mean to say that you always interpret the above hunk as:

   @@ -10,4 +10,4 @@
    context before
   -old 1
   +new 1
   -old 2
   +new 2
    context after

and call "replace 'old 1' with 'new 1'" as "change 1", "replace 'old
2' with 'new 2'" as "change 2"?

If it is what you are doing, it does not make much sense to me.  "new 1"
may correspond to "old 1" and "old 2" while "new 2" may be an independent
addition.  E.g.

   @@ -10,4 +10,4 @@
    context before
   -#define add(x,y) \
   - (x) + (y)
   +#define add(x,y) ((x)+(y))
   +#define sub(x,y) ((x)-(y))
    context after

I might want to pick bugfix of add() definition without using the new
definition of sub().

Please call

	"-old 1" - change #1
        "-old 2" - change #2
        "+new 1" - change #3
        "+new 2" - change #4

and try explaining what you are doing again, pretty please?

^ permalink raw reply

* Re: Git rebase failure: .dotest overwritten
From: René Scharfe @ 2008-07-15 21:48 UTC (permalink / raw)
  To: Stephan Beyer; +Cc: Joe Fiorini, git, Johannes Schindelin, Jari Aalto
In-Reply-To: <20080715212211.GL6244@leksak.fem-net>

Stephan Beyer schrieb:
> git-rebase (without -i/-m) generates a directory called ".dotest/" to
> save temporary stuff like the commits you want to rebase.

In February it was discussed to move .dotest below $GIT_DIR.  There was
even a patch (to rename it to .git-dotest).  I suspect the upcoming
version 1.6.0 is a good opportunity to finally remove this wart.

René

^ permalink raw reply

* Re: [PATCH 1/4] Allow e-mails to be sent with the Unix sendmail tool
From: Catalin Marinas @ 2008-07-15 21:47 UTC (permalink / raw)
  To: Karl Hasselström; +Cc: Mark Brown, git
In-Reply-To: <20080715123432.GB17008@diana.vm.bytemark.co.uk>

2008/7/15 Karl Hasselström <kha@treskal.com>:
> On 2008-07-15 13:22:20 +0100, Mark Brown wrote:
>
>> On Sun, Jul 13, 2008 at 12:40:26PM +0100, Catalin Marinas wrote:
>>
>> > If the stgit.smtpserver configuration option does not have a
>> > host:port format, it is assumed to be an external tool. For
>> > example, to use sendmail just set this variable to
>> > "/usr/sbin/sendmail -t -i" (see the examples/gitconfig file).
>>
>> I'd really expect to be able to just specify the hostname alone if
>> using the standard SMTP port. Perhaps checking for a / in the server
>> might be less surprising?
>
> This is actually what the code does, and what the documentation claims
> it should do. It's just the commit message that's confused. Catalin?

Yes, indeed, but too many things to keep in sync :-).

-- 
Catalin

^ permalink raw reply

* Re: [PATCH] Only use GIT_CONFIG in "git config", not other programs
From: Daniel Barkalow @ 2008-07-15 21:46 UTC (permalink / raw)
  To: sverre; +Cc: Junio C Hamano, git, Johannes Schindelin
In-Reply-To: <bd6139dc0807151056q61a9379l84761fe021d4541c@mail.gmail.com>

On Tue, 15 Jul 2008, Sverre Rabbelier wrote:

> On Mon, Jun 30, 2008 at 9:37 AM, Daniel Barkalow <barkalow@iabervon.org> wrote:
> > @@ -611,31 +613,28 @@ int git_config(config_fn_t fn, void *data)
> >  {
> >        int ret = 0;
> >        char *repo_config = NULL;
> > -       const char *home = NULL, *filename;
> > +       const char *home = NULL;
> >
> >        /* $GIT_CONFIG makes git read _only_ the given config file,
> >         * $GIT_CONFIG_LOCAL will make it process it in addition to the
> >         * global config file, the same way it would the per-repository
> >         * config file otherwise. */
> 
> I noticed today while looking at config.c (because of the 'git config
> oddity' thread) that this reference to GIT_CONFIG_LOCAL was not
> removed?

Actually, it looks like that whole comment is obsolete; only 
builtin-config.c cares about any of this stuff.

> On latest next:
> $ grep GIT_CONFIG_LOCAL *
> config.c:	 * $GIT_CONFIG_LOCAL will make it process it in addition to the
> git-svn:		my $file = $ENV{GIT_CONFIG} || $ENV{GIT_CONFIG_LOCAL} ||
> git-svn.perl:		my $file = $ENV{GIT_CONFIG} || $ENV{GIT_CONFIG_LOCAL} ||

I'm not sure if git-svn is doing the right thing here, either.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: Git rebase failure: .dotest overwritten
From: Stephan Beyer @ 2008-07-15 21:22 UTC (permalink / raw)
  To: Joe Fiorini; +Cc: git
In-Reply-To: <73fd69b50807151408i6a916da6p7b89fe81e65fc717@mail.gmail.com>

Hi,

Joe Fiorini wrote:
> A friend of mine asked me for help with a git rebase problem.  I was
> clueless.  Here is what he was seeing:
> The main problem seems to be the following error: "fatal: Untracked
> working tree file '.dotest/0001' would be overwritten by merge."; also
> all the files that it says "already exist in working directory" did
> not exist before he ran the rebase.  Maybe it's trying to run the
> rebase more than once? His working copy was clean.  Any thoughts?  Any
> other details I can provide?

git-rebase (without -i/-m) generates a directory called ".dotest/" to
save temporary stuff like the commits you want to rebase.

And it seems that at least one commit in his repo has .dotest/* files
in the tree, so that message occurs.
Hmm, he should perhaps clean that commit up and remove the .dotest
files from it. (After figuring out that commit, he could try
	git rebase -i ...
and change the "pick" line to "edit".)

Regards.

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F

^ permalink raw reply

* [PATCH 2/2] git-gui: Allow "Stage Line" to stage adjacent changes independently
From: Johannes Sixt @ 2008-07-15 21:11 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Johannes Sixt
In-Reply-To: <1216156261-9687-1-git-send-email-johannes.sixt@telecom.at>

Consider this hunk:

  @@ -10,4 +10,4 @@
   context before
  -old 1
  -old 2
  +new 1
  +new 2
   context after

[Nomenclature: to "stage change 2" means to stage lines "-old 1" and
"+new 1", in any order; likewise for "unstage" and "change 2".]

Previously, we had this situation:

  stage change 1 was not possible
  stage change 2 was possible
  unstage change 1 was possible
  unstage change 2 was not possible

With this change we have this situation:

  stage change 1 is possible
  stage change 2 is not possible
  unstage change 1 is possible
  unstage change 2 is not possible

What's the deal?

Previously, it was impossible to stage change 1 without also staging
change 2; not even by staging the complete hunk and unstaging the unwanted
part.

With this change we can achieve all goals:

  hunk is     want staged   want unstaged   do this
  ----------------------------------------------------------------------
  unstaged    change 1      change 2        stage change 1
  unstaged    change 2      change 1        stage hunk, unstage change 1
  staged      change 1      change 2        unstage hunk, stage change 1
  staged      change 2      change 1        unstage change 1

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 lib/diff.tcl |   61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/lib/diff.tcl b/lib/diff.tcl
index ee7f391..77990c5 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -411,6 +411,53 @@ proc apply_line {x y} {
 	set hh [lindex [split $hh ,] 0]
 	set hln [lindex [split $hh -] 1]
 
+	# There is a special situation to take care of. Consider this hunk:
+	#
+	#    @@ -10,4 +10,4 @@
+	#     context before
+	#    -old 1
+	#    -old 2
+	#    +new 1
+	#    +new 2
+	#     context after
+	#
+	# We used to keep the context lines in the order they appear in the
+	# hunk. But then it is not possible to correctly stage only
+	# "-old 1" and "+new 1" - it would result in this staged text:
+	#
+	#    context before
+	#    old 2
+	#    new 1
+	#    context after
+	#
+	# (By symmetry it is not possible to *un*stage "old 2" and "new 2".)
+	#
+	# We resolve the problem by introducing an asymmetry, namely, when
+	# a "+" line is *staged*, it is moved in front of the context lines
+	# that are generated from the "-" lines that are immediately before
+	# the "+" block. That is, we construct this patch:
+	#
+	#    @@ -10,4 +10,5 @@
+	#     context before
+	#    +new 1
+	#     old 1
+	#     old 2
+	#     context after
+	#
+	# But we do *not* treat "-" lines that are *un*staged in a special
+	# way.
+	#
+	# With this asymmetry it is possible to stage the change
+	# "old 1" -> "new 1" directly, and to stage the change
+	# "old 2" -> "new 2" by first staging the entire hunk and
+	# then unstaging the change "old 1" -> "new 1".
+
+	# This is non-empty if and only if we are _staging_ changes;
+	# then it accumulates the consecutive "-" lines (after converting
+	# them to context lines) in order to be moved after the "+" change
+	# line.
+	set pre_context {}
+
 	set n 0
 	set i_l [$ui_diff index "$i_l + 1 lines"]
 	set patch {}
@@ -422,19 +469,27 @@ proc apply_line {x y} {
 		    [$ui_diff compare $the_l < $next_l]} {
 			# the line to stage/unstage
 			set ln [$ui_diff get $i_l $next_l]
-			set patch "$patch$ln"
 			if {$c1 eq {-}} {
 				set n [expr $n+1]
+				set patch "$patch$pre_context$ln"
+			} else {
+				set patch "$patch$ln$pre_context"
 			}
+			set pre_context {}
 		} elseif {$c1 ne {-} && $c1 ne {+}} {
 			# context line
 			set ln [$ui_diff get $i_l $next_l]
-			set patch "$patch$ln"
+			set patch "$patch$pre_context$ln"
 			set n [expr $n+1]
+			set pre_context {}
 		} elseif {$c1 eq $to_context} {
 			# turn change line into context line
 			set ln [$ui_diff get "$i_l + 1 chars" $next_l]
-			set patch "$patch $ln"
+			if {$c1 eq {-}} {
+				set pre_context "$pre_context $ln"
+			} else {
+				set patch "$patch $ln"
+			}
 			set n [expr $n+1]
 		}
 		set i_l $next_l
-- 
1.5.6.3.323.g1e58

^ permalink raw reply related

* [PATCH 1/2] git-gui: Fix "Stage/Unstage Line" with one line of context.
From: Johannes Sixt @ 2008-07-15 21:11 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Johannes Sixt

To "Stage/Unstage Line" we construct a patch that contains exactly one
change (either addition or removal); the hunk header was forged by counting
the old side and adjusting the count by +/-1 for the new side. But when we
counted the context we never counted the changed line itself. If the hunk
had only one removal line and one line of context, like this:

    @@ -1,3 +1,2 @@
     context 1
    -removal
     context 2

We had constructed this patch:

    @@ -1,2 +1,1 @@
     context 1
    -removal
     context 2

which does not apply because git apply deduces that it must apply at the
end of the file. ("context 2" is considered garbage and ignored.) The fix
is that removal lines must be counted towards the context of the old side.

Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
 lib/diff.tcl |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/lib/diff.tcl b/lib/diff.tcl
index 96ba949..ee7f391 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -423,6 +423,9 @@ proc apply_line {x y} {
 			# the line to stage/unstage
 			set ln [$ui_diff get $i_l $next_l]
 			set patch "$patch$ln"
+			if {$c1 eq {-}} {
+				set n [expr $n+1]
+			}
 		} elseif {$c1 ne {-} && $c1 ne {+}} {
 			# context line
 			set ln [$ui_diff get $i_l $next_l]
-- 
1.5.6.3.323.g1e58

^ permalink raw reply related

* Git rebase failure: .dotest overwritten
From: Joe Fiorini @ 2008-07-15 21:08 UTC (permalink / raw)
  To: git

A friend of mine asked me for help with a git rebase problem.  I was
clueless.  Here is what he was seeing:
The main problem seems to be the following error: "fatal: Untracked
working tree file '.dotest/0001' would be overwritten by merge."; also
all the files that it says "already exist in working directory" did
not exist before he ran the rebase.  Maybe it's trying to run the
rebase more than once? His working copy was clean.  Any thoughts?  Any
other details I can provide?
Thanks!
Joe
git rebase master
First, rewinding head to replay your work on top of it...
HEAD is now at 9bba5f1... Create function snippet
Applying People page templates
.dotest/patch:59: trailing whitespace.

.dotest/patch:60: space before tab in indent.
  parent::BuildControlArray($EventParameters);
.dotest/patch:61: space before tab in indent.
  }
.dotest/patch:65: trailing whitespace, space before tab in indent.

.dotest/patch:67: trailing whitespace.

error: .dotest/0001: already exists in working directory
error: .dotest/0002: already exists in working directory
error: .dotest/0003: already exists in working directory
error: .dotest/binary: already exists in working directory
error: .dotest/final-commit: already exists in working directory
error: .dotest/info: already exists in working directory
error: .dotest/keep: already exists in working directory
error: .dotest/last: already exists in working directory
error: .dotest/msg: already exists in working directory
error: .dotest/msg-clean: already exists in working directory
error: .dotest/next: already exists in working directory
error: .dotest/patch: already exists in working directory
error: .dotest/sign: already exists in working directory
error: .dotest/utf8: already exists in working directory
error: .dotest/whitespace: already exists in working directory
Using index info to reconstruct a base tree...
stdin:59: trailing whitespace.

stdin:60: space before tab in indent.
  parent::BuildControlArray($EventParameters);
stdin:61: space before tab in indent.
  }
stdin:65: trailing whitespace, space before tab in indent.

stdin:67: trailing whitespace.

warning: squelched 49 whitespace errors
warning: 54 lines add whitespace errors.
Falling back to patching base and 3-way merge...
fatal: Untracked working tree file '.dotest/0001' would be overwritten by merge.
Failed to merge in the changes.
Patch failed at 0001.
When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".
--
joe fiorini
http://www.faithfulgeek.org
// freelancing & knowledge sharing

^ permalink raw reply

* Re: [PATCH] git rev-parse: Fix --show-cdup inside symlinked   directory
From: Yves Orton @ 2008-07-15 20:26 UTC (permalink / raw)
  To: Rogan Dawes; +Cc: Petr Baudis, Johannes Schindelin, gitster, git
In-Reply-To: <487CF5A4.2070700@dawes.za.net>

On Tue, 2008-07-15 at 21:08 +0200, Rogan Dawes wrote:
> Yves Orton wrote:
> 
> > Hmm, realizing that was the workdir it wanted i tried it like so:
> > 
> > [dmq@somewhere apps]$ git --work-tree="$(git-rev-parse --git-dir)/.."
> > pull --rebase
> > /usr/bin/git-sh-setup: line 139: cd: /home/dmq/git_tree/main/apps/.git:
> > No such file or directory
> > Unable to determine absolute path of git directory
> > 
> > Yet:
> > 
> > [dmq@somewhere apps]$ git-rev-parse --git-dir
> > /home/dmq/git_tree/main/.git
> > 
> > is correct.
> > 
> 
> Are you sure you don't want to specify the --git-dir rather than the 
> work dir?
> 
> i.e.
> 
> git --git-dir="$(git-rev-parse --git-dir)" pull --rebase

That doesnt seem to work correctly either. If i do it from the symlinked
directory i get a notice about each file needing an update. While it
works as expected from the real repo directory.

I think this shows what i mean:

demerphq@gemini:~/git_test/bar$ git status
# On branch master
# Changed but not updated:
#   (use "git add <file>..." to update what will be committed)
#
#       modified:   bar
#
no changes added to commit (use "git add" and/or "git commit -a")
demerphq@gemini:~/git_test/bar$ git commit -a -m'changed bar'
Created commit 7cbbdc9: changed bar
 1 files changed, 1 insertions(+), 0 deletions(-)
demerphq@gemini:~/git_test/bar$ git --git-dir="$(git-rev-parse
--git-dir)" pull --rebase
bar/bar: needs update
refusing to pull with rebase: your working tree is not up-to-date
demerphq@gemini:~/git_test/bar$ cd ../foo2
demerphq@gemini:~/git_test/foo2$ git --git-dir="$(git-rev-parse
--git-dir)" pull --rebase
Current branch master is up to date.
demerphq@gemini:~/git_test/foo2$ cd ..
demerphq@gemini:~/git_test$ ls -lart
total 24
drwxr-xr-x   4 demerphq demerphq  4096 2008-07-15 22:17 foo
drwxr-xr-x 116 demerphq demerphq 12288 2008-07-15 22:18 ..
lrwxrwxrwx   1 demerphq demerphq     8 2008-07-15 22:20 bar -> foo2/bar
drwxr-xr-x   4 demerphq demerphq  4096 2008-07-15 22:20 .
drwxr-xr-x   4 demerphq demerphq  4096 2008-07-15 22:21 foo2



Yves

^ permalink raw reply

* Re: git-rebase eats empty commits
From: Stephan Beyer @ 2008-07-15 20:19 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git
In-Reply-To: <g5fpnm$3jb$1@ger.gmane.org>

Hi,

Michael J Gruber wrote:
> Stephan Beyer venit, vidit, dixit 13.07.2008 00:12:
>> To sum up, use rebase -i and when it's pausing, do "git commit --allow-empty"
>> and then "git rebase --continue" and you have what you want.
>
> I assume this is with git from master? With git 1.5.6.2 rebase -i  
> doesn't stop there, not even when I change "pick" to "edit"!

Hmm, in fact this is with my git from Debian, from master and my
sequencer-based one:

	$ /usr/bin/git --version
	git version 1.5.6
	$ ./git --version
	git version 1.5.6.3.350.g6c11a
	$ git --version
	git version 1.5.6.3.506.g902dd

And, btw, the reason of this behavior is no special case in rebase-i,
it's just that git-cherry-pick fails (exit status != 0), if you pick an
empty commit (and then rebase-i will pause because of conflict). And
*this* is because builtin-revert.c runs git-commit without --allow-empty.
This has never changed, so I cannot believe that the behavior changed in
any version of git. Or I am missing a point.

	$ git cherry-pick empty		# empty is an empty commit tagged as "empty"
	Already uptodate!
	Finished one cherry-pick.
	# Not currently on any branch.
	# Untracked files:
	[...]
	nothing added to commit but untracked files present (use "git add" to track)
	$ echo $?
	1
	$ EDITOR=touch git commit --allow-empty
	Created commit 145f1d0: empty

The same happens when doing rebase -i instead of cherry-pick.

Regards,
  Stephan

-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F

^ permalink raw reply

* Re: Standard "git svn init ; git svn fetch" behavior
From: "Peter Valdemar Mørch (Lists)" @ 2008-07-15 20:14 UTC (permalink / raw)
  Cc: git
In-Reply-To: <eaa105840807150737u11b02f50xba2c8201ce96eec0@mail.gmail.com>

Peter, thanks for your swift reply! It cleared up things for me.

^ permalink raw reply

* StGit: kha/{stable,safe,experimental} updated
From: Karl Hasselström @ 2008-07-15 19:58 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git

It's been a while since I sent one of these updates.

Catalin, you probably want to pull stable and safe, to get Miklos's
setup.py fixes (I backported them to stable, and then merged back to
the main branch). (The stable branch has a handful of fixes since
0.14.3; it might be time for 0.14.4?)


                                 -+-


The following changes since commit a6c4be12abcf0906a63de8a72c887c360f89ea82:
  Karl Hasselström (1):
        Don't allow extra diff options with "stg status"

are available in the git repository at:

  git://repo.or.cz/stgit/kha.git stable

Miklos Vajna (2):
      setup.py: don't try to import stgit.run before the python version check
      setup.py: fix error message when running with python-2.3

 setup.py |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)


                                 -+-


The following changes since commit 2e37b61d886ef21200007b57f496aaf182f42705:
  Karl Hasselström (1):
        Test for "stg edit"

are available in the git repository at:

  git://repo.or.cz/stgit/kha.git safe

Karl Hasselström (1):
      Merge branch 'stable'

Miklos Vajna (2):
      setup.py: don't try to import stgit.run before the python version check
      setup.py: fix error message when running with python-2.3

 setup.py |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)


                                 -+-


The following changes since commit d8461a9dfb9a34e582019de5a67798fad44f42c7:
  Karl Hasselström (1):
        Merge branch 'stable'

are available in the git repository at:

  git://repo.or.cz/stgit/kha.git experimental

Karl Hasselström (21):
      Discard stderr output from git apply if the caller wants
      Do simple in-index merge with diff+apply instead of read-tree
      Reuse the same temp index in a transaction
      Library functions for tree and blob manipulation
      Write to a stack log when stack is modified
      Add utility function for reordering patches
      New command: stg reset
      Log conflicts separately
      Log conflicts separately for all commands
      Add a --hard flag to stg reset
      Don't write a log entry if there were no changes
      Move stack reset function to a shared location
      New command: stg undo
      New command: stg redo
      Log and undo external modifications
      Make "stg log" show stack log instead of patch log
      Convert "stg refresh" to the new infrastructure
      New refresh tests
      Remove --undo flags from stg commands and docs
      Refactor stgit.commands.edit
      Implement "stg refresh --edit" again

 Documentation/tutorial.txt   |    4 +-
 TODO                         |    2 -
 stgit/commands/branch.py     |   19 +-
 stgit/commands/clone.py      |    2 +-
 stgit/commands/coalesce.py   |    2 +-
 stgit/commands/common.py     |   18 +-
 stgit/commands/diff.py       |    6 +-
 stgit/commands/edit.py       |   82 +------
 stgit/commands/export.py     |    2 +-
 stgit/commands/files.py      |    6 +-
 stgit/commands/float.py      |    2 +-
 stgit/commands/fold.py       |    2 +-
 stgit/commands/goto.py       |    3 +-
 stgit/commands/hide.py       |    2 +-
 stgit/commands/id.py         |    2 +-
 stgit/commands/imprt.py      |    4 +-
 stgit/commands/log.py        |  169 ++++---------
 stgit/commands/mail.py       |    8 +-
 stgit/commands/new.py        |    3 +-
 stgit/commands/patches.py    |    2 +-
 stgit/commands/pick.py       |    2 +-
 stgit/commands/pop.py        |    4 +-
 stgit/commands/pull.py       |    2 +-
 stgit/commands/push.py       |   31 +--
 stgit/commands/rebase.py     |    4 +-
 stgit/commands/redo.py       |   52 ++++
 stgit/commands/refresh.py    |  338 +++++++++++++++++---------
 stgit/commands/rename.py     |    2 +-
 stgit/commands/repair.py     |   11 +-
 stgit/commands/reset.py      |   57 +++++
 stgit/commands/resolved.py   |    2 +-
 stgit/commands/show.py       |    2 +-
 stgit/commands/sink.py       |    2 +-
 stgit/commands/status.py     |    3 +-
 stgit/commands/sync.py       |   26 +--
 stgit/commands/undo.py       |   49 ++++
 stgit/commands/unhide.py     |    2 +-
 stgit/git.py                 |    4 -
 stgit/lib/edit.py            |   99 ++++++++
 stgit/lib/git.py             |  348 +++++++++++++++++++++++-----
 stgit/lib/log.py             |  537 ++++++++++++++++++++++++++++++++++++++++++
 stgit/lib/stack.py           |   25 ++
 stgit/lib/transaction.py     |  139 ++++++++---
 stgit/main.py                |    8 +
 stgit/stack.py               |   45 +----
 stgit/utils.py               |   18 +-
 t/t1200-push-modified.sh     |    2 +-
 t/t1201-pull-trailing.sh     |    2 +-
 t/t1202-push-undo.sh         |    8 +-
 t/t1400-patch-history.sh     |  103 --------
 t/t2300-refresh-subdir.sh    |   29 +++-
 t/t3100-reset.sh             |  151 ++++++++++++
 t/t3101-reset-hard.sh        |   56 +++++
 t/t3102-undo.sh              |   86 +++++++
 t/t3103-undo-hard.sh         |   56 +++++
 t/t3104-redo.sh              |  122 ++++++++++
 t/t3105-undo-external-mod.sh |   68 ++++++
 t/t3300-edit.sh              |    4 +-
 58 files changed, 2179 insertions(+), 660 deletions(-)
 create mode 100644 stgit/commands/redo.py
 create mode 100644 stgit/commands/reset.py
 create mode 100644 stgit/commands/undo.py
 create mode 100644 stgit/lib/edit.py
 create mode 100644 stgit/lib/log.py
 delete mode 100755 t/t1400-patch-history.sh
 create mode 100755 t/t3100-reset.sh
 create mode 100755 t/t3101-reset-hard.sh
 create mode 100755 t/t3102-undo.sh
 create mode 100755 t/t3103-undo-hard.sh
 create mode 100755 t/t3104-redo.sh
 create mode 100755 t/t3105-undo-external-mod.sh

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

^ permalink raw reply

* git-svn: add/obey a hooks/prepare-commit-msg
From: W Snyder @ 2008-07-15 19:10 UTC (permalink / raw)
  To: git


First, thanks for git-svn!

I'm trying to make a edit to the log message when pulling
from SVN into GIT using git-svn.

It's going bidirectionally, so I don't want to
git-filter-branch.

I tried creating a hooks/prepare-commit-msg script; it works
ok when manually doing a "git-commit" but seems to be
ignored by "git-svn fetch".  I made a hacked-up copy of
git-svn that edits the log message the way I want and it all
seems to work.

Can support for obeying commit-msg hooks be added to
git-svn?  Or, is there another way to do this?

Thanks!

^ permalink raw reply

* Re: [PATCH] git rev-parse: Fix --show-cdup inside symlinked   directory
From: Rogan Dawes @ 2008-07-15 19:08 UTC (permalink / raw)
  To: Yves Orton; +Cc: Petr Baudis, Johannes Schindelin, gitster, git
In-Reply-To: <1216141099.19334.196.camel@gemini>

Yves Orton wrote:

> Hmm, realizing that was the workdir it wanted i tried it like so:
> 
> [dmq@somewhere apps]$ git --work-tree="$(git-rev-parse --git-dir)/.."
> pull --rebase
> /usr/bin/git-sh-setup: line 139: cd: /home/dmq/git_tree/main/apps/.git:
> No such file or directory
> Unable to determine absolute path of git directory
> 
> Yet:
> 
> [dmq@somewhere apps]$ git-rev-parse --git-dir
> /home/dmq/git_tree/main/.git
> 
> is correct.
> 

Are you sure you don't want to specify the --git-dir rather than the 
work dir?

i.e.

git --git-dir="$(git-rev-parse --git-dir)" pull --rebase

Rogan

^ permalink raw reply

* Re: Closing the merge window for 1.6.0
From: Junio C Hamano @ 2008-07-15 18:51 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Dmitry Potapov, Petr Baudis, Nicolas Pitre, Gerrit Pape, git
In-Reply-To: <alpine.DEB.1.00.0807151623120.8950@racer>

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

> On Tue, 15 Jul 2008, Dmitry Potapov wrote:
>
>> Those repos that think that access for Git 1.4 users is important for 
>> them can set indexformat=1.
>
> Unfortunately, you place quite a high maintenance burden on the repository 
> maintainers here.
>
> From the time balance sheet, it does not look good at all: a few minutes 
> for Junio to change and commit, up to a few hours (because they missed it 
> in the release notes) for probably more than hundred repository 
> maintainers that are not subscribed to the Git mailing list.
>
> And I absolutely agree with Pasky that this does _nothing_ in the vague 
> direction of wielding a reputation of being easy to use.
>
> Sure, we can make it easy on ourselves.  And it is just as easy to make it 
> hard on others.  If you're okay with that, I am not.

I was not planning to comment on this issue further as the ball is in
Debian's court, but I think you are misguided.

We are not making anything hard on others.  Sticking to 1.4.4.4 codebase
is forced by Debian (for its policy) and choice made by its users (for not
knowing or using backports).  1.5.0 and later are vastly better and we
encourage users to update on every occassion we get.

I do not know the extent of the backporting effort necessary, the size of
potentially impacted population if Debian keeps shipping unpatched
1.4.4.4, nor how much Debian cares about supporting their 1.4.4.4 users
i.e. if they are willing and able to carry distro-only forward
compatibility patches, and knowing all of these is necessary before we
declare this is worth handling _ourselves_.  That is why I did not want to
take a definitive stance on this issue before hearing from the Debian
maintainer about them -- I said "Debian has to ask with list of items",
didn't I?

What troubles me the most is that you seem to be forgetting that we are
using git to manage our codebase.  Even if this turns out to be something
we would want to handle ourselves, it does not have to come from me.  If
you care that much, you could backport whatever change is appropriate to
keep 1.4.4.X codebase alive and arrange it to be published as 1.4.4.5.

In any case, it will _definitely_ *NOT* a few minutes of me nor anybody.
Release engineering takes quite a lot of time.

^ permalink raw reply

* Re: [PATCH v2] index-pack: Honor core.deltaBaseCacheLimit when resolving deltas
From: Junio C Hamano @ 2008-07-15 18:48 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Nicolas Pitre, Stephan Hennig, Andreas Ericsson
In-Reply-To: <20080715044534.GA2794@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

>  Version 2 plugs the case Nico noticed, where the patch was causing
>  the exact behavior it was trying to prevent while recovering from
>  what it did to avoid the excessive memory usage in the first place.

Thanks both; it makes sense.

^ permalink raw reply

* Re: [PATCH] Documentation/git-submodule.txt: Add Description section
From: Heikki Orsila @ 2008-07-15 18:37 UTC (permalink / raw)
  To: Petr Baudis; +Cc: gitster, git
In-Reply-To: <20080715102119.26321.78530.stgit@localhost>

On Tue, Jul 15, 2008 at 12:22:07PM +0200, Petr Baudis wrote:
> +Submodules are a special kind of tree entries which do not refer to a blob or
> +a directory, but to a particular tree in another repository (living at a given
> +URL). 

Better to say what a submodule is, rather than what it isn't:

"Submodules are a special kind of tree entries which refer to a 
particular tree in another repository ..."

Also, I think you should make the following explicit:

"A submodule is visible as subdirectory in the working directory.
However, the submodule is not part of the main repository.
This is a differene to "remotes". In remotes only the contents of 
other repositories is tracked, but their content is not visible in the
working directory."

-- 
Heikki Orsila
heikki.orsila@iki.fi
http://www.iki.fi/shd

^ permalink raw reply

* Re: [PATCH] Only use GIT_CONFIG in "git config", not other programs
From: Sverre Rabbelier @ 2008-07-15 17:56 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: Junio C Hamano, git, Johannes Schindelin
In-Reply-To: <alpine.LNX.1.00.0806300328380.19665@iabervon.org>

On Mon, Jun 30, 2008 at 9:37 AM, Daniel Barkalow <barkalow@iabervon.org> wrote:
> @@ -611,31 +613,28 @@ int git_config(config_fn_t fn, void *data)
>  {
>        int ret = 0;
>        char *repo_config = NULL;
> -       const char *home = NULL, *filename;
> +       const char *home = NULL;
>
>        /* $GIT_CONFIG makes git read _only_ the given config file,
>         * $GIT_CONFIG_LOCAL will make it process it in addition to the
>         * global config file, the same way it would the per-repository
>         * config file otherwise. */

I noticed today while looking at config.c (because of the 'git config
oddity' thread) that this reference to GIT_CONFIG_LOCAL was not
removed? On latest next:
$ grep GIT_CONFIG_LOCAL *
config.c:	 * $GIT_CONFIG_LOCAL will make it process it in addition to the
git-svn:		my $file = $ENV{GIT_CONFIG} || $ENV{GIT_CONFIG_LOCAL} ||
git-svn.perl:		my $file = $ENV{GIT_CONFIG} || $ENV{GIT_CONFIG_LOCAL} ||

I doubt that this stray comment will hurt anyone, but it's probably
nicer not to let cruft like this accumulate, I think?

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: Closing the merge window for 1.6.0
From: Petr Baudis @ 2008-07-15 17:28 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Johannes Schindelin, Dmitry Potapov, Junio C Hamano, Gerrit Pape,
	git
In-Reply-To: <alpine.LFD.1.10.0807151213000.12484@xanadu.home>

On Tue, Jul 15, 2008 at 12:26:48PM -0400, Nicolas Pitre wrote:
> Anyway this is all hand waving until someone can come with some evidence 
> that git 1.4.4 is actually used by a significant amount of people, and 
> that those people depend on dumb transfer protocols.

That will be hard to produce. :-) _My_ personal story is that I have
Git-1.4.4.4 installed system-wide on repo.or.cz and follow git#next
locally, and quite panicked when I was inspecting some repositories
as root (using the system-wide Git) and these error messages popped up.
This may become a similar experience for others on multi-user systems
where people want to share work but don't realize that one of them has
Git installed locally and the other one doesn't. We can save them the
head-slapping and a bit of wasted life.

Out of interest, I did a simple statistics of HTTP user agents on
repo.or.cz; the dumb access does not seem very widely used overally,
it turns out. The stats begin at 19/May/2008:10:54:32 +0200. Here is the
breakdown, counting unique clients only:

# zgrep '"GET /r/.*/info/packs' /var/log/apache2/repo-access.log* | egrep -v bot\|slurp\|Gecko\|Opera |
	cut -d " " -f 1,12- | sed 's/\.g[a-f0-9]*\(\.dirty\)*"/"/' | sort -u |
	cut -d ' ' -f 2 | sort | uniq -c | sort -rn | head -n 50
   1501 "git/1.5.4.3"	<- Ubuntu Hardy (heh.. is just that it?)
    278 "git/1.5.5.1"	<- RHEL5 (ditto)
    151 "git/1.5.2.5"	<- Ubuntu Gutsy
    133 "git/1.5.5.3"	<- ? (maybe Gentoo ~x86 for some time)
    125 "git/1.5.4.5"	<- OpenSUSE 11.0, FC9, Gentoo x86, Dapper backports
    104 "git/1.5.6"	<- Debian Lenny
     94 "git/1.5.5"
     66 "git/1.5.3.7"
     63 "git/1.5.5.4"
     63 "git/1.5.5.1015"
     55 "git/1.5.2.4"	<- OpenSUSE 10.3
     51 "Mozilla/4.0 (compatible;)"	<- huh?
     42 "git/1.5.3.8"
     37 "git/1.5.5.GIT"
     37 "git/1.5.3.5.2229"
     34 "git/1.5.6.1"
     33 "git/1.5.3.6"	<- Feisty backports
     31 "git/1.5.4.1"
     30 "git/1.5.6.2"
     20 "git/1.5.6.GIT"
     18 "git/1.5.3"
     17 "git/1.5.2.2"
     17 "git/1.4.4.4"
     15 "git/1.5.6.1.1071"
     14 "git/1.5.3.3"
     13 "git/1.5.4.4"
     13 "git/1.5.4"
     11 "git/1.5.6.1062"
     11 "git/1.5.5.2"
     10 "git/1.5.5.1.316"

(I also got two 1.4.4.2 (feisty?) fetches from one client. No older
Git versions.)

So wrt. keeping backwards compatibility, this is not _very_ convincing,
I admit. ;-)

-- 
				Petr "Pasky" Baudis
GNU, n. An animal of South Africa, which in its domesticated state
resembles a horse, a buffalo and a stag. In its wild condition it is
something like a thunderbolt, an earthquake and a cyclone. -- A. Pierce

^ permalink raw reply

* Re: Closing the merge window for 1.6.0
From: Dmitry Potapov @ 2008-07-15 17:04 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Petr Baudis, Junio C Hamano, Nicolas Pitre, Gerrit Pape, git
In-Reply-To: <alpine.DEB.1.00.0807151623120.8950@racer>

On Tue, Jul 15, 2008 at 04:27:02PM +0100, Johannes Schindelin wrote:
> 
> >From the time balance sheet, it does not look good at all: a few minutes 
> for Junio to change and commit, up to a few hours (because they missed it 
> in the release notes) for probably more than hundred repository 
> maintainers that are not subscribed to the Git mailing list.

If you just grab sources and never read release notes, there is nothing
that can help you. If Git 1.6.0 is not the right moment to do these
changes then Git 1.6.1 is neither, regardless whether Debian will
release Lenny by that time or not. People do not upgrade their distro in
the day of release. Some upgraded to Etch not so long ago. So, should we
wait for another year till 1.7.0?

> 
> And I absolutely agree with Pasky that this does _nothing_ in the vague 
> direction of wielding a reputation of being easy to use.

I don't think Git 1.4 is easy to use. If you want Git that is easy to
use install Git 1.5.x. And, it is *much* easier to install Git from
backports then to deal with usability issues of Git 1.4 and the lack
of community support.  So, I don't see how this change may hurt.

> 
> Sure, we can make it easy on ourselves.  And it is just as easy to make it 
> hard on others.  If you're okay with that, I am not.

It has *nothing* to do with making easy on ourselves and hard on others.
The question here is what is the appropriate time to change these default
settings, and I believe that *major* releases are the appropriate time
while minor ones are not.


Dmitry

^ permalink raw reply

* Re: [PATCH] git rev-parse: Fix --show-cdup inside symlinked directory
From: Yves Orton @ 2008-07-15 16:58 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Johannes Schindelin, gitster, git
In-Reply-To: <1216140100.19334.189.camel@gemini>

On Tue, 2008-07-15 at 18:41 +0200, Yves Orton wrote:
> On Tue, 2008-07-15 at 17:40 +0200, Petr Baudis wrote:
> > 
> >   P.S.: Either way, there is a possible workaround to tell git about the
> > working directory manually using git --work-tree=... that I missed to
> > mention on IRC, Yves.
> 
> Hmm, am i using it wrong then?
> 
> [dmq@somewhere apps]$ git-rev-parse --git-dir
> /home/dmq/git_tree/main/.git
> [dmq@somewhere apps]$ git --work-tree="$(git-rev-parse --git-dir)" pull
> --rebase
> /usr/bin/git-sh-setup: line 139: cd: .git: No such file or directory
> Unable to determine absolute path of git directory

Hmm, realizing that was the workdir it wanted i tried it like so:

[dmq@somewhere apps]$ git --work-tree="$(git-rev-parse --git-dir)/.."
pull --rebase
/usr/bin/git-sh-setup: line 139: cd: /home/dmq/git_tree/main/apps/.git:
No such file or directory
Unable to determine absolute path of git directory

Yet:

[dmq@somewhere apps]$ git-rev-parse --git-dir
/home/dmq/git_tree/main/.git

is correct.

> cheers,
> yves
> ps: not on list, please cc me on replies (sorry for the hassle)
> 

^ permalink raw reply

* [PATCH] cherry: cache patch-ids to avoid repeating work
From: Geoffrey Irving @ 2008-07-15 16:57 UTC (permalink / raw)
  To: Junio C Hamano, git@vger.kernel.org; +Cc: Johannes Schindelin
In-Reply-To: <7f9d599f0807122014y5190463j62d106a01bf31c86@mail.gmail.com>

Add cached-sha-map.[ch] implementing a persistent hash map from sha1 to
sha1.  The map is read with mmap, and completely rewritten if any entries
change.  It would be good to add incremental update to handle the usual case
where only a few entries change.

This structure is used by patch-ids.c to cache the mapping from commit to
patch-id into $GIT_DIR/patch-id-cache.  In the one case I've tested so far,
this speeds up the second invocation of git-cherry by two orders of
magnitude.  The caching can be disabled by setting cherry.cachepatchids to
false.  Only patch-ids with default diff options are cached.

Original code cannibalized from Johannes Schindelin's notes-index structure.

Signed-off-by: Geoffrey Irving <irving@naml.us>
---

Here's a version of the patch fixing the test breakage.  Only
patch-ids.c differs from the previous version.

 Documentation/config.txt |    5 +
 Makefile                 |    2 +
 builtin-log.c            |   12 ++
 cached-sha1-map.c        |  293 ++++++++++++++++++++++++++++++++++++++++++++++
 cached-sha1-map.h        |   45 +++++++
 patch-ids.c              |   40 ++++++-
 patch-ids.h              |    2 +
 7 files changed, 398 insertions(+), 1 deletions(-)
 create mode 100644 cached-sha1-map.c
 create mode 100644 cached-sha1-map.h

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 838794d..02b8113 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -468,6 +468,11 @@ browser.<tool>.path::
 	browse HTML help (see '-w' option in linkgit:git-help[1]) or a
 	working repository in gitweb (see linkgit:git-instaweb[1]).

+cherry.cachepatchids::
+	If true, linkgit:git-cherry will store a cache of computed patch-ids
+	in $GIT_DIR/patch-id-cache in order to make repeated invocations faster.
+	Defaults to true.
+
 clean.requireForce::
 	A boolean to make git-clean do nothing unless given -f
 	or -n.   Defaults to true.
diff --git a/Makefile b/Makefile
index 4796565..f7360e1 100644
--- a/Makefile
+++ b/Makefile
@@ -356,6 +356,7 @@ LIB_H += pack-refs.h
 LIB_H += pack-revindex.h
 LIB_H += parse-options.h
 LIB_H += patch-ids.h
+LIB_H += cached-sha1-map.h
 LIB_H += path-list.h
 LIB_H += pkt-line.h
 LIB_H += progress.h
@@ -436,6 +437,7 @@ LIB_OBJS += pager.o
 LIB_OBJS += parse-options.o
 LIB_OBJS += patch-delta.o
 LIB_OBJS += patch-ids.o
+LIB_OBJS += cached-sha1-map.o
 LIB_OBJS += path-list.o
 LIB_OBJS += path.o
 LIB_OBJS += pkt-line.o
diff --git a/builtin-log.c b/builtin-log.c
index 430d876..fbfefbd 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -1081,6 +1081,16 @@ static int add_pending_commit(const char *arg,
struct rev_info *revs, int flags)
 	return -1;
 }

+static int git_cherry_config(const char *var, const char *value, void *cb)
+{
+	if (!strcmp(var, "cherry.cachepatchids")) {
+		cache_patch_ids = git_config_bool(var, value);
+		return 0;
+	}
+
+	return 0;
+}
+
 static const char cherry_usage[] =
 "git-cherry [-v] <upstream> [<head>] [<limit>]";
 int cmd_cherry(int argc, const char **argv, const char *prefix)
@@ -1094,6 +1104,8 @@ int cmd_cherry(int argc, const char **argv,
const char *prefix)
 	const char *limit = NULL;
 	int verbose = 0;

+	git_config(git_cherry_config, NULL);
+
 	if (argc > 1 && !strcmp(argv[1], "-v")) {
 		verbose = 1;
 		argc--;
diff --git a/cached-sha1-map.c b/cached-sha1-map.c
new file mode 100644
index 0000000..9cf7252
--- /dev/null
+++ b/cached-sha1-map.c
@@ -0,0 +1,293 @@
+#include "cached-sha1-map.h"
+
+union cached_sha1_map_header {
+	struct {
+		char signature[4]; /* CS1M */
+		uint32_t version;
+		uint32_t count;
+		uint32_t size;
+		uint32_t pad; /* pad to 20 bytes */
+	} u;
+	/* pad header out to 40 bytes.  As a consistency
+	 * check, pad.value stores the sha1 of pad.key. */
+	struct cached_sha1_entry pad;
+};
+
+static const char *signature = "CS1M";
+static const uint32_t version = 1;
+
+static int init_empty_map(struct cached_sha1_map *cache, uint32_t size)
+{
+	cache->count = 0;
+	cache->size = size;
+	cache->initialized = 1;
+	cache->mmapped = 0;
+	cache->dirty = 1;
+
+	cache->entries = calloc(size, sizeof(struct cached_sha1_entry));
+	if (!cache->entries) {
+		warning("failed to allocate empty map of size %"PRIu32" for %s",
+			size, git_path(cache->filename));
+		cache->size = 0;
+		cache->dirty = 0;
+		return -1;
+	}
+	return 0;
+}
+
+static int grow_map(struct cached_sha1_map *cache)
+{
+	struct cached_sha1_map new_cache;
+	uint32_t i;
+
+	if (cache->size * 2 == 0) {
+		warning("%s overflowed, so resetting to empty",
+			git_path(cache->filename));
+		return init_empty_map(cache, 64);
+	}
+
+	/* allocate cache with twice the size */
+	new_cache.filename = cache->filename;
+	if (init_empty_map(&new_cache, cache->size * 2)) {
+		warning("failed to grow %s to size %"PRIu32,
+			git_path(cache->filename), cache->size * 2);
+		return init_empty_map(cache, 64);
+	}
+
+	/* reinsert all entries */
+ 	for (i = 0; i < cache->size; i++)
+		if (!is_null_sha1(cache->entries[i].key))
+			set_cached_sha1_entry(&new_cache,
+				cache->entries[i].key, cache->entries[i].value);
+	/* finish */
+	free_cached_sha1_map(cache);
+	*cache = new_cache;
+	return 0;
+}
+
+/* Any errors that occur result in the cache being initialized to empty */
+static int init_cached_sha1_map(struct cached_sha1_map *cache)
+{
+	int fd;
+	union cached_sha1_map_header header;
+	const char *filename;
+	size_t map_size;
+	SHA_CTX ctx;
+
+	if (cache->initialized)
+		return cache->size ? 0 : -1;
+
+	filename = git_path(cache->filename);
+	fd = open(filename, O_RDONLY);
+	if (fd < 0) {
+		if (errno != ENOENT)
+			warning("failed to read '%s': %s", filename,
+				strerror(errno));
+		goto empty;
+	}
+
+	if (read_in_full(fd, &header, sizeof(header)) != sizeof(header)) {
+		warning("cannot read %s header", filename);
+		goto empty;
+	}
+
+	if (memcmp(header.u.signature, signature, 4)) {
+		warning("%s has invalid header", filename);
+		goto empty;
+	}
+
+	if (ntohl(header.u.version) != version) {
+		warning("%s has unrecognized version %"PRIu32, filename,
+			ntohl(header.u.version));
+		goto empty;
+	}
+
+	cache->count = ntohl(header.u.count);
+	cache->size = ntohl(header.u.size);
+
+	if (cache->size & (cache->size-1)) {
+		warning("%s is corrupt: size %"PRIu32" is not a power of two",
+			filename, cache->size);
+		goto empty;
+	}
+
+	if (cache->count >= cache->size) {
+		warning("%s is corrupt: count %"PRIu32" >= size %"PRIu32,
+			filename, cache->count, cache->size);
+		goto empty;
+	}
+
+	SHA1_Init(&ctx);
+	SHA1_Update(&ctx, header.pad.key, 20);
+	SHA1_Final(header.pad.key, &ctx); /* reuse pad.key to store its sha1 */
+	if (hashcmp(header.pad.key, header.pad.value)) {
+		warning("%s header has invalid sha1", filename);
+		goto empty;
+	}
+
+	cache->dirty = 0;
+	cache->initialized = 1;
+	cache->mmapped = 1;
+
+	/* mmap entire file so that file / memory blocks are aligned */
+	map_size = sizeof(struct cached_sha1_entry) * (cache->size + 1);
+	cache->entries = mmap(NULL, map_size,
+		PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
+	if (cache->entries == MAP_FAILED) {
+		/* this is just a cache, so don't free pack memory and retry */
+		warning("%s mmap failed: %s", filename, strerror(errno));
+		goto empty;
+	}
+	cache->entries += 1; /* skip header */
+	return 0;
+
+empty:
+	if (fd >= 0)
+		close(fd);
+	return init_empty_map(cache, 64);
+}
+
+int write_cached_sha1_map(struct cached_sha1_map *cache)
+{
+	union cached_sha1_map_header header;
+	struct lock_file update_lock;
+	int fd;
+	size_t map_size;
+	const char *filename;
+	SHA_CTX ctx;
+
+	if (!cache->initialized || !cache->dirty)
+		return 0;
+
+	filename = git_path(cache->filename);
+	fd = hold_lock_file_for_update(&update_lock, filename, 0);
+
+	if (fd < 0)
+	{
+		warning("could not construct %s", filename);
+		return -1;
+	}
+
+	/* initialize header */
+	memcpy(header.u.signature, signature, 4);
+	header.u.version = htonl(version);
+	header.u.count = htonl(cache->count);
+	header.u.size = htonl(cache->size);
+	header.u.pad = 0; /* make header deterministic */
+
+	/* compute header sha1 */
+	SHA1_Init(&ctx);
+	SHA1_Update(&ctx, header.pad.key, 20);
+	SHA1_Final(header.pad.value, &ctx);
+
+	map_size = sizeof(struct cached_sha1_entry) * cache->size;
+	if (write_in_full(fd, &header, sizeof(header)) != sizeof(header)
+		|| write_in_full(fd, cache->entries, map_size) != map_size)
+	{
+		warning("could not write %s", filename);
+		return -1;
+	}
+
+	if (commit_lock_file(&update_lock) < 0)
+	{
+		warning("could not write %s", filename);
+		return -1;
+	}
+
+	cache->dirty = 0;
+	return 0;
+}
+
+void free_cached_sha1_map(struct cached_sha1_map *cache)
+{
+	if (!cache->initialized)
+		return;
+
+	if (cache->mmapped)
+		munmap(cache->entries - 1,
+			sizeof(struct cached_sha1_entry) * (cache->size + 1));
+	else
+		free(cache->entries);
+}
+
+/* The fact that size is a power of two means count-1 <= INT32_MAX, so it
+ * is safe to return signed integers here. */
+static int32_t get_hash_index(const unsigned char *sha1)
+{
+	/* this is alignment safe since 40 is a multiple of 4 */
+	return ntohl(*(uint32_t*)sha1);
+}
+
+/*
+ * Returns the index if the entry exists, and the complemented index of
+ * the next free entry otherwise.  If the hash is full, returns the
+ * complement of a nonfree entry and sets count = size (this happens
+ * only if the file is corrupt).
+ */
+static int32_t find_helper(struct cached_sha1_map *cache,
+	const unsigned char *key)
+{
+	int32_t i, mask, full;
+
+	mask = cache->size - 1;
+	i = get_hash_index(key) & mask;
+	full = (i-1) & mask;
+
+	for (; ; i = (i+1) & mask) {
+		if (!hashcmp(key, cache->entries[i].key))
+			return i;
+		else if (is_null_sha1(cache->entries[i].key) || i == full)
+			return ~i;
+		if (i == full) {
+			cache->count = cache->size; /* fix count */
+			return ~1;
+		}
+	}
+}
+
+int get_cached_sha1_entry(struct cached_sha1_map *cache,
+	const unsigned char *key, unsigned char *value)
+{
+	int32_t i;
+
+	if (init_cached_sha1_map(cache))
+		return -1;
+
+	i = find_helper(cache, key);
+	if(i < 0)
+		return -1;
+
+	/* entry found, return value */
+	hashcpy(value, cache->entries[i].value);
+	return 0;
+}
+
+int set_cached_sha1_entry(struct cached_sha1_map *cache,
+	const unsigned char *key, const unsigned char *value)
+{
+	int32_t i;
+	struct cached_sha1_entry *entry;
+
+	if (init_cached_sha1_map(cache))
+		return -1;
+
+	i = find_helper(cache, key);
+
+	if (i < 0) { /* write new entry */
+		entry = cache->entries + ~i;
+		hashcpy(entry->key, key);
+		hashcpy(entry->value, value);
+		cache->count++;
+		cache->dirty = 1;
+	} else { /* overwrite existing entry */
+		entry = cache->entries + i;
+		if (hashcmp(value, entry->value)) {
+			hashcpy(entry->value, value);
+			cache->dirty = 1;
+		}
+	}
+
+	if (cache->count >= cache->size/4*3)
+		return grow_map(cache);
+	return 0;
+}
diff --git a/cached-sha1-map.h b/cached-sha1-map.h
new file mode 100644
index 0000000..296c17c
--- /dev/null
+++ b/cached-sha1-map.h
@@ -0,0 +1,45 @@
+#ifndef CACHED_SHA1_MAP_H
+#define CACHED_SHA1_MAP_H
+
+#include "cache.h"
+
+/*
+ * A cached-sha1-map is a file storing a hash map from sha1 to sha1.
+ *
+ * The file is mmap'ed, updated in memory during operation, and flushed
+ * back to disk when freed.  Currently the entire file is rewritten for
+ * any change.  This could be a significant bottleneck for common uses,
+ * so it would be good to fix this later if possible.
+ *
+ * The performance of a hash map depends highly on a good hashing
+ * algorithm, to avoid collisions.  Lucky us!  SHA-1 is a pretty good
+ * hashing algorithm.
+ */
+
+struct cached_sha1_entry {
+	unsigned char key[20];
+	unsigned char value[20];
+};
+
+struct cached_sha1_map {
+	const char *filename; /* relative to GIT_DIR */
+
+	/* rest is for internal use */
+	uint32_t count, size;
+	unsigned int initialized : 1;
+	unsigned int dirty : 1;
+	unsigned int mmapped : 1;
+	struct cached_sha1_entry *entries; /* pointer to mmap'ed memory + 1 */
+};
+
+extern int get_cached_sha1_entry(struct cached_sha1_map *cache,
+	const unsigned char *key,unsigned char *value);
+
+extern int set_cached_sha1_entry(struct cached_sha1_map *cache,
+	const unsigned char *key, const unsigned char *value);
+
+extern int write_cached_sha1_map(struct cached_sha1_map *cache);
+
+extern void free_cached_sha1_map(struct cached_sha1_map *cache);
+
+#endif
diff --git a/patch-ids.c b/patch-ids.c
index 3be5d31..5ba12fb 100644
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -2,17 +2,49 @@
 #include "diff.h"
 #include "commit.h"
 #include "patch-ids.h"
+#include "cached-sha1-map.h"
+
+int cache_patch_ids = 1;
+static struct cached_sha1_map patch_id_cache;
+
+static struct diff_options default_options;
+#define IGNORED_DIFF_OPTS (DIFF_OPT_HAS_CHANGES | DIFF_OPT_CHECK_FAILED)

 static int commit_patch_id(struct commit *commit, struct diff_options *options,
 		    unsigned char *sha1)
 {
+	int use_cache = 0;
+	int ret;
+
+	/* only cache if diff options are defaults */
+	if (cache_patch_ids) {
+		default_options.found_changes = options->found_changes;
+		default_options.flags = (options->flags & IGNORED_DIFF_OPTS)
+			| (default_options.flags & ~IGNORED_DIFF_OPTS);
+		use_cache = !memcmp(options, &default_options,
+				    sizeof(struct diff_options));
+	}
+
+	/* pull patch-id out of the cache if possible */
+	patch_id_cache.filename = "patch-id-cache";
+	if (use_cache && !get_cached_sha1_entry(&patch_id_cache,
+			commit->object.sha1, sha1))
+		return 0;
+
 	if (commit->parents)
 		diff_tree_sha1(commit->parents->item->object.sha1,
 		               commit->object.sha1, "", options);
 	else
 		diff_root_tree_sha1(commit->object.sha1, "", options);
 	diffcore_std(options);
-	return diff_flush_patch_id(options, sha1);
+	ret = diff_flush_patch_id(options, sha1);
+	if (ret)
+		return ret;
+
+	/* record commit, patch-id pair in cache */
+	if (use_cache)
+		set_cached_sha1_entry(&patch_id_cache, commit->object.sha1, sha1);
+	return 0;
 }

 static uint32_t take2(const unsigned char *id)
@@ -124,6 +156,7 @@ int init_patch_ids(struct patch_ids *ids)
 	DIFF_OPT_SET(&ids->diffopts, RECURSIVE);
 	if (diff_setup_done(&ids->diffopts) < 0)
 		return error("diff_setup_done failed");
+	default_options = ids->diffopts; /* remember defaults */
 	return 0;
 }

@@ -136,6 +169,11 @@ int free_patch_ids(struct patch_ids *ids)
 		next = patches->next;
 		free(patches);
 	}
+
+	/* write cached patch-ids and ignore any errors that arise
+	 * (e.g. if the repository is write protected) */
+	if (cache_patch_ids)
+		write_cached_sha1_map(&patch_id_cache);
 	return 0;
 }

diff --git a/patch-ids.h b/patch-ids.h
index c8c7ca1..c0ebdc1 100644
--- a/patch-ids.h
+++ b/patch-ids.h
@@ -18,4 +18,6 @@ int free_patch_ids(struct patch_ids *);
 struct patch_id *add_commit_patch_id(struct commit *, struct patch_ids *);
 struct patch_id *has_commit_patch_id(struct commit *, struct patch_ids *);

+extern int cache_patch_ids;
+
 #endif /* PATCH_IDS_H */
-- 
1.5.6.2.256.g3ef05.dirty

^ permalink raw reply related

* Re: What's cooking in git.git (topics)
From: Geoffrey Irving @ 2008-07-15 16:48 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0807151117450.3487@eeepc-johanness>

On Tue, Jul 15, 2008 at 2:22 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Mon, 14 Jul 2008, Geoffrey Irving wrote:
>
>> The problem (beyond the basic problem of me not having tried running the
>> tests) is that the current caching code isn't taking into account the
>> changing values of diff_options.  t6007 computes a patch-id for a commit
>> with one value of options.paths, and then tries to compute a _different_
>> patch-id for the same commit using a different value of options.paths.
>>
>> Here are a few different ways of fixing this:
>>
>> 1. Modify commit_patch_id in patch-ids.c to compute a sha1 of the
>>    diff_options structure and xor it with the commit sha1 to get a truly
>>    unique hash of the input.  This means the optimization can be safely
>>    applied for all patch-id computations regardless of the diff_options.
>>    I can add a diff_options_sha1 function in diff.[ch] to compute the
>>    checksum.
>>
>> 2. Restrict commit_patch_id in patch-ids.c to apply the optimization
>>    only if options.nr_paths is zero, and perhaps a few other conditions.
>>    This is rather fragile, since it would mean that the cache would
>>    break if someone decided to change the default diff options.
>
> Funnily, (2) contradicts (1).  The patch id is _different_ when you have
> nr_paths > 0.  At least in the general case.
>
> So what you propose in (1) will not work, unless you also hash the path
> names (in the correct order, otherwise you'll end up with two hashes).

The sha1 would include paths, of course, since it's part of diff_options.

> OTOH I would be really surprised if you needed --cherry-pick with paths
> and/or diff options more than once for the same commits.  So the caching
> does not make sense to begin with (especially since we do not have a
> proper way of gc'ing it, right?).
>
> So I'd suggest saving diff_opts before the command line parsing, and
> disable the cache when it is different _and/or_ (||) nr_paths.

I'll attach the patch to the other thread.

Geoffrey

^ permalink raw reply

* Re: Closing the merge window for 1.6.0
From: Sverre Rabbelier @ 2008-07-15 16:46 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Johannes Schindelin, Dmitry Potapov, Petr Baudis, Junio C Hamano,
	Gerrit Pape, git
In-Reply-To: <alpine.LFD.1.10.0807151213000.12484@xanadu.home>

On Tue, Jul 15, 2008 at 6:26 PM, Nicolas Pitre <nico@cam.org> wrote:
> Anyway this is all hand waving until someone can come with some evidence
> that git 1.4.4 is actually used by a significant amount of people, and
> that those people depend on dumb transfer protocols.

Can't we add a msg to 1.4.4.x when it finds pack version 2 to upgrade
to 1.5.x? Gets rid of the problem all together while still giving the
user a reasonable message when it finds a repo version 2.
Hey, here's an idea, can't we have 1.4.4.x just give that msg for everything?

$cat git
#!/bin/sh

echo "Please upgrade to 1.5.x, version 1.4.x is no longer supported
nor should you even want to use it </cluebat>"

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH] git rev-parse: Fix --show-cdup inside symlinked directory
From: Yves Orton @ 2008-07-15 16:41 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Johannes Schindelin, gitster, git
In-Reply-To: <20080715154036.GR10151@machine.or.cz>

On Tue, 2008-07-15 at 17:40 +0200, Petr Baudis wrote:
>   no, no, this is for the scenario other way around: you have a normal
> subdirectory in the working tree, and point a symlink _at_ it from
> $somewhere_else. Then you try to work in $somewhere_else/symlink.

Yes correct. We have a number of different repositories like so:

banana.git/apps
banana.git/lib
orange.git/config
kiwi.git/refdata

and its convenient for many of our existing apps to be able to symlink
them all together into a common tree

joined/apps -> banana.git/apps
joined/lib -> banana.git/lib
joined/config -> orange.git/config
joined/refdata -> kiwi.git/refdata

this way for instance we can swap bits around easily on the fly and say,
restart a webserver or whatever.

Currently we can do this and all our other stuff works, and you
can /mostly/ work with git from the "joined" tree, with the exception of
git pull --rebase and apparently anything else that relies on
--show-cdup

> > > This patch changes --show-cdup to always show absolute workdir path
> > > instead. I think this should hopefully cause no compatibility problems;
> > > the testsuite is passing fine, at least.
> > 
> > See the thread where I proposed a change like this, back with the infamous 
> > worktree desaster, and Junio NACKed; or the thread where Linus rightfully 
> > insists that git_dir should be relative if possible, for performance 
> > reasons.
> 
>   I see, <7vk5sly3h9.fsf@assigned-by-dhcp.cox.net>. But noone was aware
> of this possible user case. Performance reasons sound reasonable, though
> I'm not really sure if for cdup in particular this ever matters.

Would it be so bad to detect if the show-cdup actually resolves to the
right place, and if it doesnt go absolute?

> 
>   P.S.: Either way, there is a possible workaround to tell git about the
> working directory manually using git --work-tree=... that I missed to
> mention on IRC, Yves.

Hmm, am i using it wrong then?

[dmq@somewhere apps]$ git-rev-parse --git-dir
/home/dmq/git_tree/main/.git
[dmq@somewhere apps]$ git --work-tree="$(git-rev-parse --git-dir)" pull
--rebase
/usr/bin/git-sh-setup: line 139: cd: .git: No such file or directory
Unable to determine absolute path of git directory

cheers,
yves
ps: not on list, please cc me on replies (sorry for the hassle)

^ 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