Git development
 help / color / mirror / Atom feed
* Re: [PATCHv2] gitweb: refactor input parameters parse/validation
From: Giuseppe Bilotta @ 2008-10-10 17:33 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Jakub Narebski
In-Reply-To: <20081010150108.GC29829@spearce.org>

On Fri, Oct 10, 2008 at 5:01 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
> Giuseppe Bilotta <giuseppe.bilotta@gmail.com> wrote:
>> +sub validate_action {
>> +     my $input = shift || return undef;
>> +     return undef unless exists $actions{$action};
>> +     return $action;
>> +}
>
> Shouldn't $action here be $input?

Urgh. Yes, yes it should. Good catch. Should I resend?


-- 
Giuseppe "Oblomov" Bilotta

^ permalink raw reply

* Re: [RFC PATCH] describe: Make --tags and --all match lightweight tags  more often
From: Pierre Habouzit @ 2008-10-10 17:12 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Junio C Hamano, Erez Zilber
In-Reply-To: <20081010165952.GI8203@spearce.org>

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

On Fri, Oct 10, 2008 at 04:59:52PM +0000, Shawn O. Pearce wrote:
> If the caller supplies --tags they want the lightweight, unannotated
> tags to be searched for a match.  If a lightweight tag is closer
> in the history, it should be matched, even if an annotated tag is
> reachable further back in the commit chain.
> 
> The same applies with --all when matching any other type of ref.
> 
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> ---
> 
>  This come out of the discussions earlier last week, where folks
>  were confused about the meaning of --tags and wanted to see it
>  behave as they expected, which was to match the nearest tag,
>  no matter its "type".
> 
>  The code is unchanged from what I sent out before, but now it has
>  updated test vectors and a commit message.
> 
>  Thoughts?

I would like to see an enhanced information in the documentation so that
people remember that lightweight tags are not meant to be constant over
time and that's a bad idea to use them.

What the discussion showed, is that the people don't know about
annotated tags, and git-describe should have a stub of documentation
that points to git-tag(1) so that people learn about it.

Apart from that, it feels fine.

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

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

^ permalink raw reply

* [RFC PATCH] describe: Make --tags and --all match lightweight tags more often
From: Shawn O. Pearce @ 2008-10-10 16:59 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Pierre Habouzit, Erez Zilber
In-Reply-To: <20080930083940.GA11453@artemis.corp>

If the caller supplies --tags they want the lightweight, unannotated
tags to be searched for a match.  If a lightweight tag is closer
in the history, it should be matched, even if an annotated tag is
reachable further back in the commit chain.

The same applies with --all when matching any other type of ref.

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

 This come out of the discussions earlier last week, where folks
 were confused about the meaning of --tags and wanted to see it
 behave as they expected, which was to match the nearest tag,
 no matter its "type".

 The code is unchanged from what I sent out before, but now it has
 updated test vectors and a commit message.

 Thoughts?

 builtin-describe.c  |    6 ++----
 t/t6120-describe.sh |    8 ++++----
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/builtin-describe.c b/builtin-describe.c
index ec404c8..fd54fec 100644
--- a/builtin-describe.c
+++ b/builtin-describe.c
@@ -15,8 +15,8 @@ static const char * const describe_usage[] = {
 };
 
 static int debug;	/* Display lots of verbose info */
-static int all;	/* Default to annotated tags only */
-static int tags;	/* But allow any tags if --tags is specified */
+static int all;	/* Any valid ref can be used */
+static int tags;	/* Either lightweight or annotated tags */
 static int longformat;
 static int abbrev = DEFAULT_ABBREV;
 static int max_candidates = 10;
@@ -112,8 +112,6 @@ static int compare_pt(const void *a_, const void *b_)
 {
 	struct possible_tag *a = (struct possible_tag *)a_;
 	struct possible_tag *b = (struct possible_tag *)b_;
-	if (a->name->prio != b->name->prio)
-		return b->name->prio - a->name->prio;
 	if (a->depth != b->depth)
 		return a->depth - b->depth;
 	if (a->found_order != b->found_order)
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 16cc635..e6c9e59 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -91,10 +91,10 @@ check_describe D-* HEAD^^
 check_describe A-* HEAD^^2
 check_describe B HEAD^^2^
 
-check_describe A-* --tags HEAD
-check_describe A-* --tags HEAD^
-check_describe D-* --tags HEAD^^
-check_describe A-* --tags HEAD^^2
+check_describe c-* --tags HEAD
+check_describe c-* --tags HEAD^
+check_describe e-* --tags HEAD^^
+check_describe c-* --tags HEAD^^2
 check_describe B --tags HEAD^^2^
 
 check_describe B-0-* --long HEAD^^2^
-- 
1.6.0.2.687.g8544f

-- 
Shawn.

^ permalink raw reply related

* Re: [PATCH] git-gui: Correctly set up locators in case of preset URL variable
From: Shawn O. Pearce @ 2008-10-10 16:42 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git, Petr Baudis
In-Reply-To: <1223028826-10306-1-git-send-email-pasky@suse.cz>

Petr Baudis <pasky@suse.cz> wrote:
> This patch fixes locators setup in case the URL variable is already set,
> e.g. in the clone dialog during 'git gui clone'.

Huh.  Better, but Remote->Push... still crashes with your series:

can't read "push_url": no such variable
can't read "push_url": no such variable
    while executing
"set $urlvar"
    (procedure "location_input" line 26)
    invoked from within
"location_input $w.dest.url_t push_url push"
    (procedure "do_push_anywhere" line 53)
    invoked from within
"do_push_anywhere"
    invoked from within
".#mbar.#mbar#remote invoke active"
    ("uplevel" body line 1)
    invoked from within
"uplevel #0 [list $w invoke active]"
    (procedure "tk::MenuInvoke" line 50)
    invoked from within
"tk::MenuInvoke .#mbar.#mbar#remote 1"
    (command bound to event)

 
> diff --git a/git-gui/lib/transport.tcl b/git-gui/lib/transport.tcl
> index 277e6b8..02c4eca 100644
> --- a/git-gui/lib/transport.tcl
> +++ b/git-gui/lib/transport.tcl
> @@ -68,8 +68,13 @@ proc location_input {widget urlvar op} {
>  
>  	global _locator_template _locator_input _locator_var
>  	trace remove variable _locator_input write locator_update
> -	set _locator_template $default_locator
> -	set _locator_input {}
> +	if {[set $urlvar] == {}} {
> +		set _locator_template $default_locator
> +		set _locator_input {}
> +	} else {
> +		set _locator_template "URL"
> +		set _locator_input [set $urlvar]
> +	}
>  	set _locator_var $urlvar
>  	trace add variable _locator_input write locator_update
>  
> -- 
> tg: (3c6c738..) t/git-gui/locator-preset (depends on: git-gui/locators t/git-gui/clonecmd)

-- 
Shawn.

^ permalink raw reply

* Re: git confused by rename
From: Björn Steinbrink @ 2008-10-10 16:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Michael P. Soulier, git
In-Reply-To: <7v3aj4momt.fsf@gitster.siamese.dyndns.org>

On 2008.10.10 08:54:50 -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> > Because git does not care how you created B (IOW, it does not matter
> > if B was typed from scratch with copying and pasting, or created by
> > copying and editing), the "renamed: A -> B" entry itself is not
> > surprising nor look like a bug at all,...
> 
> The above observation of mine is correct, but I forgot that "git status"
> (and the comment in the commit template from "git commit") is generated
> internally with "diff-index -B -M".  So if
> 
>  (0) had A but not B in the HEAD commit;
>  (1) you created B that is very similar to the original A; and
>  (2) you modified A beyond recognition;
> 
> then git will say "A was modified and B was created by renaming A".
> 
> > ... but its presense at the same time as "modified: A" does feel very
> > fishy.
> 
> So this is not fishy anymore.
> 
> This however makes me wonder if "diff-index -B -M" should say B is copied
> (instead of being renamed) from A and A is modified in such a case.  I do
> not think we would want to make such a change without thinking things,
> through.

Oh, heh, my patch doesn't help in that case. It's still modify + rename
with it, of course, as -B still breaks the stuff apart. Me being
confused about -M reporting copies in some cases remains though, I had
really expected that only -C would detect copies. Of course, this
confusion is purely from a user's point of view, I have zero knowledge
about the involved code...

Björn

^ permalink raw reply

* Re: tip tree clone fail
From: H. Peter Anvin @ 2008-10-10 15:58 UTC (permalink / raw)
  To: Wang Chen
  Cc: Thomas Gleixner, Ingo Molnar, FNST-Lai Jiangshan,
	FJ-KOSAKI Motohiro, git, Junio C Hamano
In-Reply-To: <48EF1902.4070309@cn.fujitsu.com>

Wang Chen wrote:
>>
>> So http transport is wreckaged. (git version 1.6.0.1 here, Wang is using
>> 1.5.3.x)
> 
> My git version is 1.5.5.1, although it doesn't matter ;)
> 

http transport requires that "git update-server-info" is done after each 
push.  Otherwise, it ends up in a trainwreck.

Shawn Pierce at Google is working on a long-term solution, but in the 
meantime, if you have to use http transport, murder your IT people.

	-hpa

^ permalink raw reply

* [PATCH] git-status: Detect copies as well as renames
From: Björn Steinbrink @ 2008-10-10 16:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Michael P. Soulier, git
In-Reply-To: <7vabdcmp4n.fsf@gitster.siamese.dyndns.org>

Signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de>
---
On 2008.10.10 08:44:08 -0700, Junio C Hamano wrote:
> "Michael P. Soulier" <msoulier@digitaltorque.ca> writes:
> 
> > #       renamed:    templates/scrc/index.html -> templates/scrc/cres_taps.html
> > #       modified:   templates/scrc/index.html
> >
> > Looking at the last two lines here, there is obviously an issue.
> 
> I am puzzled.
> 
> Looking at wt-status.c "renamed: A -> B" will be shown only when A and
> B are similar enough *and* there is no A remaining in the final
> result, and in such a case, "modified: A" should not be shown.
> Because git does not care how you created B (IOW, it does not matter
> if B was typed from scratch with copying and pasting, or created by
> copying and editing), the "renamed: A -> B" entry itself is not
> surprising nor look like a bug at all, but its presense at the same
> time as "modified: A" does feel very fishy.

Maybe something like this is required? The "1" meant that only renames
were to be detected, so I'm quite puzzled why it detects copies in some
cases at all. At least some trivial:

cp Makefile Makefile2
echo 123 >> Makefile
git add Makefile*

Now correctly shows the Makefile2 being a copy of Makefile, while the
old behaviour was to simply show it as a new file. _BUT_ when you also
did:

git mv Makefile Makefile3

You see a rename of Makefile and a copy of Makefile. Even though we
didn't request copy detection... Hu?

I feel like stabbing in the dark though, so I intentionally left out a
description in the commit message that tells why this change is good. I
simply have no idea, it just _felt_ better. ;-)

Björn

 wt-status.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 889e50f..3d645b9 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -210,7 +210,7 @@ static void wt_status_print_updated(struct wt_status *s)
 	rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
 	rev.diffopt.format_callback = wt_status_print_updated_cb;
 	rev.diffopt.format_callback_data = s;
-	rev.diffopt.detect_rename = 1;
+	rev.diffopt.detect_rename = DIFF_DETECT_COPY;
 	rev.diffopt.rename_limit = 200;
 	rev.diffopt.break_opt = 0;
 	run_diff_index(&rev, 1);
@@ -310,7 +310,7 @@ static void wt_status_print_verbose(struct wt_status *s)
 	init_revisions(&rev, NULL);
 	setup_revisions(0, NULL, &rev, s->reference);
 	rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
-	rev.diffopt.detect_rename = 1;
+	rev.diffopt.detect_rename = DIFF_DETECT_COPY;
 	rev.diffopt.file = s->fp;
 	rev.diffopt.close_file = 0;
 	run_diff_index(&rev, 1);
-- 
1.6.0.2.307.gc427.dirty

^ permalink raw reply related

* Re: git confused by rename
From: Junio C Hamano @ 2008-10-10 15:54 UTC (permalink / raw)
  To: Michael P. Soulier; +Cc: git
In-Reply-To: <7vabdcmp4n.fsf@gitster.siamese.dyndns.org>

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

> "Michael P. Soulier" <msoulier@digitaltorque.ca> writes:
>
>> #       renamed:    templates/scrc/index.html -> templates/scrc/cres_taps.html
>> #       modified:   templates/scrc/index.html
>>
>> Looking at the last two lines here, there is obviously an issue.
>
> I am puzzled.

Not anymore.

> Looking at wt-status.c "renamed: A -> B" will be shown only when A and B
> are similar enough *and* there is no A remaining in the final result, and
> in such a case, "modified: A" should not be shown.

Except for one case.

> Because git does not
> care how you created B (IOW, it does not matter if B was typed from
> scratch with copying and pasting, or created by copying and editing), the
> "renamed: A -> B" entry itself is not surprising nor look like a bug at
> all,...

The above observation of mine is correct, but I forgot that "git status"
(and the comment in the commit template from "git commit") is generated
internally with "diff-index -B -M".  So if

 (0) had A but not B in the HEAD commit;
 (1) you created B that is very similar to the original A; and
 (2) you modified A beyond recognition;

then git will say "A was modified and B was created by renaming A".

> ... but its presense at the same time as "modified: A" does feel very
> fishy.

So this is not fishy anymore.

This however makes me wonder if "diff-index -B -M" should say B is copied
(instead of being renamed) from A and A is modified in such a case.  I do
not think we would want to make such a change without thinking things,
through.

^ permalink raw reply

* Re: gitster is getting back to normal, slowly
From: Shawn O. Pearce @ 2008-10-10 15:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vej2omplg.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> I am back from a Japan trip to attend the one-year-after-funeral service
> for my stepfather and a vacation.  I did not plan to touch keyboard while
> in there but I ended up giving a 3-hour git talk to about two dozen git
> users (but none of the people I know from this list) and would-be users [*1*].

Welcome back.  ;-)
 
> I am going through my mailing list backlog, and it will probably be a few
> more days before I can fully catch up.  In the meantime, I've pulled from
> Shawn, who again did a wonderful job of being an interim maintainer [*2*],
> and pushed the result out, but I am afraind that my trees will lag behind
> his for a few more days.  If things go well, I expect to ask Shawn to stop
> applying patches at the end of Sunday and take it over from there.

Thanks.  I think I made two minor flubs; there's a git-svn commit with
an unwrapped commit message (0a1a1c8615 git-svn: call 'fatal' correctly)
and a "maint" topic branch was accidentally based on master.  It merged
to next that way, but I caught it and cherry-picked only the necessary
change into maint.

There is new content in my tree again today (just pushed there).
So we would still appreciate people tracking (and testing) my
branches for a bit.

I'll send out a "what's in" summary pair around Sunday to help ease
the transition for Junio, so he has a summary of what's where.

> *2* Thanks, Shawn.  Part of me is very much tempted to ask him being an
> interim maintainer for a few more weeks so that I can see how it feels
> like to be a contributor again, but I'd refrain from doing so ;-)

About 2 or 3 days into your absence I realized (again) how difficult
and time consuming your job of maintainer is.  I honestly do not know
how you do it, day in and day out, for as long as you have.

I think I've been tracking about 4-5 hours per day on git maintenance
lately.  Its a wonder you are able to even write patches.  My hat's
off to you, sir.  ;-)

Anyway, I'm more than happy to continue playing patch monkey for a
few weeks if you want to take a some time and just be a contributor
again.  I know you have a number of WIP branches that haven't been
able to move along... I've been skipping over them with Meta/RB. :-\

Either way we really should be thinking about freezing and rolling
a 1.6.0.3 soon-ish.

-- 
Shawn.

^ permalink raw reply

* Re: git confused by rename
From: Junio C Hamano @ 2008-10-10 15:44 UTC (permalink / raw)
  To: Michael P. Soulier; +Cc: git
In-Reply-To: <fb6605670810100741k7cff25c7y4df66834cf7482a0@mail.gmail.com>

"Michael P. Soulier" <msoulier@digitaltorque.ca> writes:

> #       renamed:    templates/scrc/index.html -> templates/scrc/cres_taps.html
> #       modified:   templates/scrc/index.html
>
> Looking at the last two lines here, there is obviously an issue.

I am puzzled.

Looking at wt-status.c "renamed: A -> B" will be shown only when A and B
are similar enough *and* there is no A remaining in the final result, and
in such a case, "modified: A" should not be shown.  Because git does not
care how you created B (IOW, it does not matter if B was typed from
scratch with copying and pasting, or created by copying and editing), the
"renamed: A -> B" entry itself is not surprising nor look like a bug at
all, but its presense at the same time as "modified: A" does feel very
fishy.

^ permalink raw reply

* gitster is getting back to normal, slowly
From: Junio C Hamano @ 2008-10-10 15:34 UTC (permalink / raw)
  To: git; +Cc: spearce

Here is a brief status update from me.

I am back from a Japan trip to attend the one-year-after-funeral service
for my stepfather and a vacation.  I did not plan to touch keyboard while
in there but I ended up giving a 3-hour git talk to about two dozen git
users (but none of the people I know from this list) and would-be users [*1*].

I am going through my mailing list backlog, and it will probably be a few
more days before I can fully catch up.  In the meantime, I've pulled from
Shawn, who again did a wonderful job of being an interim maintainer [*2*],
and pushed the result out, but I am afraind that my trees will lag behind
his for a few more days.  If things go well, I expect to ask Shawn to stop
applying patches at the end of Sunday and take it over from there.

[Footnote]

*1* If you happen to read/speak Japanese, you might be interested in these
links:

    http://d.hatena.ne.jp/takagimasahiro/20081008/1223413046
    http://d.hatena.ne.jp/kunit/20081004

*2* Thanks, Shawn.  Part of me is very much tempted to ask him being an
interim maintainer for a few more weeks so that I can see how it feels
like to be a contributor again, but I'd refrain from doing so ;-)

^ permalink raw reply

* Re: [PATCHv2] gitweb: refactor input parameters parse/validation
From: Shawn O. Pearce @ 2008-10-10 15:01 UTC (permalink / raw)
  To: Giuseppe Bilotta; +Cc: git, Jakub Narebski
In-Reply-To: <1223457985-27094-1-git-send-email-giuseppe.bilotta@gmail.com>

Giuseppe Bilotta <giuseppe.bilotta@gmail.com> wrote:
> Since input parameters can be obtained both from CGI parameters and
> PATH_INFO, we would like most of the code to be agnostic about the way
> parameters were retrieved. We thus collect all the parameters into the
> new %input_params hash, delaying validation after the collection is
> completed.

> @@ -672,6 +694,25 @@ sub href (%) {
>  ## ======================================================================
>  ## validation, quoting/unquoting and escaping
>  
> +sub validate_action {
> +	my $input = shift || return undef;
> +	return undef unless exists $actions{$action};
> +	return $action;
> +}

Shouldn't $action here be $input?

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] fix pread()'s short read in index-pack
From: Nicolas Pitre @ 2008-10-10 14:54 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Alex Riesen, Junio C Hamano, git
In-Reply-To: <20081010141756.GB8203@spearce.org>

On Fri, 10 Oct 2008, Shawn O. Pearce wrote:

> Alex Riesen <raa.lkml@gmail.com> wrote:
> > 2008/10/10 Nicolas Pitre <nico@cam.org>:
> > >
> > > Since v1.6.0.2~13^2~ the completion of a thin pack uses sha1write() for
> > > its ability to compute a SHA1 on the written data.  This also provides
> > > data buffering which, along with commit 92392b4a45, will confuse pread()
> > > whenever an appended object is 1) freed due to memory pressure because
> > > of the depth-first delta processing, and 2) needed again because it has
> > > many delta children, and 3) its data is still buffered by sha1write().
> > 
> > BTW, I run into complications trying to test the fix on Shawn's
> > master: conflicts,
> > and my trivial resolution wasn't good enough.
> 
> Huh.
> 
> Color me confused.  What the heck was the base version?
> 
> Near as I can tell Nico wrote this fix against a8032d12 (sha1write:
> don't copy full sized buffers).  This is in Junio's master.
> 
> But the changes conflict with what is in Junio's maint.  So the
> patch doesn't apply cleanly due to the sha1flush function's
> signature changing.
> 
> And the changes conflict with 9126f009 (fix openssl headers
> conflicting with custom SHA1 implementations), which is in my
> master, and was written by Nico.
> 
> ;-)

Whatever.

Your truly is still recovering from a knee surgery and therefore is 
literally on drugs.

> Anyway, its merged now.

Thanks.  The important thing is that it must be included in what would 
be v1.6.0.3.


Nicolas

^ permalink raw reply

* Re: Git newbie question: permissions
From: Samuel Lucas Vaz de Mello @ 2008-10-10 14:44 UTC (permalink / raw)
  To: git
In-Reply-To: <2008-10-09-23-41-14+trackit+sam@rfc1149.net>

Samuel Tardieu wrote:
> This should fix your current situation. The "sharedrepository = 1"
> will tell git to maintain a proper shared state in the future
> on objects it creates (i.e. mirror "user" permission to "group" ones).
>   
Is git-gc supposed to respect sharedrepository=1 and create 
group-writable files?
For me, it's recreating the files under logs/refs/heads with 644 
permissions.

BR,

 - Samuel

^ permalink raw reply

* git confused by rename
From: Michael P. Soulier @ 2008-10-10 14:41 UTC (permalink / raw)
  To: git

# On branch ajax-callrecording
# Changes to be committed:
#   (use "git reset HEAD <file>..." to unstage)
#
#       modified:   js/urls.py
#       modified:   js/views.py
#       modified:   scrc/urls.py
#       modified:   scrc/views.py
#       new file:   templates/js/scrc.js
#       modified:   templates/js/teleworker.js
#       renamed:    templates/scrc/index.html -> templates/scrc/cres_taps.html
#       modified:   templates/scrc/index.html

Looking at the last two lines here, there is obviously an issue.

I created a new cres_taps.html file, and modified the index.html file, and for
some reason Git thinks that I renamed index.html.

Looks like a bug, unless I'm missing something.

Mike
-- 
Michael P. Soulier <msoulier@digitaltorque.ca>
"Any intelligent fool can make things bigger and more complex... It takes a
touch of genius - and a lot of courage to move in the opposite direction."
--Albert Einstein

^ permalink raw reply

* Re: Really remove a file ?
From: Björn Steinbrink @ 2008-10-10 14:32 UTC (permalink / raw)
  To: marcreddist; +Cc: git
In-Reply-To: <8CAF8D313562BEE-1588-259E@webmail-da15.sysops.aol.com>

On 2008.10.10 05:38:25 -0400, marcreddist@aim.com wrote:
>> You'll probably also want to run "git gc" on your repo to actually
>> get rid of the huge object that was added (or does filter-branch do
>> this automatically?).
>
> I'm not sure it's required by git-filter-branch alone. In this case :
>
> git-gc saves almost 5% after the file deletion
>
> it saves 4.5% before the file deletion
>
> If I run git gc before and after the git filter-branch, it saves 4.5%  
> and then 0.2%.

Did you clear the refs/original namespace and your reflogs? Otherwise,
the huge object is most likely still referenced and thus won't get
pruned. Also, I usually prefer "git repack -adf" over "git gc" in such
situations, but that's probably just because I don't know the right
way to force "git gc" to immediately prune stuff just once.

But don't do the pruning until you're absolutely sure that you don't
require the old stuff anymore.

Björn

^ permalink raw reply

* RE: Implementing commit signing
From: Balasubramaniam, Arunan @ 2008-10-10 14:19 UTC (permalink / raw)
  To: spearce; +Cc: Git Mailing List


Shawn O. Pearce wrote:
> Well, the idea was to sign the diff, but in a way that would
> reasonably allow it to be applied with limited fuzz, such as what
> git-apply would accept.  Thus signed changes could be emailed out
> by git format-patch and git send-email, and applied with git am,
> and the signature is still valid so long as the committer didn't
> mess with the patch.
> 
> Obviously if a commit was reverted and then reapplied again later,
> yes, the signature on the reapply may actually be valid, as the
> parents weren't taken into consideration.

I have realized I have been mixing things up; I was talking about
authors but mostly thinking about committers. The solution you
proposed tracked the author, so my question about it being
reapplied was a bit meaningless since the signature did not say
anything about the committer. My apologies.

As with having separate author and committer fields, would it make
sense to allow author and committer signatures? Just leaving aside
the issue of how much text it would take up for now, the committer's
signature could be created as tag signatures are, while the
author's could be as you originally described and would track the
content. If it reapplied to a different parent, then the signature
would be OK, but the committer would be listed as the one who
reapplied it. This would match how rebase works.

A project operating like the Linux kernel with patches only pulled
and emailed might use the author signature without the committer
one. I don't think there is any case for committer signing only.

If you ever merged or rebased, you automatically strip any committer
signature. If the operation succeeds, and the diff against the
parent matches the original diff, you could reuse the original
author signature.

Arunan


"Misys" is the trade name for Misys plc (registered in England and Wales). Registration Number: 01360027. Registered office: One Kingdom Street, London W2 6BL, United Kingdom. For a list of Misys group operating companies please go to http://www.misys.com/html/about_us/group_operating_companies/. This email and any attachments have been scanned for known viruses using multiple scanners. This email message is intended for the named recipient only. It may be privileged and/or confidential. If you are not the named recipient of this email please notify us immediately and do not copy it or use it for any purpose, nor disclose its contents to any other person. This email does not constitute the commencement of legal relations between you and Misys plc. Please refer to the executed contract betw
 een you and the relevant member of the Misys group for the identity of the contracting party with which you are dealing. 

^ permalink raw reply

* Re: [PATCH] fix pread()'s short read in index-pack
From: Shawn O. Pearce @ 2008-10-10 14:17 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Nicolas Pitre, Junio C Hamano, git
In-Reply-To: <81b0412b0810092352n2a9cb5d2m6a0550354af0b397@mail.gmail.com>

Alex Riesen <raa.lkml@gmail.com> wrote:
> 2008/10/10 Nicolas Pitre <nico@cam.org>:
> >
> > Since v1.6.0.2~13^2~ the completion of a thin pack uses sha1write() for
> > its ability to compute a SHA1 on the written data.  This also provides
> > data buffering which, along with commit 92392b4a45, will confuse pread()
> > whenever an appended object is 1) freed due to memory pressure because
> > of the depth-first delta processing, and 2) needed again because it has
> > many delta children, and 3) its data is still buffered by sha1write().
> 
> BTW, I run into complications trying to test the fix on Shawn's
> master: conflicts,
> and my trivial resolution wasn't good enough.

Huh.

Color me confused.  What the heck was the base version?

Near as I can tell Nico wrote this fix against a8032d12 (sha1write:
don't copy full sized buffers).  This is in Junio's master.

But the changes conflict with what is in Junio's maint.  So the
patch doesn't apply cleanly due to the sha1flush function's
signature changing.

And the changes conflict with 9126f009 (fix openssl headers
conflicting with custom SHA1 implementations), which is in my
master, and was written by Nico.

;-)

Anyway, its merged now.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] git-daemon: Worked around uclibc buffer problem
From: Lars Stoltenow @ 2008-10-10 13:33 UTC (permalink / raw)
  To: git
In-Reply-To: <alpine.LFD.2.00.0810092010370.26244@xanadu.home>

On Thu, Oct 09, 2008 at 08:16:03PM -0400, Nicolas Pitre wrote:
> What is your client version?  From v1.6.0.2 and later (with commit 
> 6b9c42b4da) this shouldn't matter as things are properly handled on the 
> receiving end.

Yah, looks good. I did test it with 1.6.0.1 and some 1.5.* versions on
the client side only. With 1.6.0.2, it works with the unfixed daemon too.

^ permalink raw reply

* Git User's Survey 2008 - last call!
From: Jakub Narebski @ 2008-10-10 12:39 UTC (permalink / raw)
  To: git

It is now last day of Git User's Survey 2008.  So if you didn't submit 
your replies, please do it now at
  http://tinyurl.com/GitSurvey2008
  http://www.survs.com/survey?id=M3PIVU72&channel=2WXE4BVTW8

Thanks in advance
-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: merge hook
From: Johannes Schindelin @ 2008-10-10 12:30 UTC (permalink / raw)
  To: Erik Colson; +Cc: git
In-Reply-To: <20081010102603.GC303@Mac2.local>

Hi,

On Fri, 10 Oct 2008, Erik Colson wrote:

> On Fri, Oct 10, 2008 at 12:07:16PM +0200, Johannes Schindelin wrote:
> 
> > I guess that you want to disallow merging in changes from people who 
> > changed files that are none of their business?  If so, it would be 
> > nice to contribute the hook as a patch to contrib/.
> 
> wrong guess ;) The purpose is to check if source code still has debug 
> code in it. Our policy is to have it removed before reaching master, but 
> checking by hand is way too buggy ;)

Heh.  Still, if it is not too specific, would be nice to have it in 
contrib/.  So I can point other people to it ;-)

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH (GITK) v2 4/4] gitk: Implement batch lookup and caching of encoding attrs.
From: Alexander Gavrilov @ 2008-10-10 12:22 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: git, Johannes Sixt
In-Reply-To: <18671.16658.667581.499095@cargo.ozlabs.ibm.com>

On Fri, Oct 10, 2008 at 3:48 PM, Paul Mackerras <paulus@samba.org> wrote:
> Alexander Gavrilov writes:
>
>> When the diff contains thousands of files, calling git-check-attr
>> once per file is very slow. With this patch gitk does attribute
>> lookup in batches of 30 files while reading the diff file list,
>> which leads to a very noticeable speedup.
>
> Why only 30 at a time?  The logic would be simpler if cache_gitattr
> just did all the paths in one call to git check-attr, and it should be
> able to cope with 1000 paths in one call, I would think, which is the
> most that gettreediffline will give it.

OS-enforced command-line size limit on Windows is 32K. Cramming in
1000 paths would leave only 32 characters for each path.

> Also, I wonder why we now have two levels of caching of the encoding
> attribute.  Your patch 1/4 introduced path_encoding_cache, which was
> fine, but now we have path_attr_cache as well, which seems to me to
> serve exactly the same function since the encoding is the only
> attribute we ever ask about.  Surely we don't need both caches?

If the (git-gui) patch that reimplements the tcl_encoding procedure is
applied, we may drop the path_encoding_cache. Current implementation
is too slow for batch lookup, especially if the encoding is actually
not supported, and without the cache the lookup would be done on every
loading of a diff.

> Even with this batching, I am a bit concerned that adding the encoding
> support might make things noticeably slower for people who don't need
> any encoding support (which would be the majority, I think).  We may
> end up needing an option to turn off the checking of the encoding
> attribute.

I hope that most diffs don't contain thousands of files at once. And
actual huge diffs are likely to be relatively slow to load anyway.

Alexander

^ permalink raw reply

* Re: [PATCH (GITK) v2 4/4] gitk: Implement batch lookup and caching of encoding attrs.
From: Paul Mackerras @ 2008-10-10 11:48 UTC (permalink / raw)
  To: Alexander Gavrilov; +Cc: git, Johannes Sixt
In-Reply-To: <1222772422-28020-5-git-send-email-angavrilov@gmail.com>

Alexander Gavrilov writes:

> When the diff contains thousands of files, calling git-check-attr
> once per file is very slow. With this patch gitk does attribute
> lookup in batches of 30 files while reading the diff file list,
> which leads to a very noticeable speedup.

Why only 30 at a time?  The logic would be simpler if cache_gitattr
just did all the paths in one call to git check-attr, and it should be
able to cope with 1000 paths in one call, I would think, which is the
most that gettreediffline will give it.

Also, I wonder why we now have two levels of caching of the encoding
attribute.  Your patch 1/4 introduced path_encoding_cache, which was
fine, but now we have path_attr_cache as well, which seems to me to
serve exactly the same function since the encoding is the only
attribute we ever ask about.  Surely we don't need both caches?

Even with this batching, I am a bit concerned that adding the encoding
support might make things noticeably slower for people who don't need
any encoding support (which would be the majority, I think).  We may
end up needing an option to turn off the checking of the encoding
attribute.

Paul.

^ permalink raw reply

* [PATCH] rebase -i: do not fail when there is no commit to cherry-pick
From: Johannes Schindelin @ 2008-10-10 11:42 UTC (permalink / raw)
  To: Leonardo Sobral Cunha, gitster; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0810101319150.22125@pacific.mpi-cbg.de.mpi-cbg.de>


In case there is no commit to apply (for example because you rebase to
upstream and all your local patches have been applied there), do not
fail.  The non-interactive rebase already behaves that way.

Do this by introducing a new command, "noop", which is substituted for
an empty commit list, so that deleting the commit list can still abort
as before.

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

	On Fri, 10 Oct 2008, Johannes Schindelin wrote:

	> Maybe we should add a new command "noop" which is automatically 
	> inserted when there is no other command?

	And this implements that.

 git-rebase--interactive.sh    |    3 ++-
 t/t3404-rebase-interactive.sh |   11 +++++++++++
 2 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 076414a..49a6b5c 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -271,7 +271,7 @@ do_next () {
 		"$DOTEST"/amend || exit
 	read command sha1 rest < "$TODO"
 	case "$command" in
-	'#'*|'')
+	'#'*|''|noop)
 		mark_action_done
 		;;
 	pick|p)
@@ -583,6 +583,7 @@ first and then run 'git rebase --continue' again."
 			--abbrev=7 --reverse --left-right --cherry-pick \
 			$UPSTREAM...$HEAD | \
 			sed -n "s/^>/pick /p" > "$TODO"
+		test -s "$TODO" || echo noop >> "$TODO"
 		cat >> "$TODO" << EOF
 
 # Rebase $SHORTUPSTREAM..$SHORTHEAD onto $SHORTONTO
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index e0ded19..7d10a27 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -419,4 +419,15 @@ test_expect_success 'rebase with a file named HEAD in worktree' '
 
 '
 
+test_expect_success 'do "noop" when there is nothing to cherry-pick' '
+
+	git checkout -b branch4 HEAD &&
+	GIT_EDITOR=: git commit --amend \
+		--author="Somebody else <somebody@else.com>" 
+	test $(git rev-parse branch3) != $(git rev-parse branch4) &&
+	git rebase -i branch3 &&
+	test $(git rev-parse branch3) = $(git rev-parse branch4)
+
+'
+
 test_done
-- 
1.6.0.2.713.g3dcb0

^ permalink raw reply related

* Re: Different behaviour of rebase -i
From: Johannes Schindelin @ 2008-10-10 11:40 UTC (permalink / raw)
  To: Leonardo Sobral Cunha; +Cc: git
In-Reply-To: <6beb92080810091204n6df4338fs3b182194a8d6454e@mail.gmail.com>

Hi,

On Thu, 9 Oct 2008, Leonardo Sobral Cunha wrote:

> While doing a git rebase with my upstream branch ahead of my local 
> branch, first git rewinds my local branch to point to the upstream and 
> then prints "Nothing to do". As expected, my local branch ends pointing 
> to the upstream head.
> 
> But when I use the same command with the same branches using git rebase 
> -i, nothing happens and my local branch does not point to upstream (the 
> behaviour is different).
> 
> Is this different behaviour of rebase -i expected or is this a bug? This 
> is still happening in git version 1.6.0.2.443.g52e83

This is by design.  You need to have a way to stop rebasing in the editor.  
For example when you realize that you passed the wrong upstream or 
something.

The most intuitive way (I thought) is to behave the same as "git commit": 
if you do not want to continue, just delete everything.

Now, I can see that it is unconvenient.

A workaround would be to add a line "bla" and when rebase complains about 
the unknown command, continue with "git rebase --skip".

Maybe we should add a new command "noop" which is automatically inserted 
when there is no other command?

Ciao,
Dscho

^ 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