Git development
 help / color / mirror / Atom feed
* Re: [PATCH] rebase -i: only automatically amend commit if HEAD did not change
From: Junio C Hamano @ 2008-07-22 23:55 UTC (permalink / raw)
  To: Avery Pennarun; +Cc: Johannes Schindelin, git, gitster
In-Reply-To: <32541b130807221522r2a43c49cl6400f00dbe7451a0@mail.gmail.com>

"Avery Pennarun" <apenwarr@gmail.com> writes:

> On 7/22/08, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>>  If the user called "rebase -i", marked a commit as "edit", "rebase
>>  --continue" would automatically amend the commit when there were
>>  staged changes.
>>
>>  However, this is actively wrong when the current commit is not the
>>  one marked with "edit".  So guard against this.
>
> This patch is perhaps a symptom of something I've been meaning to ask
> about for a while.
>
> Why doesn't "edit" just stage the commit and not auto-commit it at
> all?  That way an amend would *never* be necessary, and rebase
> --continue would always do a commit -a (if there was anything left to
> commit).  The special case fixed by this patch would thus not be
> needed.

That would actually be in-line with the way how "rebase --skip" does the
resetting without asking the user to do so, wouldn't it?

^ permalink raw reply

* Re: [PATCH] Rename ".dotest/" to ".git/rebase" and ".dotest-merge" to "rebase-merge"
From: Stephan Beyer @ 2008-07-22 23:47 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Olivier Marin, Theodore Tso, Nanako Shiraishi,
	Johannes Schindelin, René Scharfe, Joe Fiorini, git,
	Jari Aalto
In-Reply-To: <7v3am5zfea.fsf@gitster.siamese.dyndns.org>

Hi,

Junio C Hamano wrote:
> Olivier Marin <dkr+ml.git@free.fr> writes:
> > @@ -203,9 +204,10 @@ then
> >  
> >  	case "$abort" in
> >  	t)
> > -		rm -fr "$dotest" &&
> > +		git rerere clear &&
> >  		git read-tree -m -u ORIG_HEAD &&
[...]
> diff --git a/git-am.sh b/git-am.sh
> index a44bd7a..5cbf8f4 100755
> --- a/git-am.sh
> +++ b/git-am.sh
> @@ -203,9 +203,9 @@ then
>  
>  	case "$abort" in
>  	t)
> -		rm -fr "$dotest" &&
> -		git read-tree -m -u ORIG_HEAD &&
> -		git reset ORIG_HEAD && :
> +		git rerere clear
> +		git read-tree --reset -u HEAD ORIG_HEAD

Perhaps I am confused, but ...
Why is there "HEAD" and "ORIG_HEAD" and not only "ORIG_HEAD"?

Regards.

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

^ permalink raw reply

* Re: regression in  92392b4
From: Pierre Habouzit @ 2008-07-22 23:37 UTC (permalink / raw)
  To: spearce, Git ML, Junio C Hamano
In-Reply-To: <20080722231745.GD11831@artemis.madism.org>

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

On Tue, Jul 22, 2008 at 11:17:45PM +0000, Pierre Habouzit wrote:
>   Hi, here is a manual painful down-secting (opposed to a bisect ;P) I
> did, since git in next cannot fetch on a regular basis for me. The
> culprit seems to be commit  92392b4:
> 
>     ┌─(1:11)──<~/dev/scm/git 92392b4...>──
>     └[artemis] git fetch
>     remote: Counting objects: 461, done.
>     remote: Compressing objects: 100% (141/141), done.
>     remote: Total 263 (delta 227), reused 155 (delta 121)
>     Receiving objects: 100% (263/263), 95.55 KiB, done.
>     fatal: Out of memory, malloc failed
>     fatal: index-pack failed
>     [2]    16674 abort (core dumped)  git fetch
> 
>     ┌─(1:12)──<~/dev/scm/git 92392b4...>──
>     └[artemis] git checkout -m HEAD~1; make git-index-pack
>     Previous HEAD position was 92392b4... index-pack: Honor core.deltaBaseCacheLimit when resolving deltas
>     HEAD is now at 03993e1... index-pack: Track the object_entry that creates each base_data
>     GIT_VERSION = 1.5.6.3.3.g03993
> 	CC index-pack.o
> 	LINK git-index-pack
> 
>     ┌─(1:12)──<~/dev/scm/git 03993e1...>──
>     └[artemis] git fetch
>     remote: Counting objects: 461, done.
>     remote: Compressing objects: 100% (141/141), done.
>     remote: Total 263 (delta 227), reused 155 (delta 121)
>     Receiving objects: 100% (263/263), 95.55 KiB, done.
>     Resolving deltas: 100% (227/227), completed with 153 local objects.
>     From git://git.kernel.org/pub/scm/git/git
>        5ba2c22..0868a30  html       -> origin/html
>        2857e17..abeeabe  man        -> origin/man
>        93310a4..95f8ebb  master     -> origin/master
>        559998f..e8bf351  next       -> origin/next
> 
> You can see the commit sha's in the prompt. 03993e1 is fine, 92392b4 is
> broken, I've absolutely no clue about what happens.
> 
> All I can say is that at some point in get_data_from_pack, obj[1].idx
> points to something that is *not* a sha so it's probably corrupted.
> (from index-pack.c).

  Though reading the code, we trust c->data NULL-iness to mean we have
no data, and there is one code path that fails to reset it. The problem
is I'm not able to reproduce the bug, because I foolishly didn't
backuped the git repository that exhibited the failure, so I cannot know
if that can be the problem:

--- snip ---
From c3749f7bc50c642c5d437b2746d4ba589b7d9739 Mon Sep 17 00:00:00 2001
From: Pierre Habouzit <madcoder@debian.org>
Date: Wed, 23 Jul 2008 01:35:11 +0200
Subject: [PATCH] index-pack: missing pointer reset to NULL.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 index-pack.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/index-pack.c b/index-pack.c
index ac20a46..19c39e5 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -257,6 +257,7 @@ static void unlink_base_data(struct base_data *c)
 		base_cache = NULL;
 	if (c->data) {
 		free(c->data);
+		c->data = NULL;
 		base_cache_used -= c->size;
 	}
 }
-- 
1.6.0.rc0.153.ge8bf3.dirty


[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply related

* Re: [PATCH] git-filter-branch.sh: Allow running in bare repositories
From: Petr Baudis @ 2008-07-22 23:36 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: gitster, git
In-Reply-To: <alpine.DEB.1.00.0807230025050.8986@racer>

  Hi,

On Wed, Jul 23, 2008 at 12:27:07AM +0100, Johannes Schindelin wrote:
> On Wed, 23 Jul 2008, Petr Baudis wrote:
> 
> > This also fixes suspicious shell boolean expression during a check
> > for dirty working tree.
> 
> If you are talking about X && Y || Z, it is well established (and should 
> not be suspicious to a shell hacker like the creator of Cogito) that Z is 
> executed if either X fails, or X succeeds and Y fails.

  um, oops. I actually never got to know these by heart since I learnt
to expliciply group the expressions early on. I guess my only excuse is
that I've stumbled at 0bdf93cbf earlier and understood it the _wrong_
way around since I'm getting really sleepy. ;-)

  I still think my change improves the code readibility so it could be
kept, but I'm fairly neutral on this.

> > +test_expect_success 'rewrite bare repository identically' '
> > +	(git config core.bare true && cd .git && git-filter-branch branch)
> > +'
> > +git config core.bare false
> 
> Any reason why this is done outside the test?

  If the test fails in the middle, not resetting this might negatively
affect the rest of the testsuite.

-- 
				Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name.  -- Ken Thompson and Dennis M. Ritchie

^ permalink raw reply

* Re: regression in  92392b4
From: Johannes Schindelin @ 2008-07-22 23:34 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: spearce, Git ML, Junio C Hamano
In-Reply-To: <20080722231745.GD11831@artemis.madism.org>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2507 bytes --]

Hi,

On Wed, 23 Jul 2008, Pierre Habouzit wrote:

>   Hi, here is a manual painful down-secting (opposed to a bisect ;P) I
> did, since git in next cannot fetch on a regular basis for me. The
> culprit seems to be commit  92392b4:
> 
>     ┌─(1:11)──<~/dev/scm/git 92392b4...>──
>     └[artemis] git fetch
>     remote: Counting objects: 461, done.
>     remote: Compressing objects: 100% (141/141), done.
>     remote: Total 263 (delta 227), reused 155 (delta 121)
>     Receiving objects: 100% (263/263), 95.55 KiB, done.
>     fatal: Out of memory, malloc failed
>     fatal: index-pack failed
>     [2]    16674 abort (core dumped)  git fetch
> 
>     ┌─(1:12)──<~/dev/scm/git 92392b4...>──
>     └[artemis] git checkout -m HEAD~1; make git-index-pack
>     Previous HEAD position was 92392b4... index-pack: Honor core.deltaBaseCacheLimit when resolving deltas
>     HEAD is now at 03993e1... index-pack: Track the object_entry that creates each base_data
>     GIT_VERSION = 1.5.6.3.3.g03993
> 	CC index-pack.o
> 	LINK git-index-pack
> 
>     ┌─(1:12)──<~/dev/scm/git 03993e1...>──
>     └[artemis] git fetch
>     remote: Counting objects: 461, done.
>     remote: Compressing objects: 100% (141/141), done.
>     remote: Total 263 (delta 227), reused 155 (delta 121)
>     Receiving objects: 100% (263/263), 95.55 KiB, done.
>     Resolving deltas: 100% (227/227), completed with 153 local objects.
>     From git://git.kernel.org/pub/scm/git/git
>        5ba2c22..0868a30  html       -> origin/html
>        2857e17..abeeabe  man        -> origin/man
>        93310a4..95f8ebb  master     -> origin/master
>        559998f..e8bf351  next       -> origin/next
> 
> You can see the commit sha's in the prompt. 03993e1 is fine, 92392b4 is
> broken, I've absolutely no clue about what happens.
> 
> All I can say is that at some point in get_data_from_pack, obj[1].idx
> points to something that is *not* a sha so it's probably corrupted.
> (from index-pack.c).

Just a guess:

-- snipsnap --
[PATCH] index-pack: set base_data's data member to NULL after freeing it

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

 index-pack.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/index-pack.c b/index-pack.c
index ac20a46..19c39e5 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -257,6 +257,7 @@ static void unlink_base_data(struct base_data *c)
 		base_cache = NULL;
 	if (c->data) {
 		free(c->data);
+		c->data = NULL;
 		base_cache_used -= c->size;
 	}
 }

^ permalink raw reply related

* Re: [PATCH] bring description of git diff --cc up to date
From: Jonathan Nieder @ 2008-07-22 23:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, David Greaves
In-Reply-To: <7vd4l5lio1.fsf@gitster.siamese.dyndns.org>

Junio C Hamano wrote:

> Actually Linus talks about "when you have two versions to
> choose from, and if the result matches one of them, then it is not
> interesting".

That is much better - thanks.  (The description by Linus I was referring
to was from
<http://thread.gmane.org/gmane.comp.version-control.git/89415>:
"So "--cc" only shows output if: the merge itself actually changed
something from _all_ parents" - which is not too misleading since the
two-parent case is the usual one.)

How about this, then?
--- %< ---
Subject: document diff --cc's long-ago-changed semantics

In February 2006 [1], the definition of "interesting hunk" for
git's "compact combined diff" format changed, without any
corresponding change in documentation.  This patch brings the
documentation up to date.

[1] commit bf1c32bdec8223785c779779d0a660a099f69a63
    combine-diff: update --cc "uninteresting hunks" logic
---
 Documentation/git-diff-tree.txt    |   12 +++++++-----
 Documentation/rev-list-options.txt |    9 +++++----
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-diff-tree.txt b/Documentation/git-diff-tree.txt
index 0e45b58..14dc70d 100644
--- a/Documentation/git-diff-tree.txt
+++ b/Documentation/git-diff-tree.txt
@@ -92,12 +92,14 @@ include::pretty-options.txt[]
 --cc::
 	This flag changes the way a merge commit patch is displayed,
 	in a similar way to the '-c' option. It implies the '-c'
-	and '-p' options and further compresses the patch output
-	by omitting hunks that show differences from only one
-	parent, or show the same change from all but one parent
-	for an Octopus merge.  When this optimization makes all
+	and '-p' options and makes the patch output
+	even more compact by omitting uninteresting hunks.  A hunk is
+	considered uninteresting if the person merging had two versions
+	to choose between among all of the parents and the result shows
+	no changes from one of those versions.
+	When this optimization makes all
 	hunks disappear, the commit itself and the commit log
-	message is not shown, just like in any other "empty diff" case.
+	message are not shown, just like in any other "empty diff" case.
 
 --always::
 	Show the commit itself and the commit log message even
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index b6f5d87..c61d05d 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -111,10 +111,11 @@ options may be given. See linkgit:git-diff-files[1] for more options.
 
 --cc::
 
-	This flag implies the '-c' options and further compresses the
-	patch output by omitting hunks that show differences from only
-	one parent, or show the same change from all but one parent for
-	an Octopus merge.
+	This flag implies the '-c' option and makes the patch output
+	even more compact by omitting uninteresting hunks.  A hunk is
+	considered uninteresting if the person merging had two versions
+	to choose between among all of the parents and the result shows
+	no changes from one of those versions.
 
 -r::
 
-- 
1.5.6.3.549.g8ca11

^ permalink raw reply related

* Re: [PATCH] git-filter-branch.sh: Allow running in bare repositories
From: Johannes Schindelin @ 2008-07-22 23:27 UTC (permalink / raw)
  To: Petr Baudis; +Cc: gitster, git
In-Reply-To: <20080722223710.29084.61373.stgit@localhost>

Hi,

On Wed, 23 Jul 2008, Petr Baudis wrote:

> Commit 46eb449c restricted git-filter-branch to non-bare repositories
> unnecessarily; git-filter-branch can work on bare repositories just
> fine.

I think this is fine.

> This also fixes suspicious shell boolean expression during a check
> for dirty working tree.

If you are talking about X && Y || Z, it is well established (and should 
not be suspicious to a shell hacker like the creator of Cogito) that Z is 
executed if either X fails, or X succeeds and Y fails.

> +test_expect_success 'rewrite bare repository identically' '
> +	(git config core.bare true && cd .git && git-filter-branch branch)
> +'
> +git config core.bare false

Any reason why this is done outside the test?

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] Respect crlf attribute even if core.autocrlf has not been set
From: Johannes Schindelin @ 2008-07-22 23:23 UTC (permalink / raw)
  To: Dmitry Potapov; +Cc: git, gitster
In-Reply-To: <20080722231153.GN2925@dpotapov.dyndns.org>

Hi,

On Wed, 23 Jul 2008, Dmitry Potapov wrote:

> On Tue, Jul 22, 2008 at 10:56:04PM +0100, Johannes Schindelin wrote:
> > 
> > When a file's crlf attribute is explicitely set, it does not make 
> > sense to ignore it, just because the config variable core.autocrlf has 
> > not been set.
> 
> Hmm... About a week ago, I was about to propose the same change, but 
> after reading documentation and some thinking I was not able to convince 
> myself that this change would be the right thing to do.

Well, I have a shared repository, where I set the attribute.  Now, every 
once in a while, people check in text _with_ CR/LF.  Yes, that is right, I 
marked it explicitely as crlf, yet I am on the whim of the people choosing 
to set the config variable or not.

And I could not care less what the documentation says: if it does not make 
sense, it does not make sense.

> > +test_expect_success 'attribute crlf is heeded even without core.autocrlf' '
> 
> s/heeded/needed/

Nope.  "heeded" is what I meant.  I am not a native speaker, so this could 
be wrong.  But "needed" is not what I meant (the sentence would not make 
sense with "needed").

Ciao,
Dscho

^ permalink raw reply

* Re: Q: how to remove or move submodules?
From: Petr Baudis @ 2008-07-22 23:22 UTC (permalink / raw)
  To: Rob Sanheim; +Cc: git
In-Reply-To: <fc113d400807122338o637cc159n5e19fea5a15dc866@mail.gmail.com>

  Hi,

On Sun, Jul 13, 2008 at 02:38:51AM -0400, Rob Sanheim wrote:
> I don't see any info on what the 'right' way is to delete or move
> submodules around in any of the docs.  Normally I end up just hacking
> my way through it and hand modifying the .gitmodules file, but this
> seems wrong.  Is there a recommended way?

  currently, you have to hand-modify the .gitmodules file, then remove
the directory from the working tree and then from the index. In part
inspired by your question, I have submitted patches to add git mv and
git rm support for submodules some time ago, but they probably won't
make it to 1.6.0.

				Petr "Pasky" Baudis

^ permalink raw reply

* regression in  92392b4
From: Pierre Habouzit @ 2008-07-22 23:17 UTC (permalink / raw)
  To: spearce; +Cc: Git ML, Junio C Hamano

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

  Hi, here is a manual painful down-secting (opposed to a bisect ;P) I
did, since git in next cannot fetch on a regular basis for me. The
culprit seems to be commit  92392b4:

    ┌─(1:11)──<~/dev/scm/git 92392b4...>──
    └[artemis] git fetch
    remote: Counting objects: 461, done.
    remote: Compressing objects: 100% (141/141), done.
    remote: Total 263 (delta 227), reused 155 (delta 121)
    Receiving objects: 100% (263/263), 95.55 KiB, done.
    fatal: Out of memory, malloc failed
    fatal: index-pack failed
    [2]    16674 abort (core dumped)  git fetch

    ┌─(1:12)──<~/dev/scm/git 92392b4...>──
    └[artemis] git checkout -m HEAD~1; make git-index-pack
    Previous HEAD position was 92392b4... index-pack: Honor core.deltaBaseCacheLimit when resolving deltas
    HEAD is now at 03993e1... index-pack: Track the object_entry that creates each base_data
    GIT_VERSION = 1.5.6.3.3.g03993
	CC index-pack.o
	LINK git-index-pack

    ┌─(1:12)──<~/dev/scm/git 03993e1...>──
    └[artemis] git fetch
    remote: Counting objects: 461, done.
    remote: Compressing objects: 100% (141/141), done.
    remote: Total 263 (delta 227), reused 155 (delta 121)
    Receiving objects: 100% (263/263), 95.55 KiB, done.
    Resolving deltas: 100% (227/227), completed with 153 local objects.
    From git://git.kernel.org/pub/scm/git/git
       5ba2c22..0868a30  html       -> origin/html
       2857e17..abeeabe  man        -> origin/man
       93310a4..95f8ebb  master     -> origin/master
       559998f..e8bf351  next       -> origin/next

You can see the commit sha's in the prompt. 03993e1 is fine, 92392b4 is
broken, I've absolutely no clue about what happens.

All I can say is that at some point in get_data_from_pack, obj[1].idx
points to something that is *not* a sha so it's probably corrupted.
(from index-pack.c).


-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCH] Respect crlf attribute even if core.autocrlf has not been set
From: Dmitry Potapov @ 2008-07-22 23:11 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <alpine.DEB.1.00.0807222255450.8986@racer>

On Tue, Jul 22, 2008 at 10:56:04PM +0100, Johannes Schindelin wrote:
> 
> When a file's crlf attribute is explicitely set, it does not make sense
> to ignore it, just because the config variable core.autocrlf has not
> been set.

Hmm... About a week ago, I was about to propose the same change, but
after reading documentation and some thinking I was not able to convince
myself that this change would be the right thing to do.

First, let's look at what Git's documentation says:

===
`crlf`
^^^^^^

This attribute controls the line-ending convention.

Set::

	Setting the `crlf` attribute on a path is meant to mark
	the path as a "text" file.  'core.autocrlf' conversion
	takes place without guessing the content type by
	inspection.

<snip>

The `core.autocrlf` conversion
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

If the configuration variable `core.autocrlf` is false, no
conversion is done.
===

So, my reading is that if I set the `crlf` attribute on some path, but
I have core.autocrlf=false, there will be no conversion.

And this can be used to mark text files in .gitattribute, which is
stored in the repository and thus it is shared among users with
different end-of-line ending, i.e. you can have something like this
in .gitattribute:

*.[ch] crlf
*.txt crlf

but on Unix, you have core.autocrlf=false, so no conversion is done,
while, on Windows, you set core.autocrlf=true, so you will have crlf
conversion without any guessing.

Now, I can agree with you that using the 'crlf' attribute to mark text
files may appear not very intuitive (you may expect that crlf means that
those files always need crlf conversion), but right now we do not have
any better way to mark text files and the using crlf in this role is
explicitly suggested by documentation. See above.


> diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh
> index 1be7446..0bb3e6f 100755
> --- a/t/t0020-crlf.sh
> +++ b/t/t0020-crlf.sh
> @@ -436,4 +436,14 @@ test_expect_success 'invalid .gitattributes (must not crash)' '
>  
>  '
>  
> +test_expect_success 'attribute crlf is heeded even without core.autocrlf' '

s/heeded/needed/


Dmitry

^ permalink raw reply

* Re: [FYI PATCH] git wrapper: DWIM mistyped commands
From: Sverre Rabbelier @ 2008-07-22 23:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vwsjdjz9e.fsf@gitster.siamese.dyndns.org>

On Wed, Jul 23, 2008 at 1:05 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>>       So I mistyped 'reabse' for the hundred trillionth time, but I
>>       will never have to correct my mistakes again.
>>
>>       Note: this patch is _not_ meant for inclusion.
>
> Heh, I do "git emrge" all the time ;-)
>
> Time to start a "gitster private edition" branch for my own use, I guess.

If it is the same typo all the time, wouldn't aliasses do the trick?

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH] perl/Makefile: update NO_PERL_MAKEMAKER section
From: Petr Baudis @ 2008-07-22 23:09 UTC (permalink / raw)
  To: Brandon Casey; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <XKT02T_QW2tKLHpR7e3VuZjLXv3RP2E0GD54gXKrdIFm8xQsKvAyjg@cipher.nrlssc.navy.mil>

On Tue, Jul 22, 2008 at 04:15:41PM -0500, Brandon Casey wrote:
> The perl modules must be copied to blib/lib so they are available for
> testing.
> 
> Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>

I don't understand why do you need to do this; perl.mak should do this
on its own during project-wide make all. What Perl version are you using?
How does the pm_to_blib target look like?

	pm_to_blib : $(TO_INST_PM)
        $(NOECHO) $(ABSPERLRUN) -MExtUtils::Install -e 'pm_to_blib({@ARGV}, '\''$(INST_LIB)/auto'\'', '\''$(PM_FILTER)'\'')' -- \
          Git.pm $(INST_LIBDIR)/Git.pm
        $(NOECHO) $(TOUCH) pm_to_blib

here. Is your INST_LIB = blib/lib and INST_LIBDIR = $(INST_LIB)?

-- 
				Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name.  -- Ken Thompson and Dennis M. Ritchie

^ permalink raw reply

* Re: [PATCH] Add help.autocorrect to enable/disable autocorrecting
From: Junio C Hamano @ 2008-07-22 23:08 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Alex Riesen, git
In-Reply-To: <alpine.DEB.1.00.0807222242160.8986@racer>

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

> Hi,
>
> On Tue, 22 Jul 2008, Alex Riesen wrote:
>
>> @@ -704,9 +707,10 @@ const char *help_unknown_cmd(const char *cmd)
>>  
>>  	if (!main_cmds.cnt)
>>  		die ("Uh oh.  Your system reports no Git commands at all.");
>> +	git_config(git_help_config, NULL);
>>  	best_similarity = similarity(main_cmds.names[0]->name);
>> -	if (main_cmds.cnt < 2 || best_similarity <
>> -			similarity(main_cmds.names[1]->name)) {
>> +	if (autocorrect && (main_cmds.cnt < 2 ||
>> +		best_similarity < similarity(main_cmds.names[1]->name))) {
>>  		if (!*cwd)
>>  			exit(1);
>>  		if (chdir(cwd))
>
> This "if" already checks if there is only one candidate.  So you should 
> just add an inner "if (autocorrect) ... else single = 1;" or some such.
>
> However, I think that the intention of this patch is too much DWIMery, 
> which might be good for me (just like my "git add remote" patch), but not 
> for the general audience.

Please make autocorrect not a binary but optionally the number of
deciseconds before it continues, so that I have a chance to hit ^C ;-)

^ permalink raw reply

* Re: [FYI PATCH] git wrapper: DWIM mistyped commands
From: Junio C Hamano @ 2008-07-22 23:05 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0807222100150.8986@racer>

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

> 	So I mistyped 'reabse' for the hundred trillionth time, but I
> 	will never have to correct my mistakes again.
>
> 	Note: this patch is _not_ meant for inclusion.

Heh, I do "git emrge" all the time ;-)

Time to start a "gitster private edition" branch for my own use, I guess.

^ permalink raw reply

* Re: [PATCH] Documentation/git-ls-tree.txt: Add a caveat about prefixing pathspec
From: Petr Baudis @ 2008-07-22 22:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd4l6sqqz.fsf@gitster.siamese.dyndns.org>

On Mon, Jul 21, 2008 at 05:32:20PM -0700, Junio C Hamano wrote:
> Petr Baudis <pasky@suse.cz> writes:
> 
> >> 	commit.  E.g.
> >> 
> >> 		$ cd some/deep/path
> >> 		$ git ls-tree --name-only -r HEAD~20
> >> 
> >> 	will list the files in some/deep/path (i.e. where you are) 20
> >> 	commits ago, just like running "/bin/ls" there will give you the
> >> 	list of files you have right now.
> >
> > Frankly, I think this is overdoing it. I'm all for being positive, but
> > it is obvious why this is good thing when you inspect a root tree and
> > there's no need to be too wordy about it...
> 
> I mildly disagree.

We may throw a dice or go with your version, I don't care *that* much
about this change, I just wouldn't make it personally.

> If the person had truly understood that, why do we even have this thread
> to begin with?

To nudge the person to the "aha" moment.

> Description on *what* it does (i.e. "like what ls -a does in the current
> working directory" we have in the Description section) obviously was not
> good enough.

I don't understand; what does auto-prefixing have to do with the
"ls -a" mention?

> It will be better understood if you describe *why* it does
> it that way at the same time.

My version implies that for examining the root tree, without surplusage.

-- 
				Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name.  -- Ken Thompson and Dennis M. Ritchie

^ permalink raw reply

* [PATCH] git-filter-branch.sh: Allow running in bare repositories
From: Petr Baudis @ 2008-07-22 22:37 UTC (permalink / raw)
  To: gitster; +Cc: git, Johannes Schindelin

Commit 46eb449c restricted git-filter-branch to non-bare repositories
unnecessarily; git-filter-branch can work on bare repositories just
fine.

This also fixes suspicious shell boolean expression during a check
for dirty working tree.

Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 git-filter-branch.sh     |   36 ++++++++++++++++++++----------------
 t/t7003-filter-branch.sh |    8 ++++++++
 2 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index d04c346..dcfc6be 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -97,9 +97,11 @@ USAGE="[--env-filter <command>] [--tree-filter <command>] \
 OPTIONS_SPEC=
 . git-sh-setup
 
-git diff-files --quiet &&
-	git diff-index --cached --quiet HEAD -- ||
-	die "Cannot rewrite branch(es) with a dirty working directory."
+if [ "$(is_bare_repository)" = false ]; then
+	(git diff-files --quiet &&
+	 git diff-index --cached --quiet HEAD --) ||
+		die "Cannot rewrite branch(es) with a dirty working directory."
+fi
 
 tempdir=.git-rewrite
 filter_env=
@@ -434,18 +436,20 @@ rm -rf "$tempdir"
 
 trap - 0
 
-unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE
-test -z "$ORIG_GIT_DIR" || {
-	GIT_DIR="$ORIG_GIT_DIR" && export GIT_DIR
-}
-test -z "$ORIG_GIT_WORK_TREE" || {
-	GIT_WORK_TREE="$ORIG_GIT_WORK_TREE" &&
-	export GIT_WORK_TREE
-}
-test -z "$ORIG_GIT_INDEX_FILE" || {
-	GIT_INDEX_FILE="$ORIG_GIT_INDEX_FILE" &&
-	export GIT_INDEX_FILE
-}
-git read-tree -u -m HEAD
+if [ "$(is_bare_repository)" = false ]; then
+	unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE
+	test -z "$ORIG_GIT_DIR" || {
+		GIT_DIR="$ORIG_GIT_DIR" && export GIT_DIR
+	}
+	test -z "$ORIG_GIT_WORK_TREE" || {
+		GIT_WORK_TREE="$ORIG_GIT_WORK_TREE" &&
+		export GIT_WORK_TREE
+	}
+	test -z "$ORIG_GIT_INDEX_FILE" || {
+		GIT_INDEX_FILE="$ORIG_GIT_INDEX_FILE" &&
+		export GIT_INDEX_FILE
+	}
+	git read-tree -u -m HEAD
+fi
 
 exit $ret
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index e26f726..a0ab096 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -38,6 +38,14 @@ test_expect_success 'result is really identical' '
 	test $H = $(git rev-parse HEAD)
 '
 
+test_expect_success 'rewrite bare repository identically' '
+	(git config core.bare true && cd .git && git-filter-branch branch)
+'
+git config core.bare false
+test_expect_success 'result is really identical' '
+	test $H = $(git rev-parse HEAD)
+'
+
 test_expect_success 'rewrite, renaming a specific file' '
 	git-filter-branch -f --tree-filter "mv d doh || :" HEAD
 '

^ permalink raw reply related

* [PATCH] Documentation/git-filter-branch: Remove Useless Use of Plumbing
From: Petr Baudis @ 2008-07-22 22:24 UTC (permalink / raw)
  To: gitster; +Cc: git

The example to remove file using index-filter uses git update-index
 --remove where git rm --cached works as well.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 Documentation/git-filter-branch.txt |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt
index a3edc00..7ba9dab 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -191,7 +191,7 @@ Thus you may instead want to use `rm -f filename` as the script.
 A significantly faster version:
 
 --------------------------------------------------------------------------
-git filter-branch --index-filter 'git update-index --remove filename' HEAD
+git filter-branch --index-filter 'git rm --cached filename' HEAD
 --------------------------------------------------------------------------
 
 Now, you will get the rewritten history saved in HEAD.

^ permalink raw reply related

* Re: [PATCH] rebase -i: only automatically amend commit if HEAD did not change
From: Avery Pennarun @ 2008-07-22 22:22 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <alpine.DEB.1.00.0807222235520.8986@racer>

On 7/22/08, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>  If the user called "rebase -i", marked a commit as "edit", "rebase
>  --continue" would automatically amend the commit when there were
>  staged changes.
>
>  However, this is actively wrong when the current commit is not the
>  one marked with "edit".  So guard against this.

This patch is perhaps a symptom of something I've been meaning to ask
about for a while.

Why doesn't "edit" just stage the commit and not auto-commit it at
all?  That way an amend would *never* be necessary, and rebase
--continue would always do a commit -a (if there was anything left to
commit).  The special case fixed by this patch would thus not be
needed.

It would also make it more obvious how to remove files from a commit,
for example, without having to learn about "git reset".  For that
matter, you wouldn't have to learn about "git commit --amend" either,
and it would save typing.

It would also be a little more consistent with "squash", which already
lets you edit the commit message by default.

Just a thought.  Presumably it was implemented the way it is for some
reason, but I haven't seen any discussion about it.

Have fun,

Avery

^ permalink raw reply

* [PATCH v2] git daemon: avoid waking up too often
From: Johannes Schindelin @ 2008-07-22 22:03 UTC (permalink / raw)
  To: git, gitster
In-Reply-To: <alpine.DEB.1.00.0807222251570.8986@racer>


To avoid waking up unnecessarily, a pipe is set up that is only ever
written to by child_handler(), when a child disconnects, as suggested
per Junio.

This avoids waking up the main process every second to see if a child
was disconnected.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	Darn.  Forgot to commit after fixing the compiler warning:
	The now-unused variable timeout is gone.

	That is the only difference to the version I have running in
	production ;-)

 daemon.c |   25 +++++++++++--------------
 1 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/daemon.c b/daemon.c
index 7df41a6..4540e8d 100644
--- a/daemon.c
+++ b/daemon.c
@@ -16,6 +16,7 @@
 static int log_syslog;
 static int verbose;
 static int reuseaddr;
+static int child_handler_pipe[2];
 
 static const char daemon_usage[] =
 "git daemon [--verbose] [--syslog] [--export-all]\n"
@@ -788,6 +789,7 @@ static void child_handler(int signo)
 				pid = -pid;
 			dead_child[reaped % MAX_CHILDREN] = pid;
 			children_reaped = reaped + 1;
+			write(child_handler_pipe[1], &status, 1);
 			continue;
 		}
 		break;
@@ -933,29 +935,24 @@ static int service_loop(int socknum, int *socklist)
 	struct pollfd *pfd;
 	int i;
 
-	pfd = xcalloc(socknum, sizeof(struct pollfd));
+	if (pipe(child_handler_pipe) < 0)
+		die ("Could not set up pipe for child handler");
+
+	pfd = xcalloc(socknum + 1, sizeof(struct pollfd));
 
 	for (i = 0; i < socknum; i++) {
 		pfd[i].fd = socklist[i];
 		pfd[i].events = POLLIN;
 	}
+	pfd[socknum].fd = child_handler_pipe[0];
+	pfd[socknum].events = POLLIN;
 
 	signal(SIGCHLD, child_handler);
 
 	for (;;) {
 		int i;
-		int timeout;
 
-		/*
-		 * This 1-sec timeout could lead to idly looping but it is
-		 * here so that children culled in child_handler() are reported
-		 * without too much delay.  We could probably set up a pipe
-		 * to ourselves that we poll, and write to the fd from child_handler()
-		 * to wake us up (and consume it when the poll() returns...
-		 */
-		timeout = (children_spawned != children_deleted) ? 1000 : -1;
-		i = poll(pfd, socknum, timeout);
-		if (i < 0) {
+		if (poll(pfd, socknum + 1, -1) < 0) {
 			if (errno != EINTR) {
 				error("poll failed, resuming: %s",
 				      strerror(errno));
@@ -963,9 +960,9 @@ static int service_loop(int socknum, int *socklist)
 			}
 			continue;
 		}
-		if (i == 0) {
+		if (pfd[socknum].revents & POLLIN) {
+			read(child_handler_pipe[0], &i, 1);
 			check_dead_children();
-			continue;
 		}
 
 		for (i = 0; i < socknum; i++) {
-- 
1.6.0.rc0.22.gf2096d.dirty

^ permalink raw reply related

* [PATCH] Respect crlf attribute even if core.autocrlf has not been set
From: Johannes Schindelin @ 2008-07-22 21:56 UTC (permalink / raw)
  To: git, gitster


When a file's crlf attribute is explicitely set, it does not make sense
to ignore it, just because the config variable core.autocrlf has not
been set.

This patch does not affect the case when the crlf attribute is unset.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	There have been no comments on this patch so far, however I think
	that this is a valid fix which should be in 1.6.0.

 convert.c       |    2 +-
 t/t0020-crlf.sh |   10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/convert.c b/convert.c
index 78efed8..d038d2f 100644
--- a/convert.c
+++ b/convert.c
@@ -126,7 +126,7 @@ static int crlf_to_git(const char *path, const char *src, size_t len,
 	struct text_stat stats;
 	char *dst;
 
-	if ((action == CRLF_BINARY) || !auto_crlf || !len)
+	if ((action == CRLF_BINARY) || (!auto_crlf && action < 0) || !len)
 		return 0;
 
 	gather_stats(src, len, &stats);
diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh
index 1be7446..0bb3e6f 100755
--- a/t/t0020-crlf.sh
+++ b/t/t0020-crlf.sh
@@ -436,4 +436,14 @@ test_expect_success 'invalid .gitattributes (must not crash)' '
 
 '
 
+test_expect_success 'attribute crlf is heeded even without core.autocrlf' '
+
+	echo "allcrlf crlf=input" > .gitattributes &&
+	git config --unset core.autocrlf &&
+	git add allcrlf &&
+	git show :allcrlf | append_cr > expect &&
+	test_cmp allcrlf expect
+
+'
+
 test_done
-- 
1.6.0.rc0.22.gf2096d.dirty

^ permalink raw reply related

* [PATCH] git daemon: avoid waking up too often
From: Johannes Schindelin @ 2008-07-22 21:52 UTC (permalink / raw)
  To: git, gitster


To avoid waking up unnecessarily, a pipe is set up that is only ever
written to by child_handler(), when a child disconnects, as suggested
per Junio.

This avoids waking up the main process every second to see if a child
was disconnected.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	I have this in production ever since I posted it the first time,
	without problems.

	Oh, and it removes more lines than it introduces ;-)

 daemon.c |   24 +++++++++++-------------
 1 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/daemon.c b/daemon.c
index 7df41a6..850f6f9 100644
--- a/daemon.c
+++ b/daemon.c
@@ -16,6 +16,7 @@
 static int log_syslog;
 static int verbose;
 static int reuseaddr;
+static int child_handler_pipe[2];
 
 static const char daemon_usage[] =
 "git daemon [--verbose] [--syslog] [--export-all]\n"
@@ -788,6 +789,7 @@ static void child_handler(int signo)
 				pid = -pid;
 			dead_child[reaped % MAX_CHILDREN] = pid;
 			children_reaped = reaped + 1;
+			write(child_handler_pipe[1], &status, 1);
 			continue;
 		}
 		break;
@@ -933,12 +935,17 @@ static int service_loop(int socknum, int *socklist)
 	struct pollfd *pfd;
 	int i;
 
-	pfd = xcalloc(socknum, sizeof(struct pollfd));
+	if (pipe(child_handler_pipe) < 0)
+		die ("Could not set up pipe for child handler");
+
+	pfd = xcalloc(socknum + 1, sizeof(struct pollfd));
 
 	for (i = 0; i < socknum; i++) {
 		pfd[i].fd = socklist[i];
 		pfd[i].events = POLLIN;
 	}
+	pfd[socknum].fd = child_handler_pipe[0];
+	pfd[socknum].events = POLLIN;
 
 	signal(SIGCHLD, child_handler);
 
@@ -946,16 +953,7 @@ static int service_loop(int socknum, int *socklist)
 		int i;
 		int timeout;
 
-		/*
-		 * This 1-sec timeout could lead to idly looping but it is
-		 * here so that children culled in child_handler() are reported
-		 * without too much delay.  We could probably set up a pipe
-		 * to ourselves that we poll, and write to the fd from child_handler()
-		 * to wake us up (and consume it when the poll() returns...
-		 */
-		timeout = (children_spawned != children_deleted) ? 1000 : -1;
-		i = poll(pfd, socknum, timeout);
-		if (i < 0) {
+		if (poll(pfd, socknum + 1, -1) < 0) {
 			if (errno != EINTR) {
 				error("poll failed, resuming: %s",
 				      strerror(errno));
@@ -963,9 +961,9 @@ static int service_loop(int socknum, int *socklist)
 			}
 			continue;
 		}
-		if (i == 0) {
+		if (pfd[socknum].revents & POLLIN) {
+			read(child_handler_pipe[0], &i, 1);
 			check_dead_children();
-			continue;
 		}
 
 		for (i = 0; i < socknum; i++) {
-- 
1.6.0.rc0.22.gf2096d.dirty

^ permalink raw reply related

* Re: [PATCH] rebase -i: only automatically amend commit if HEAD did not change
From: Stephan Beyer @ 2008-07-22 21:48 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <alpine.DEB.1.00.0807222235520.8986@racer>

Hi,

Johannes Schindelin wrote:
> If the user called "rebase -i", marked a commit as "edit", "rebase
> --continue" would automatically amend the commit when there were
> staged changes.
> 
> However, this is actively wrong when the current commit is not the
> one marked with "edit".  So guard against this.

Hmm, I like it. ;-)

> @@ -419,7 +419,9 @@ do
>  		else
>  			. "$DOTEST"/author-script ||
>  				die "Cannot find the author identity"
> -			if test -f "$DOTEST"/amend
> +			if test -f "$DOTEST"/amend &&
> +				test $(git rev-parse HEAD) = \
> +					$(cat "$DOTEST"/amend)
>  			then
>  				git reset --soft HEAD^ ||
>  				die "Cannot rewind the HEAD"

So if this fails, a non-amending commit is done.  Agreed. :)

Regards,
  Stephan

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

^ permalink raw reply

* Re: [PATCH] Add help.autocorrect to enable/disable autocorrecting
From: Johannes Schindelin @ 2008-07-22 21:44 UTC (permalink / raw)
  To: Alex Riesen; +Cc: git
In-Reply-To: <20080722212633.GF5113@blimp.local>

Hi,

On Tue, 22 Jul 2008, Alex Riesen wrote:

> @@ -704,9 +707,10 @@ const char *help_unknown_cmd(const char *cmd)
>  
>  	if (!main_cmds.cnt)
>  		die ("Uh oh.  Your system reports no Git commands at all.");
> +	git_config(git_help_config, NULL);
>  	best_similarity = similarity(main_cmds.names[0]->name);
> -	if (main_cmds.cnt < 2 || best_similarity <
> -			similarity(main_cmds.names[1]->name)) {
> +	if (autocorrect && (main_cmds.cnt < 2 ||
> +		best_similarity < similarity(main_cmds.names[1]->name))) {
>  		if (!*cwd)
>  			exit(1);
>  		if (chdir(cwd))

This "if" already checks if there is only one candidate.  So you should 
just add an inner "if (autocorrect) ... else single = 1;" or some such.

However, I think that the intention of this patch is too much DWIMery, 
which might be good for me (just like my "git add remote" patch), but not 
for the general audience.

Ciao,
Dscho

^ permalink raw reply

* Re: Hacks for AIX
From: Brandon Casey @ 2008-07-22 21:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Chris Cowan, git
In-Reply-To: <hT7kYDX9f_eBxi8JC0s7jG9oqm8sZ1QCTlEg1n8Dqus7U98hWLWnBA@cipher.nrlssc.navy.mil>

Brandon Casey wrote:
> Junio C Hamano wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>>>     * /usr/bin/patch - really old version, doesn't do well with some
>>>> diff formats.   I avoid using it.
>>> t4109 seems to use patch to produce expected output for the tests; we
>>> should ship a precomputed expected results.  Do you know of any other
>>> places "patch" is used?
>> As usual, I won't commit this patch unless I hear from people who
>> potentially would benefit from it.  I do not need such a patch myself and
>> I really shouldn't be spending too much of my time on these.
> 
> 
> Unless I'm doing something wrong, this doesn't apply cleanly to master.

I figured out that your patch was against maint.

I see it is now applied to master and it helps out on solaris with old patch.

Now I need to get rid of 'diff -U0' in t1002-read-tree-m-u-2way.sh.

-brandon

^ 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