Git development
 help / color / mirror / Atom feed
* Re: Re: Re: Re: Remove need to untrack before tracking new branch
From: Martin Schlemmer @ 2005-04-14  9:40 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <20050414091106.GX25711@pasky.ji.cz>

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

(PS, can you check the fact that your mail client keeps on adding a 'Re:
' ...)

On Thu, 2005-04-14 at 11:11 +0200, Petr Baudis wrote:
> Please trim the replied mails a bit, snipping old and irrelevant parts.
> This is insane. :-)
> 
> Dear diary, on Thu, Apr 14, 2005 at 10:38:25AM CEST, I got a letter
> where Martin Schlemmer <azarah@nosferatu.za.org> told me that...
> ..snip..
> > Normalize show-diff output, add --update-modes target to update-cache,
> > and make sure we only show real changes after changing the tracked
> > branch, as well as update the file modes according to the cache.
> 
> I'm lost. Why do you do --update-modes? That makes no sense to me.
> You introduce them to the cache out-of-order w.r.t. commits, that means
> in the normal git usage they are already unrevertable.
> 

Right, afterwards I thought I did add it to the wrong place.


> What are you trying to do? Mode changes _are_ real changes. You _don't_
> want to silence them. What you want is to even show them more explicitly
> in show-diff.
> 

No, you do not understand.  If you actually change the mode, it will
show.  What now happens, is that say I track the 'linus' branch, then
untrack, and then track 'pasky' again, the Patches will be applied, but
not the mode changes which are stored in the cache ...  Let me show you:

-----
$ ls -l $(./show-diff -s | cut -d: -f1)
-rw-r--r--  1 root root  168 Apr 14 11:33 commit-id
-rw-r--r--  1 root root 2213 Apr 14 11:33 git
-rw-r--r--  1 root root 1168 Apr 14 11:33 gitXnormid.sh
-rw-r--r--  1 root root  403 Apr 14 11:33 gitadd.sh
-rw-r--r--  1 root root  844 Apr 14 11:33 gitaddremote.sh
-rw-r--r--  1 root root 1899 Apr 14 11:33 gitapply.sh
-rw-r--r--  1 root root  479 Apr 14 11:33 gitcancel.sh
-rw-r--r--  1 root root 2512 Apr 14 11:33 gitcommit.sh
-rw-r--r--  1 root root 2152 Apr 14 11:33 gitdiff-do
-rw-r--r--  1 root root  819 Apr 14 11:33 gitdiff.sh
-rw-r--r--  1 root root  717 Apr 14 11:33 gitexport.sh
-rw-r--r--  1 root root  524 Apr 14 11:33 gitlog.sh
-rw-r--r--  1 root root  228 Apr 14 11:33 gitls.sh
-rw-r--r--  1 root root  904 Apr 14 11:33 gitlsobj.sh
-rw-r--r--  1 root root  665 Apr 14 11:33 gitmerge.sh
-rw-r--r--  1 root root 2044 Apr 14 11:33 gitpull.sh
-rw-r--r--  1 root root  433 Apr 14 11:33 gitrm.sh
-rw-r--r--  1 root root  614 Apr 14 11:33 gittag.sh
-rw-r--r--  1 root root 2272 Apr 14 11:33 gittrack.sh
-rw-r--r--  1 root root  284 Apr 14 11:33 parent-id
-rw-r--r--  1 root root  177 Apr 14 11:33 tree-id
-----

(Note no 'x' bit ...)

And that is _after_ doing:

 $ git track linus; git track

So basically the modes that are stored in the cache are not applied ...
Although, yes, I prob should add the relevant code to checkout-cache.

> The --refreshes are fine.
> 
> > Signed-off-by: Martin Schlemmer <azarah@nosferatu.za.org>
> > 
> > gitcancel.sh:  ec58f7444a42cd3cbaae919fc68c70a3866420c0
> > --- ec58f7444a42cd3cbaae919fc68c70a3866420c0/gitcancel.sh
> > +++ uncommitted/gitcancel.sh
> > @@ -12,7 +12,8 @@
> > 
> >  # FIXME: Does not revert mode changes!
> > 
> > -show-diff | patch -p0 -R
> > +show-diff | patch -p1 -R
> >  rm -f .git/add-queue .git/rm-queue .git/merged
> > 
> > -update-cache --refresh
> > +# --update-modes need to be before --refresh
> > +update-cache --update-modes --refresh
> 
> Here, e.g., you should do the very opposite - change the modes back to
> how are they in the cache.
> 

Uhm, that is what it did ...  Like I said above, the wrong place though
to add the code to update-cache.

> > show-diff.c:  a531ca4078525d1c8dcf84aae0bfa89fed6e5d96
> > --- a531ca4078525d1c8dcf84aae0bfa89fed6e5d96/show-diff.c
> > +++ uncommitted/show-diff.c
> > @@ -5,13 +5,18 @@
> >   */
> >  #include "cache.h"
> > 
> > -static void show_differences(char *name,
> > +static void show_differences(struct cache_entry *ce,
> >         void *old_contents, unsigned long long old_size)
> >  {
> >         static char cmd[1000];
> > +       static char sha1[41];
> > +       int n;
> >         FILE *f;
> > 
> > -       snprintf(cmd, sizeof(cmd), "diff -L %s -u -N  - %s", name, name);
> > +       for (n = 0; n < 20; n++)
> > +               snprintf(&(sha1[n*2]), 3, "%02x", ce->sha1[n]);
> > +       snprintf(cmd, sizeof(cmd), "diff -L %s/%s -L uncommitted/%s -u -N  - %s",
> > +               sha1, ce->name, ce->name, ce->name);
> 
> The "directory" sha1 is the sha1 of the tree, not of the particular
> file - that one is in the "attributes" list (parentheses after the
> filename), together with mode.
> 

Does it really matter?  It is more just to get the patch prefix right,
and I did it as it went nicely with the printed:

----
show-diff.c:  a531ca4078525d1c8dcf84aae0bfa89fed6e5d96
----

for example ...

> >         f = popen(cmd, "w");
> >         if (old_size)
> >                 fwrite(old_contents, old_size, 1, f);
> > @@ -99,7 +104,7 @@
> >                         continue;
> > 
> >                 new = read_sha1_file(ce->sha1, type, &size);
> > -               show_differences(ce->name, new, size);
> > +               show_differences(ce, new, size);
> >                 free(new);
> >         }
> >         return 0;
> > update-cache.c:  62d0a6c41560d40863c44599355af10d9e089312
> > --- 62d0a6c41560d40863c44599355af10d9e089312/update-cache.c
> > +++ uncommitted/update-cache.c
> > @@ -210,6 +210,39 @@
> >         }
> >  }
> > 
> > +static struct cache_entry *update_file_mode(struct cache_entry *ce)
> > +{
> > +       struct stat st;
> > +       int changed;
> > +
> > +       if (stat(ce->name, &st) < 0)
> > +               return NULL;
> > +
> > +       changed = cache_match_stat(ce, &st);
> > +       if (!changed)
> > +               return ce;
> > +
> > +       if (changed & MODE_CHANGED)
> > +               if (chmod(ce->name, ce->st_mode))
> > +                       return NULL;
> > +
> > +       return ce;
> > +}
> > +
> > +static void update_modes(void)
> > +{
> > +       int i;
> > +
> > +       for (i = 0; i < active_nr; i++) {
> > +               struct cache_entry *ce = active_cache[i];
> > +
> > +               if (!update_file_mode(ce)) {
> > +                       printf("%s: needs update\n", ce->name);
> > +                       continue;
> > +               }
> > +       }
> > +}
> > +
> >  /*
> >   * We fundamentally don't like some paths: we don't want
> >   * dot or dot-dot anywhere, and in fact, we don't even want
> 
> Oh. So you are actually doing the _opposite_! But that's very wrong too.
> update-cache is for the direction tree -> index. For the opposite
> direction, you should use checkout-cache.
> 

Right, will do.

> > @@ -282,6 +315,10 @@
> >                                 refresh_cache();
> >                                 continue;
> >                         }
> > +                       if (!strcmp(path, "--update-modes")) {
> > +                               update_modes();
> > +                               continue;
> > +                       }
> >                         die("unknown option %s", path);
> >                 }
> >                 if (!verify_path(path)) {
> > 
> > 
> > -- 
> > Martin Schlemmer
> > 
> 


-- 
Martin Schlemmer


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: Date handling.
From: David Woodhouse @ 2005-04-14  9:31 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.58.0504140153230.7211@ppc970.osdl.org>

On Thu, 2005-04-14 at 02:00 -0700, Linus Torvalds wrote:
> I do like text output, but if it is painful, the "unix seconds" format is 
> certainly a hell of a lot simpler. And quite frankly, if we change it, we 
> might as well just change it all the way. So I'd almost prefer (1).

Text _output_ is easy to generate; we don't need to store text in the
database for that. So I've changed my mind -- I prefer (1) too.

-- 
dwmw2


^ permalink raw reply

* Re: Re: Re: Re: Remove need to untrack before tracking new branch
From: Petr Baudis @ 2005-04-14  9:11 UTC (permalink / raw)
  To: Martin Schlemmer; +Cc: git
In-Reply-To: <1113467905.23299.81.camel@nosferatu.lan>

Please trim the replied mails a bit, snipping old and irrelevant parts.
This is insane. :-)

Dear diary, on Thu, Apr 14, 2005 at 10:38:25AM CEST, I got a letter
where Martin Schlemmer <azarah@nosferatu.za.org> told me that...
..snip..
> Normalize show-diff output, add --update-modes target to update-cache,
> and make sure we only show real changes after changing the tracked
> branch, as well as update the file modes according to the cache.

I'm lost. Why do you do --update-modes? That makes no sense to me.
You introduce them to the cache out-of-order w.r.t. commits, that means
in the normal git usage they are already unrevertable.

What are you trying to do? Mode changes _are_ real changes. You _don't_
want to silence them. What you want is to even show them more explicitly
in show-diff.

The --refreshes are fine.

> Signed-off-by: Martin Schlemmer <azarah@nosferatu.za.org>
> 
> gitcancel.sh:  ec58f7444a42cd3cbaae919fc68c70a3866420c0
> --- ec58f7444a42cd3cbaae919fc68c70a3866420c0/gitcancel.sh
> +++ uncommitted/gitcancel.sh
> @@ -12,7 +12,8 @@
> 
>  # FIXME: Does not revert mode changes!
> 
> -show-diff | patch -p0 -R
> +show-diff | patch -p1 -R
>  rm -f .git/add-queue .git/rm-queue .git/merged
> 
> -update-cache --refresh
> +# --update-modes need to be before --refresh
> +update-cache --update-modes --refresh

Here, e.g., you should do the very opposite - change the modes back to
how are they in the cache.

> show-diff.c:  a531ca4078525d1c8dcf84aae0bfa89fed6e5d96
> --- a531ca4078525d1c8dcf84aae0bfa89fed6e5d96/show-diff.c
> +++ uncommitted/show-diff.c
> @@ -5,13 +5,18 @@
>   */
>  #include "cache.h"
> 
> -static void show_differences(char *name,
> +static void show_differences(struct cache_entry *ce,
>         void *old_contents, unsigned long long old_size)
>  {
>         static char cmd[1000];
> +       static char sha1[41];
> +       int n;
>         FILE *f;
> 
> -       snprintf(cmd, sizeof(cmd), "diff -L %s -u -N  - %s", name, name);
> +       for (n = 0; n < 20; n++)
> +               snprintf(&(sha1[n*2]), 3, "%02x", ce->sha1[n]);
> +       snprintf(cmd, sizeof(cmd), "diff -L %s/%s -L uncommitted/%s -u -N  - %s",
> +               sha1, ce->name, ce->name, ce->name);

The "directory" sha1 is the sha1 of the tree, not of the particular
file - that one is in the "attributes" list (parentheses after the
filename), together with mode.

>         f = popen(cmd, "w");
>         if (old_size)
>                 fwrite(old_contents, old_size, 1, f);
> @@ -99,7 +104,7 @@
>                         continue;
> 
>                 new = read_sha1_file(ce->sha1, type, &size);
> -               show_differences(ce->name, new, size);
> +               show_differences(ce, new, size);
>                 free(new);
>         }
>         return 0;
> update-cache.c:  62d0a6c41560d40863c44599355af10d9e089312
> --- 62d0a6c41560d40863c44599355af10d9e089312/update-cache.c
> +++ uncommitted/update-cache.c
> @@ -210,6 +210,39 @@
>         }
>  }
> 
> +static struct cache_entry *update_file_mode(struct cache_entry *ce)
> +{
> +       struct stat st;
> +       int changed;
> +
> +       if (stat(ce->name, &st) < 0)
> +               return NULL;
> +
> +       changed = cache_match_stat(ce, &st);
> +       if (!changed)
> +               return ce;
> +
> +       if (changed & MODE_CHANGED)
> +               if (chmod(ce->name, ce->st_mode))
> +                       return NULL;
> +
> +       return ce;
> +}
> +
> +static void update_modes(void)
> +{
> +       int i;
> +
> +       for (i = 0; i < active_nr; i++) {
> +               struct cache_entry *ce = active_cache[i];
> +
> +               if (!update_file_mode(ce)) {
> +                       printf("%s: needs update\n", ce->name);
> +                       continue;
> +               }
> +       }
> +}
> +
>  /*
>   * We fundamentally don't like some paths: we don't want
>   * dot or dot-dot anywhere, and in fact, we don't even want

Oh. So you are actually doing the _opposite_! But that's very wrong too.
update-cache is for the direction tree -> index. For the opposite
direction, you should use checkout-cache.

> @@ -282,6 +315,10 @@
>                                 refresh_cache();
>                                 continue;
>                         }
> +                       if (!strcmp(path, "--update-modes")) {
> +                               update_modes();
> +                               continue;
> +                       }
>                         die("unknown option %s", path);
>                 }
>                 if (!verify_path(path)) {
> 
> 
> -- 
> Martin Schlemmer
> 





-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
C++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor

^ permalink raw reply

* Re: Date handling.
From: Linus Torvalds @ 2005-04-14  9:12 UTC (permalink / raw)
  To: David Woodhouse; +Cc: git
In-Reply-To: <Pine.LNX.4.58.0504140153230.7211@ppc970.osdl.org>



On Thu, 14 Apr 2005, Linus Torvalds wrote:
> 
> Yeah, I think this is the right thing to do. I can change "commit" to do 
> it.

I take that back. I'd be much happier with you doing and testing it, 
because now I'm crashing.

		Linus

^ permalink raw reply

* Re: Merge with git-pasky II.
From: Linus Torvalds @ 2005-04-14  9:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Petr Baudis, Christopher Li, git
In-Reply-To: <7v64ypsqev.fsf@assigned-by-dhcp.cox.net>



On Thu, 14 Apr 2005, Junio C Hamano wrote:
> 
> I have to handle the following cases.  I think I currently do
> wrong things to them:
> 
>   5.1a both head modify to the same thing.
>   5.1b one head removes, the other does not do anything.
>   5.1c both head remove.
>   5.3 one head removes, the other head modifies.

There's another interesting set of cases: one side creates a file, and the
other one creates a directory. 

> I am not sure what to do with 5.3.

My very _strong_ preference is to just inform the user about a merge that
cannot be performed, and not let it be automated. BIG warning, with some 
way for the user to specify the end result.

The thing is, these are pretty rare cases. But in order to make people 
feel good about the _common_ case, it's important that they feel safe 
about the rare one.

Put another way: if git tells me when it can't do something (with some
specificity), I can then fix the situation up and try again. I might curse
a while, and maybe it ends up being so common that I might even automate
it, but at least I'll be able to trust the end result.

In contrast, if git does something that _may_ be nonsensical, then I'll
worry all the time, and not trust git. That's much worse than an 
occasional curse.

So the rule should be: only merge when it's "obviously the right thing".  
If it's not obvious, the merge should _not_ try to guess what the right
thing is. It's much better to fail loudly.

(That's especially true early on. There may be cases that end up being
obvious after some usage. But I'd rather find them by having git be too
stupid, than find out the hard way that git lost some data because it
thought it was ok to remove a file that had been modified)

			Linus

^ permalink raw reply

* Re: Date handling.
From: Linus Torvalds @ 2005-04-14  9:00 UTC (permalink / raw)
  To: David Woodhouse; +Cc: git
In-Reply-To: <1113466592.12012.192.camel@baythorne.infradead.org>



On Thu, 14 Apr 2005, David Woodhouse wrote:
> 
> I see two possible solutions:
> 	1. Just store seconds-since-GMT-epoch and if we really want, the
> 	   timezone as auxiliary information.

Yeah, I think this is the right thing to do. I can change "commit" to do 
it.

I used to think that the date was purely a "enforced comment" (like the
committer info is, as far as git is concerned), which is why I used the 
simple textual representation. But yes, when I wrote that "rev-tree" thing 
I did curse that and consider just changing it.

It's still just technically a "hint", since time isn't synchronized in any 
way (and in a distributed system, time _cannot_ be synchronized). But 
it's a useful hint, so ..

> 	2. Store dates in RFC2822 form.
> 
> Unless someone convincingly expresses a preference before I get to work
> and start playing with it, I'll implement the latter.

I do like text output, but if it is painful, the "unix seconds" format is 
certainly a hell of a lot simpler. And quite frankly, if we change it, we 
might as well just change it all the way. So I'd almost prefer (1).

But "He who does the work gets to choose the implementation". And I do
agree that this is a bad format decision, and that we should change it. It 
shouldn't even be that painful. Only "rev-tree" cares, and even rev-tree 
doesn't care _that_ deeply.

		Linus

^ permalink raw reply

* Plug memory leak in update-cache.c
From: Martin Schlemmer @ 2005-04-14  8:53 UTC (permalink / raw)
  To: GIT Mailing Lists


[-- Attachment #1.1: Type: text/plain, Size: 641 bytes --]

Hi,

Might not be that an big an issue as it should be freed on exit, but
might cause problems with big trees.

----

Plug memory leak in update-cache.c.

Signed-off-by: Martin Schlemmer <azarah@nosferatu.za.org>

update-cache.c:  22f3ccd47db4f0888901109a8cbf883d272d1cba
--- 22f3ccd47db4f0888901109a8cbf883d272d1cba/update-cache.c
+++ uncommitted/update-cache.c
@@ -202,6 +202,7 @@
                        printf("%s: needs update\n", ce->name);
                        continue;
                }
+               free(active_cache[i]);
                active_cache[i] = new;
        }
 }


-- 
Martin Schlemmer


[-- Attachment #1.2: git-plug-leak-in-update_cache.patch --]
[-- Type: text/x-patch, Size: 302 bytes --]

update-cache.c:  22f3ccd47db4f0888901109a8cbf883d272d1cba
--- 22f3ccd47db4f0888901109a8cbf883d272d1cba/update-cache.c
+++ uncommitted/update-cache.c
@@ -202,6 +202,7 @@
 			printf("%s: needs update\n", ce->name);
 			continue;
 		}
+		free(active_cache[i]);
 		active_cache[i] = new;
 	}
 }

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: Merge with git-pasky II.
From: Junio C Hamano @ 2005-04-14  8:39 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Petr Baudis, Christopher Li, git
In-Reply-To: <Pine.LNX.4.58.0504140051550.7211@ppc970.osdl.org>

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> But I'm really happy that you seem to have implemented my first 
LT> suggestion and I seem to have been wasting my time. 

Thanks for the kind words.

>> 5. for each path involved:
>> 
>> 5.0 if neither heads change it, leave it as is;
>> 5.1 if only one head changes a path and the other does not, just
>> get the changed version;
>> 5.2 if both heads change it, check all three out and run merge.

LT> You missed one case: 

LT>     5.0.1 if both heads change it to the same thing, take the new thing

LT> but maybe you counted that as 5.0 (it _should_ fall out automatically from
LT> the fact that "diff-tree" between the two destination trees shows no
LT> difference for such a file).

Actually I am not handling that.  It really is 5.1a---the exact
same code path as 5.1 can be used for this case, and as you
point out it is really a quite important optimization.

I have to handle the following cases.  I think I currently do
wrong things to them:

  5.1a both head modify to the same thing.
  5.1b one head removes, the other does not do anything.
  5.1c both head remove.
  5.3 one head removes, the other head modifies.

Handling of 5.1a, 5.1b and 5.1c are obvious.

  5.1a Update dircache to the same new thing.  Without -f or -o
       flag do not touch ,,merge-temp/. directory; with -f or
       -o, leave the new file in ,,merge-temp/.

  5.1b Remove the path from dircache and do not have the file in
       ,,merge-temp/. directory regardless of -f or -o flags.

  5.1c Same as 5.1b

I am not sure what to do with 5.3.  My knee-jerk reaction is to
leave the modified result in ,,merge-temp/$path~ without
touching dircache.  If the merger wants to pick it up, he can
rename $path~ to $path temporarily, run show-diff on it (I think
giving an option to show-diff to specify paths would be helpful
for this workflow), to decide if he wants to keep the file or
not.  Suggestions?


^ permalink raw reply

* Re: Re: Re: Remove need to untrack before tracking new branch
From: Martin Schlemmer @ 2005-04-14  8:38 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <1113467335.23299.77.camel@nosferatu.lan>


[-- Attachment #1.1: Type: text/plain, Size: 8842 bytes --]

On Thu, 2005-04-14 at 10:28 +0200, Martin Schlemmer wrote:
> On Thu, 2005-04-14 at 08:55 +0200, Martin Schlemmer wrote:
> > On Thu, 2005-04-14 at 00:19 +0200, Petr Baudis wrote:
> > > Dear diary, on Wed, Apr 13, 2005 at 02:15:37PM CEST, I got a letter
> > > where Martin Schlemmer <azarah@nosferatu.za.org> told me that...
> > > > On Wed, 2005-04-13 at 11:26 +0200, Petr Baudis wrote:
> > > >> > Dear diary, on Wed, Apr 13, 2005 at 10:41:12AM CEST, I got a letter
> > > > > where Martin Schlemmer <azarah@nosferatu.za.org> told me that...
> > > > > > On Wed, 2005-04-13 at 09:54 +0200, Petr Baudis wrote:
> > > > > > PS: not having looked deeper yet, why does fsck-cache always find
> > > > > > unreferenced blobs/commits (no matter what tree is tracked, they stay
> > > > > > the same) ?  And trying to remove them leads to more, which leads to an
> > > > > > empty .git/opjects/ =)  Also, leading to this, will adding an option to
> > > > > > remove disconnected commits/blobs from local commits (that was
> > > > > > disconnected with a pull) be a viable option to add?
> > > > > 
> > > > > fsck-cache is concerned only by the objects database, so all the HEADs
> > > > > are unreferenced commits too. This is a right thing, the HEAD tracking
> > > > > should stay purely in the scripts - if we want to make fsck-cache
> > > > > smarter about that, we should implement git fsck or something.
> > > > > 
> > > > > Killing unreferenced blobs should be safe, I think.
> > > > > 
> > > > > > First, about the 'git diff' thing I asked yesterday .. what I meant, was
> > > > > > should it actually output this:
> > > > > > 
> > > > > > ----
> > > > > > COPYING:  fe2a4177a760fd110e78788734f167bd633be8de 33
> > > > > > Makefile:  929aa49a3dbe683ad52094099797bc636a7949a6 33
> > > > > > README:  46c6a9ea48ddd1dda45ca585f49975a6869ffe51 33
> > > > > > ...
> > > > > > ----
> > > > > > 
> > > > > > Shouldn't it just show actual changes?
> > > > > 
> > > > > This is an actual change. It's just that it's a change to metadata
> > > > > (somewhat esotherically described by the "33"), not the file contents.
> > > > > 
> > > > > BTW, git diff does actually something completely different from git diff
> > > > > with any arguments. It diffs to the directory cache, not to any tree! It
> > > > > just wraps show-diff, which has also a different output format (not
> > > > > outputting "git diffs"). The worst thing is that it requires a different
> > > > > -p option to apply. Someone should purge this wart, I think.
> > > > > 
> > > > 
> > > > Check applied patch (also in the new output).
> > > 
> > > Please send patches inline and properly signed off.
> > > 
> > 
> > The new evo have a bad habit of screwing the tabs, but sure.
> > 
> > > > > > Also on the same note .. should 'git ci' without listed files to be
> > > > > > committed, really add a reference to all files as it currently do in the
> > > > > > commit/blob/whatever info, instead of just the changed/added files (see
> > > > > > the git-seperate-dir.patch you have not yet commented on for reference)?
> > > > > 
> > > > > ...
> > > > > 
> > > > 
> > > > Patch will also resolve this.
> > > 
> > > Your patch is bad - it removes the pure metadata changes, but you
> > > definitively do not want to do that! If you are annoyed by meaningless
> > > time changes etc, do update-cache --refresh. Ignoring mode changes is a
> > > pure disaster.
> > > 
> > 
> > Ahh - and there was light.  I do not have a problem with the mode
> > changes - its just _all_ files was shown after tracked branch was
> > changed.  How about below patch?
> > 
> > > > > > I know its in its infancy, but I am not sure on what scm you are basing
> > > > > > it, so not sure how things should behave.
> > > > > 
> > > > > I'm trying to base it on common sense and principle of least surprise.
> > > > > :-)
> > > > > 
> > > > 
> > > > Ok, I'll just bug you then if I am not sure on how you want something ;p
> > > 
> > > Or do it somehow and I'll bug you back if I don't like it. ;-)
> > > 
> > 
> > Ditto
> > 
> > 
> > ----
> > 
> > Normalize show-diff output and make sure we only show real changes after
> > changing the tracked branch.
> > 
> > Signed-off-by: Martin Schlemmer <azarah@nosferatu.za.org>
> > 
> 
> Ok, so I forgot to pull, and missed gitcancel.sh being added.
> How about this one?  (One attached in case evo is brain dead and mangles
> the tabs)
> 

Urk, forgot the update-cache in gitcancel.sh, and prefix is wrong.  I am
not sure this is needed for gitexport.sh though?

----

Normalize show-diff output, add --update-modes target to update-cache,
and make sure we only show real changes after changing the tracked
branch, as well as update the file modes according to the cache.

Signed-off-by: Martin Schlemmer <azarah@nosferatu.za.org>

gitcancel.sh:  ec58f7444a42cd3cbaae919fc68c70a3866420c0
--- ec58f7444a42cd3cbaae919fc68c70a3866420c0/gitcancel.sh
+++ uncommitted/gitcancel.sh
@@ -12,7 +12,8 @@

 # FIXME: Does not revert mode changes!

-show-diff | patch -p0 -R
+show-diff | patch -p1 -R
 rm -f .git/add-queue .git/rm-queue .git/merged

-update-cache --refresh
+# --update-modes need to be before --refresh
+update-cache --update-modes --refresh
gittrack.sh:  03d6db1fb3a70605ef249c632c04e542457f0808
--- 03d6db1fb3a70605ef249c632c04e542457f0808/gittrack.sh
+++ uncommitted/gittrack.sh
@@ -51,6 +51,8 @@

        read-tree $(tree-id "$name")
        gitdiff.sh local "$name" | gitapply.sh
+       # --update-modes need to be before --refresh
+       update-cache --update-modes --refresh

 else
        [ "$tracking" ] || \
@@ -61,6 +63,8 @@
        if [ -s ".git/HEAD.local" ]; then
                gitdiff.sh "$tracking" local | gitapply.sh
                read-tree $(tree-id local)
+               # --update-modes need to be before --refresh
+               update-cache --update-modes --refresh

                head=$(cat .git/HEAD)
                branchhead=$(cat .git/heads/$tracking)
show-diff.c:  a531ca4078525d1c8dcf84aae0bfa89fed6e5d96
--- a531ca4078525d1c8dcf84aae0bfa89fed6e5d96/show-diff.c
+++ uncommitted/show-diff.c
@@ -5,13 +5,18 @@
  */
 #include "cache.h"

-static void show_differences(char *name,
+static void show_differences(struct cache_entry *ce,
        void *old_contents, unsigned long long old_size)
 {
        static char cmd[1000];
+       static char sha1[41];
+       int n;
        FILE *f;

-       snprintf(cmd, sizeof(cmd), "diff -L %s -u -N  - %s", name, name);
+       for (n = 0; n < 20; n++)
+               snprintf(&(sha1[n*2]), 3, "%02x", ce->sha1[n]);
+       snprintf(cmd, sizeof(cmd), "diff -L %s/%s -L uncommitted/%s -u -N  - %s",
+               sha1, ce->name, ce->name, ce->name);
        f = popen(cmd, "w");
        if (old_size)
                fwrite(old_contents, old_size, 1, f);
@@ -99,7 +104,7 @@
                        continue;

                new = read_sha1_file(ce->sha1, type, &size);
-               show_differences(ce->name, new, size);
+               show_differences(ce, new, size);
                free(new);
        }
        return 0;
update-cache.c:  62d0a6c41560d40863c44599355af10d9e089312
--- 62d0a6c41560d40863c44599355af10d9e089312/update-cache.c
+++ uncommitted/update-cache.c
@@ -210,6 +210,39 @@
        }
 }

+static struct cache_entry *update_file_mode(struct cache_entry *ce)
+{
+       struct stat st;
+       int changed;
+
+       if (stat(ce->name, &st) < 0)
+               return NULL;
+
+       changed = cache_match_stat(ce, &st);
+       if (!changed)
+               return ce;
+
+       if (changed & MODE_CHANGED)
+               if (chmod(ce->name, ce->st_mode))
+                       return NULL;
+
+       return ce;
+}
+
+static void update_modes(void)
+{
+       int i;
+
+       for (i = 0; i < active_nr; i++) {
+               struct cache_entry *ce = active_cache[i];
+
+               if (!update_file_mode(ce)) {
+                       printf("%s: needs update\n", ce->name);
+                       continue;
+               }
+       }
+}
+
 /*
  * We fundamentally don't like some paths: we don't want
  * dot or dot-dot anywhere, and in fact, we don't even want
@@ -282,6 +315,10 @@
                                refresh_cache();
                                continue;
                        }
+                       if (!strcmp(path, "--update-modes")) {
+                               update_modes();
+                               continue;
+                       }
                        die("unknown option %s", path);
                }
                if (!verify_path(path)) {


-- 
Martin Schlemmer


[-- Attachment #1.2: git-show-diff.patch --]
[-- Type: text/x-patch, Size: 3200 bytes --]

gitcancel.sh:  ec58f7444a42cd3cbaae919fc68c70a3866420c0
--- ec58f7444a42cd3cbaae919fc68c70a3866420c0/gitcancel.sh
+++ uncommitted/gitcancel.sh
@@ -12,7 +12,8 @@
 
 # FIXME: Does not revert mode changes!
 
-show-diff | patch -p0 -R
+show-diff | patch -p1 -R
 rm -f .git/add-queue .git/rm-queue .git/merged
 
-update-cache --refresh
+# --update-modes need to be before --refresh
+update-cache --update-modes --refresh
gittrack.sh:  03d6db1fb3a70605ef249c632c04e542457f0808
--- 03d6db1fb3a70605ef249c632c04e542457f0808/gittrack.sh
+++ uncommitted/gittrack.sh
@@ -51,6 +51,8 @@
 
 	read-tree $(tree-id "$name")
 	gitdiff.sh local "$name" | gitapply.sh
+	# --update-modes need to be before --refresh
+	update-cache --update-modes --refresh
 
 else
 	[ "$tracking" ] || \
@@ -61,6 +63,8 @@
 	if [ -s ".git/HEAD.local" ]; then
 		gitdiff.sh "$tracking" local | gitapply.sh
 		read-tree $(tree-id local)
+		# --update-modes need to be before --refresh
+		update-cache --update-modes --refresh
 
 		head=$(cat .git/HEAD)
 		branchhead=$(cat .git/heads/$tracking)
show-diff.c:  a531ca4078525d1c8dcf84aae0bfa89fed6e5d96
--- a531ca4078525d1c8dcf84aae0bfa89fed6e5d96/show-diff.c
+++ uncommitted/show-diff.c
@@ -5,13 +5,18 @@
  */
 #include "cache.h"
 
-static void show_differences(char *name,
+static void show_differences(struct cache_entry *ce,
 	void *old_contents, unsigned long long old_size)
 {
 	static char cmd[1000];
+	static char sha1[41];
+	int n;
 	FILE *f;
 
-	snprintf(cmd, sizeof(cmd), "diff -L %s -u -N  - %s", name, name);
+	for (n = 0; n < 20; n++)
+		snprintf(&(sha1[n*2]), 3, "%02x", ce->sha1[n]);
+	snprintf(cmd, sizeof(cmd), "diff -L %s/%s -L uncommitted/%s -u -N  - %s",
+		sha1, ce->name, ce->name, ce->name);
 	f = popen(cmd, "w");
 	if (old_size)
 		fwrite(old_contents, old_size, 1, f);
@@ -99,7 +104,7 @@
 			continue;
 
 		new = read_sha1_file(ce->sha1, type, &size);
-		show_differences(ce->name, new, size);
+		show_differences(ce, new, size);
 		free(new);
 	}
 	return 0;
update-cache.c:  62d0a6c41560d40863c44599355af10d9e089312
--- 62d0a6c41560d40863c44599355af10d9e089312/update-cache.c
+++ uncommitted/update-cache.c
@@ -210,6 +210,39 @@
 	}
 }
 
+static struct cache_entry *update_file_mode(struct cache_entry *ce)
+{
+	struct stat st;
+	int changed;
+
+	if (stat(ce->name, &st) < 0)
+		return NULL;
+
+	changed = cache_match_stat(ce, &st);
+	if (!changed)
+		return ce;
+
+	if (changed & MODE_CHANGED)
+		if (chmod(ce->name, ce->st_mode))
+			return NULL;
+
+	return ce;
+}
+
+static void update_modes(void)
+{
+	int i;
+
+	for (i = 0; i < active_nr; i++) {
+		struct cache_entry *ce = active_cache[i];
+
+		if (!update_file_mode(ce)) {
+			printf("%s: needs update\n", ce->name);
+			continue;
+		}
+	}
+}
+
 /*
  * We fundamentally don't like some paths: we don't want
  * dot or dot-dot anywhere, and in fact, we don't even want
@@ -282,6 +315,10 @@
 				refresh_cache();
 				continue;
 			}
+			if (!strcmp(path, "--update-modes")) {
+				update_modes();
+				continue;
+			}
 			die("unknown option %s", path);
 		}
 		if (!verify_path(path)) {

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: Re: Re: Remove need to untrack before tracking new branch
From: Martin Schlemmer @ 2005-04-14  8:28 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <1113461754.23299.68.camel@nosferatu.lan>


[-- Attachment #1.1: Type: text/plain, Size: 8317 bytes --]

On Thu, 2005-04-14 at 08:55 +0200, Martin Schlemmer wrote:
> On Thu, 2005-04-14 at 00:19 +0200, Petr Baudis wrote:
> > Dear diary, on Wed, Apr 13, 2005 at 02:15:37PM CEST, I got a letter
> > where Martin Schlemmer <azarah@nosferatu.za.org> told me that...
> > > On Wed, 2005-04-13 at 11:26 +0200, Petr Baudis wrote:
> > >> > Dear diary, on Wed, Apr 13, 2005 at 10:41:12AM CEST, I got a letter
> > > > where Martin Schlemmer <azarah@nosferatu.za.org> told me that...
> > > > > On Wed, 2005-04-13 at 09:54 +0200, Petr Baudis wrote:
> > > > > PS: not having looked deeper yet, why does fsck-cache always find
> > > > > unreferenced blobs/commits (no matter what tree is tracked, they stay
> > > > > the same) ?  And trying to remove them leads to more, which leads to an
> > > > > empty .git/opjects/ =)  Also, leading to this, will adding an option to
> > > > > remove disconnected commits/blobs from local commits (that was
> > > > > disconnected with a pull) be a viable option to add?
> > > > 
> > > > fsck-cache is concerned only by the objects database, so all the HEADs
> > > > are unreferenced commits too. This is a right thing, the HEAD tracking
> > > > should stay purely in the scripts - if we want to make fsck-cache
> > > > smarter about that, we should implement git fsck or something.
> > > > 
> > > > Killing unreferenced blobs should be safe, I think.
> > > > 
> > > > > First, about the 'git diff' thing I asked yesterday .. what I meant, was
> > > > > should it actually output this:
> > > > > 
> > > > > ----
> > > > > COPYING:  fe2a4177a760fd110e78788734f167bd633be8de 33
> > > > > Makefile:  929aa49a3dbe683ad52094099797bc636a7949a6 33
> > > > > README:  46c6a9ea48ddd1dda45ca585f49975a6869ffe51 33
> > > > > ...
> > > > > ----
> > > > > 
> > > > > Shouldn't it just show actual changes?
> > > > 
> > > > This is an actual change. It's just that it's a change to metadata
> > > > (somewhat esotherically described by the "33"), not the file contents.
> > > > 
> > > > BTW, git diff does actually something completely different from git diff
> > > > with any arguments. It diffs to the directory cache, not to any tree! It
> > > > just wraps show-diff, which has also a different output format (not
> > > > outputting "git diffs"). The worst thing is that it requires a different
> > > > -p option to apply. Someone should purge this wart, I think.
> > > > 
> > > 
> > > Check applied patch (also in the new output).
> > 
> > Please send patches inline and properly signed off.
> > 
> 
> The new evo have a bad habit of screwing the tabs, but sure.
> 
> > > > > Also on the same note .. should 'git ci' without listed files to be
> > > > > committed, really add a reference to all files as it currently do in the
> > > > > commit/blob/whatever info, instead of just the changed/added files (see
> > > > > the git-seperate-dir.patch you have not yet commented on for reference)?
> > > > 
> > > > ...
> > > > 
> > > 
> > > Patch will also resolve this.
> > 
> > Your patch is bad - it removes the pure metadata changes, but you
> > definitively do not want to do that! If you are annoyed by meaningless
> > time changes etc, do update-cache --refresh. Ignoring mode changes is a
> > pure disaster.
> > 
> 
> Ahh - and there was light.  I do not have a problem with the mode
> changes - its just _all_ files was shown after tracked branch was
> changed.  How about below patch?
> 
> > > > > I know its in its infancy, but I am not sure on what scm you are basing
> > > > > it, so not sure how things should behave.
> > > > 
> > > > I'm trying to base it on common sense and principle of least surprise.
> > > > :-)
> > > > 
> > > 
> > > Ok, I'll just bug you then if I am not sure on how you want something ;p
> > 
> > Or do it somehow and I'll bug you back if I don't like it. ;-)
> > 
> 
> Ditto
> 
> 
> ----
> 
> Normalize show-diff output and make sure we only show real changes after
> changing the tracked branch.
> 
> Signed-off-by: Martin Schlemmer <azarah@nosferatu.za.org>
> 

Ok, so I forgot to pull, and missed gitcancel.sh being added.
How about this one?  (One attached in case evo is brain dead and mangles
the tabs)

----

Normalize show-diff output, add --update-modes target to update-cache,
and make sure we only show real changes after changing the tracked
branch, as well as update the file modes according to the cache.

Signed-off-by: Martin Schlemmer <azarah@nosferatu.za.org>

gitcancel.sh:  ec58f7444a42cd3cbaae919fc68c70a3866420c0
--- gitcancel.sh
+++ gitcancel.sh        2005-04-14 10:26:49.000000000 +0200
@@ -12,7 +12,7 @@

 # FIXME: Does not revert mode changes!

-show-diff | patch -p0 -R
+show-diff | patch -p1 -R
 rm -f .git/add-queue .git/rm-queue .git/merged

 update-cache --refresh
gittrack.sh:  03d6db1fb3a70605ef249c632c04e542457f0808
--- gittrack.sh
+++ gittrack.sh 2005-04-14 10:26:49.000000000 +0200
@@ -51,6 +51,8 @@

        read-tree $(tree-id "$name")
        gitdiff.sh local "$name" | gitapply.sh
+       # --update-modes need to be before --refresh
+       update-cache --update-modes --refresh

 else
        [ "$tracking" ] || \
@@ -61,6 +63,8 @@
        if [ -s ".git/HEAD.local" ]; then
                gitdiff.sh "$tracking" local | gitapply.sh
                read-tree $(tree-id local)
+               # --update-modes need to be before --refresh
+               update-cache --update-modes --refresh

                head=$(cat .git/HEAD)
                branchhead=$(cat .git/heads/$tracking)
show-diff.c:  a531ca4078525d1c8dcf84aae0bfa89fed6e5d96
--- show-diff.c
+++ show-diff.c 2005-04-14 10:26:49.000000000 +0200
@@ -5,13 +5,18 @@
  */
 #include "cache.h"

-static void show_differences(char *name,
+static void show_differences(struct cache_entry *ce,
        void *old_contents, unsigned long long old_size)
 {
        static char cmd[1000];
+       static char sha1[41];
+       int n;
        FILE *f;

-       snprintf(cmd, sizeof(cmd), "diff -L %s -u -N  - %s", name, name);
+       for (n = 0; n < 20; n++)
+               snprintf(&(sha1[n*2]), 3, "%02x", ce->sha1[n]);
+       snprintf(cmd, sizeof(cmd), "diff -L %s/%s -L uncommitted/%s -u -N  - %s",
+               sha1, ce->name, ce->name, ce->name);
        f = popen(cmd, "w");
        if (old_size)
                fwrite(old_contents, old_size, 1, f);
@@ -99,7 +104,7 @@
                        continue;

                new = read_sha1_file(ce->sha1, type, &size);
-               show_differences(ce->name, new, size);
+               show_differences(ce, new, size);
                free(new);
        }
        return 0;
update-cache.c:  62d0a6c41560d40863c44599355af10d9e089312
--- update-cache.c
+++ update-cache.c      2005-04-14 10:27:07.000000000 +0200
@@ -210,6 +210,39 @@
        }
 }

+static struct cache_entry *update_file_mode(struct cache_entry *ce)
+{
+       struct stat st;
+       int changed;
+
+       if (stat(ce->name, &st) < 0)
+               return NULL;
+
+       changed = cache_match_stat(ce, &st);
+       if (!changed)
+               return ce;
+
+       if (changed & MODE_CHANGED)
+               if (chmod(ce->name, ce->st_mode))
+                       return NULL;
+
+       return ce;
+}
+
+static void update_modes(void)
+{
+       int i;
+
+       for (i = 0; i < active_nr; i++) {
+               struct cache_entry *ce = active_cache[i];
+
+               if (!update_file_mode(ce)) {
+                       printf("%s: needs update\n", ce->name);
+                       continue;
+               }
+       }
+}
+
 /*
  * We fundamentally don't like some paths: we don't want
  * dot or dot-dot anywhere, and in fact, we don't even want
@@ -282,6 +315,10 @@
                                refresh_cache();
                                continue;
                        }
+                       if (!strcmp(path, "--update-modes")) {
+                               update_modes();
+                               continue;
+                       }
                        die("unknown option %s", path);
                }
                if (!verify_path(path)) {



-- 
Martin Schlemmer


[-- Attachment #1.2: git-show-diff.patch --]
[-- Type: text/x-patch, Size: 3045 bytes --]

gitcancel.sh:  ec58f7444a42cd3cbaae919fc68c70a3866420c0
--- gitcancel.sh
+++ gitcancel.sh	2005-04-14 10:26:49.000000000 +0200
@@ -12,7 +12,7 @@
 
 # FIXME: Does not revert mode changes!
 
-show-diff | patch -p0 -R
+show-diff | patch -p1 -R
 rm -f .git/add-queue .git/rm-queue .git/merged
 
 update-cache --refresh
gittrack.sh:  03d6db1fb3a70605ef249c632c04e542457f0808
--- gittrack.sh
+++ gittrack.sh	2005-04-14 10:26:49.000000000 +0200
@@ -51,6 +51,8 @@
 
 	read-tree $(tree-id "$name")
 	gitdiff.sh local "$name" | gitapply.sh
+	# --update-modes need to be before --refresh
+	update-cache --update-modes --refresh
 
 else
 	[ "$tracking" ] || \
@@ -61,6 +63,8 @@
 	if [ -s ".git/HEAD.local" ]; then
 		gitdiff.sh "$tracking" local | gitapply.sh
 		read-tree $(tree-id local)
+		# --update-modes need to be before --refresh
+		update-cache --update-modes --refresh
 
 		head=$(cat .git/HEAD)
 		branchhead=$(cat .git/heads/$tracking)
show-diff.c:  a531ca4078525d1c8dcf84aae0bfa89fed6e5d96
--- show-diff.c
+++ show-diff.c	2005-04-14 10:26:49.000000000 +0200
@@ -5,13 +5,18 @@
  */
 #include "cache.h"
 
-static void show_differences(char *name,
+static void show_differences(struct cache_entry *ce,
 	void *old_contents, unsigned long long old_size)
 {
 	static char cmd[1000];
+	static char sha1[41];
+	int n;
 	FILE *f;
 
-	snprintf(cmd, sizeof(cmd), "diff -L %s -u -N  - %s", name, name);
+	for (n = 0; n < 20; n++)
+		snprintf(&(sha1[n*2]), 3, "%02x", ce->sha1[n]);
+	snprintf(cmd, sizeof(cmd), "diff -L %s/%s -L uncommitted/%s -u -N  - %s",
+		sha1, ce->name, ce->name, ce->name);
 	f = popen(cmd, "w");
 	if (old_size)
 		fwrite(old_contents, old_size, 1, f);
@@ -99,7 +104,7 @@
 			continue;
 
 		new = read_sha1_file(ce->sha1, type, &size);
-		show_differences(ce->name, new, size);
+		show_differences(ce, new, size);
 		free(new);
 	}
 	return 0;
update-cache.c:  62d0a6c41560d40863c44599355af10d9e089312
--- update-cache.c
+++ update-cache.c	2005-04-14 10:27:07.000000000 +0200
@@ -210,6 +210,39 @@
 	}
 }
 
+static struct cache_entry *update_file_mode(struct cache_entry *ce)
+{
+	struct stat st;
+	int changed;
+
+	if (stat(ce->name, &st) < 0)
+		return NULL;
+
+	changed = cache_match_stat(ce, &st);
+	if (!changed)
+		return ce;
+
+	if (changed & MODE_CHANGED)
+		if (chmod(ce->name, ce->st_mode))
+			return NULL;
+
+	return ce;
+}
+
+static void update_modes(void)
+{
+	int i;
+
+	for (i = 0; i < active_nr; i++) {
+		struct cache_entry *ce = active_cache[i];
+
+		if (!update_file_mode(ce)) {
+			printf("%s: needs update\n", ce->name);
+			continue;
+		}
+	}
+}
+
 /*
  * We fundamentally don't like some paths: we don't want
  * dot or dot-dot anywhere, and in fact, we don't even want
@@ -282,6 +315,10 @@
 				refresh_cache();
 				continue;
 			}
+			if (!strcmp(path, "--update-modes")) {
+				update_modes();
+				continue;
+			}
 			die("unknown option %s", path);
 		}
 		if (!verify_path(path)) {

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Date handling.
From: David Woodhouse @ 2005-04-14  8:16 UTC (permalink / raw)
  To: git

The date handling is somewhat unreliable. We render dates into textual
representation using the committer's locale (day names, etc), then later
attempt to interpret that in some other locale. And we were just using
localtime without even specifying the timezone so the timestamp was
fairly randomised anyway. In fact, an $AUTHOR_DATE environment variable
was making its way into the database entirely unchecked. 

I see two possible solutions:
	1. Just store seconds-since-GMT-epoch and if we really want, the
	   timezone as auxiliary information.
	2. Store dates in RFC2822 form.

Unless someone convincingly expresses a preference before I get to work
and start playing with it, I'll implement the latter.

-- 
dwmw2



^ permalink raw reply

* Re: Yet another base64 patch
From: Linus Torvalds @ 2005-04-14  8:17 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: git
In-Reply-To: <425DEF64.60108@zytor.com>



On Wed, 13 Apr 2005, H. Peter Anvin wrote:
> 
> Checking out the total kernel tree (time checkout-cache -a into an empty 
> directory):
> 
> 		Cache cold	Cache hot
> stock		3:46.95		19.95
> base64	5:56.20		23.74
> flat		2:44.13		15.68

So why is "base64" worse than the stock one?

As mentioned, the "flat" version may be faster, but it really isn't an
option. 32000 objects is peanuts. Any respectable source tree may hit that
in a short time, and will break in horrible ways on many Linux
filesystems.

So you need at least a single level of subdirectory. 

What I don't get is why the stock hex version would be better than base64.

I like the result, I just don't _understand_ it.

		Linus

^ permalink raw reply

* Re: Merge with git-pasky II.
From: Linus Torvalds @ 2005-04-14  8:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Petr Baudis, Christopher Li, git
In-Reply-To: <7vfyxtsurd.fsf@assigned-by-dhcp.cox.net>



On Thu, 14 Apr 2005, Junio C Hamano wrote:
> 
> I now have a Perl script that uses rev-tree, cat-file,
> diff-tree, show-files (with one modification so that it can deal
> with pathnames with embedded newlines), update-cache (with one
> modification so that I can add an entry for a file that does not
> exist to the dircache) and merge (from RCS).  Quick and dirty.

That's exactly what I wanted. Q'n'D is how the ball gets rolling.

In the meantime I wrote a very stupid "merge-tree" which does things
slightly differently, but I really think your approach (aka my original
approach) is actually a lot faster. I was just starting to worry that the 
ball didn't start, so I wrote an even hackier one.

My really hacky one is called "merge-tree", and it really only merges one 
directory. For each entry in the directory it says either

	select <mode> <sha1> path

or

	merge <mode>-><mode>,<mode> <sha1>-><sha1>,<sha1> path

depending on whether it could directly select the right object or not.

It's actually exactly the same algorithm as the first one, but I was 
afraid the first one would be so abstract that it (a) might not work and 
(b) wouldn't get people to work it out. This "one directory at a time with 
very explicit output" thing is much more down-to-earth, but it's also
likely slower because it will need script help more often.

That said, I don't know. MOST of the time there will be just a single 
"directory" entry that needs merging, and then the script would just need 
to recurse into that directory with the new "tree" objects. So it might 
not be too horrible.

But I'm really happy that you seem to have implemented my first 
suggestion and I seem to have been wasting my time. 

>  5. for each path involved:
> 
>   5.0 if neither heads change it, leave it as is;
>   5.1 if only one head changes a path and the other does not, just
>       get the changed version;
>   5.2 if both heads change it, check all three out and run merge.

You missed one case: 

    5.0.1 if both heads change it to the same thing, take the new thing

but maybe you counted that as 5.0 (it _should_ fall out automatically from
the fact that "diff-tree" between the two destination trees shows no
difference for such a file).

Now, arguably, your 5.2 will do things right, but the thing is, it's 
actually fairly _common_ that both heads have changed something to the 
same thing. Namely if there was a previous merge that already handled that 
case, but that previous merge may not be a proper parent of the new 
commits.  So from a performance standpoint you really don't want to 
consider that to be a merge - you just pick up the new contents directly.

See?

(My stupid "merge-tree" should show the algorithm in painful obviousity. 
Of course, my stipid merge-tree may also be painfully buggy. You be the 
judge).

> It does not currently commit.  You can go to ./,,merge-temp/ and
> see show-diff to see the result of the merge.  Files added in
> one head has already been run "update-cache" when the script
> ends, but changed and merged files are not---dircache still has
> the common ancestor view.

That sounds good.

> Also to implement 'changed only by one-side' without actually
> checking the file out, I needed to add one option to
> 'update-cache'.  --cacheinfo flag is used this way:
> 
>     $ update-cache --cacheinfo mode sha1 path

Yes. My "merge-tree" needs the exact same thing.

Looks good from your explanation, but I'm too tired to look at the code. 
It's 1AM, and the kids get up at 7.

I'm not much of a hacker, I usually crash by 10PM these days ;^)

			Linus

^ permalink raw reply

* Re: Yet another base64 patch
From: Linus Torvalds @ 2005-04-14  7:40 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: Christopher Li, git
In-Reply-To: <425E0D62.9000401@zytor.com>



On Wed, 13 Apr 2005, H. Peter Anvin wrote:
> 
> Actually, the subdirectory hack has the same effect, so you lose 
> regardless.  Doesn't mean that you can't construct cases where the 
> subdirectory hack doesn't win, but I maintain that those are likely to 
> be artificial.

I'll tell you why a flat object directory format simply isn't an option.

Hint: maximum directory size. It's limited by n_link, and it's almost
universally a 16-bit number on Linux (and generally artifically limited to
32000 entries).

In other words, if you ever expect to have more than 32000 objects, a flat 
space simply isn't possible.

		Linus

^ permalink raw reply

* Trying base32
From: H. Peter Anvin @ 2005-04-14  7:26 UTC (permalink / raw)
  To: git

Out of curiosity, I cooked up a version of git which used a flat base32, 
instead of base64, repository.  The goal of this is, of course, to 
provide for compatibility with case-deficient filesystems.

The answer is that the base32 format is measurably slower than base64, 
with a cache-hot checkout time of 19.19 seconds (as compared to 15.68 
for base64.)  So the length of the filenames does matter.

Anyway, I have uploaded the base32 patch onto kernel.org, and I have 
moved all the git patches into:

ftp://ftp.kernel.org/pub/linux/kernel/people/hpa/git/

At this point, though, I would recommend the base64-flat patch.

	-hpa

^ permalink raw reply

* Re: Merge with git-pasky II.
From: Junio C Hamano @ 2005-04-14  7:05 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Petr Baudis, Christopher Li, git
In-Reply-To: <Pine.LNX.4.58.0504132020550.7211@ppc970.osdl.org>

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> On that note - I've been avoiding doing the merge-tree thing, in the hope 
LT> that somebody else does what I've described.

I now have a Perl script that uses rev-tree, cat-file,
diff-tree, show-files (with one modification so that it can deal
with pathnames with embedded newlines), update-cache (with one
modification so that I can add an entry for a file that does not
exist to the dircache) and merge (from RCS).  Quick and dirty.

The changes to show-files is to give it an optional '-z' flag,
which chanegs record terminator to NUL character instead of LF.

The script git-merge.perl takes two head commits.  It basically
follows what you described as I remember ;-):

 1. runs rev-tree with --edges to find the common anscestor.

 2. creates a temporary directory "./,,merge-temp"; create a
    symlink ./,,merge-temp/.git/objects that points at
    .git/objects.

 3. sets up dircache there, initially populated with this common
    ancestor tree.  No files are checked out.  Just set up
    .git/index and that's it.

 4. runs diff-tree to find what has been changed in each head.

 5. for each path involved:

  5.0 if neither heads change it, leave it as is;
  5.1 if only one head changes a path and the other does not, just
      get the changed version;
  5.2 if both heads change it, check all three out and run merge.

It does not currently commit.  You can go to ./,,merge-temp/ and
see show-diff to see the result of the merge.  Files added in
one head has already been run "update-cache" when the script
ends, but changed and merged files are not---dircache still has
the common ancestor view.  So show-diff you will be seeing may
be enormous and not very useful if two forks were done in the
distant past.  After reviewing the merge result, you can
update-cache, write-tree and commit-tree as usual, but with one
caveat:  do not run "show-files | xargs update-cache" if you are
running git-merge.perl without -f flag!

By default, git-merge.perl creates absolute minimum number of
files in ./,,merge-temp---only the merged files are left there
so that you can inspect them.  You will not see unmodified
files nor files changed only by one side of the merge.

If you give '-o' (oneside checkout) flag to git-merge.perl, then
the files only one side of the merge changed are also checked
out in ./,,merge-temp.  If you give '-f' (full checkout) flag to
git-merge.perl, then in addition to what '-o' checks out,
unchanged files are checked out in ./,,merge-temp.  This default
is geared towards a huge tree with small merges (favorite case
of Linus, if I understand correctly).

Running 'show-diff' in such a sparsely populated merge result
tree gives you huge results because recent show-diff shows diffs
with empty files.  I added a '-r' flag to show-diff, which
squelches diffs with empty files.

Also to implement 'changed only by one-side' without actually
checking the file out, I needed to add one option to
'update-cache'.  --cacheinfo flag is used this way:

    $ update-cache --cacheinfo mode sha1 path

and adds the pathname with mode and sha1 to the .git/index
without actually requiring you to have such a file there.

Signed-off-by: Junio C Hamano <junkio@cox.net>

---

 show-diff.c    |   11 ++-
 show-files.c   |   12 ++-
 update-cache.c |   25 +++++++
 git-merge.perl |  193 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 234 insertions(+), 7 deletions(-)


show-diff.c:  a531ca4078525d1c8dcf84aae0bfa89fed6e5d96
--- show-diff.c
+++ show-diff.c	2005-04-13 22:47:33.000000000 -0700
@@ -58,15 +58,20 @@
 int main(int argc, char **argv)
 {
 	int silent = 0;
+	int silent_on_nonexisting_files = 0;
 	int entries = read_cache();
 	int i;
 
 	while (argc-- > 1) {
 		if (!strcmp(argv[1], "-s")) {
-			silent = 1;
+			silent_on_nonexisting_files = silent = 1;
 			continue;
 		}
-		usage("show-diff [-s]");
+		if (!strcmp(argv[1], "-r")) {
+			silent_on_nonexisting_files = 1;
+			continue;
+		}
+		usage("show-diff [-s] [-r]");
 	}
 
 	if (entries < 0) {
@@ -83,7 +88,7 @@
 
 		if (stat(ce->name, &st) < 0) {
 			printf("%s: %s\n", ce->name, strerror(errno));
-			if (errno == ENOENT && !silent)
+			if (errno == ENOENT && !silent_on_nonexisting_files)
 				show_diff_empty(ce);
 			continue;
 		}
show-files.c:  a9fa6767a418f870a34b39379f417bf37b17ee18
--- show-files.c
+++ show-files.c	2005-04-13 21:18:40.000000000 -0700
@@ -14,6 +14,7 @@
 static int show_cached = 0;
 static int show_others = 0;
 static int show_ignored = 0;
+static int line_terminator = '\n';
 
 static const char **dir;
 static int nr_dir;
@@ -105,12 +106,12 @@
 	}
 	if (show_others) {
 		for (i = 0; i < nr_dir; i++)
-			printf("%s\n", dir[i]);
+			printf("%s%c", dir[i], line_terminator);
 	}
 	if (show_cached) {
 		for (i = 0; i < active_nr; i++) {
 			struct cache_entry *ce = active_cache[i];
-			printf("%s\n", ce->name);
+			printf("%s%c", ce->name, line_terminator);
 		}
 	}
 	if (show_deleted) {
@@ -119,7 +120,7 @@
 			struct stat st;
 			if (!stat(ce->name, &st))
 				continue;
-			printf("%s\n", ce->name);
+			printf("%s%c", ce->name, line_terminator);
 		}
 	}
 	if (show_ignored) {
@@ -134,6 +135,11 @@
 	for (i = 1; i < argc; i++) {
 		char *arg = argv[i];
 
+		if (!strcmp(arg, "-z")) {
+			line_terminator = 0;
+			continue;
+		}
+
 		if (!strcmp(arg, "--cached")) {
 			show_cached = 1;
 			continue;
update-cache.c:  8f149d5a4ab60e030a0ab19fdb59b8ee2576ee71
--- update-cache.c
+++ update-cache.c	2005-04-13 23:27:54.000000000 -0700
@@ -203,6 +203,8 @@
 {
 	int i, newfd, entries;
 	int allow_options = 1;
+	const char *sha1_force = NULL;
+	const char *mode_force = NULL;
 
 	newfd = open(".git/index.lock", O_RDWR | O_CREAT | O_EXCL, 0600);
 	if (newfd < 0)
@@ -235,14 +237,35 @@
 				refresh_cache();
 				continue;
 			}
+			if (!strcmp(path, "--cacheinfo")) {
+				mode_force = argv[++i];
+				sha1_force = argv[++i];
+				continue;
+			}
 			die("unknown option %s", path);
 		}
 		if (!verify_path(path)) {
 			fprintf(stderr, "Ignoring path %s\n", argv[i]);
 			continue;
 		}
-		if (add_file_to_cache(path))
+		if (sha1_force && mode_force) {
+			struct cache_entry *ce;
+			int namelen = strlen(path);
+			int mode;
+			int size = cache_entry_size(namelen);
+			sscanf(mode_force, "%o", &mode);
+			ce = malloc(size);
+			memset(ce, 0, size);
+			memcpy(ce->name, path, namelen);
+			ce->namelen = namelen;
+			ce->st_mode = mode;
+			get_sha1_hex(sha1_force, ce->sha1);
+
+			add_cache_entry(ce, 1);
+		}
+		else if (add_file_to_cache(path))
 			die("Unable to add %s to database", path);
+		mode_force = sha1_force = NULL;
 	}
 	if (write_cache(newfd, active_cache, active_nr) ||
 	    rename(".git/index.lock", ".git/index"))

--- /dev/null	2005-03-19 15:28:25.000000000 -0800
+++ git-merge.perl	2005-04-13 23:45:23.000000000 -0700
@@ -0,0 +1,193 @@
+#!/usr/bin/perl -w
+
+use Getopt::Long;
+
+my $full_checkout = 0;
+my $oneside_checkout = 0;
+GetOptions("full" => \$full_checkout,
+	   "oneside" => \$oneside_checkout)
+    or die;
+
+if ($full_checkout) {
+    $oneside_checkout = 1;
+}
+
+sub read_rev_tree {
+    my (@head) = @_;
+    my ($fhi);
+    open $fhi, '-|', 'rev-tree', '--edges', @head
+	or die "$!: rev-tree --edges @head";
+    my $common;
+    while (<$fhi>) {
+	chomp;
+	(undef, undef, $common) = split(/ /, $_);
+	if ($common =~ s/^([a-f0-f]{40}):\d+$/$1/) {
+	    last;
+	}
+    }
+    close $fhi;
+    return $common;
+}
+
+sub read_commit_tree {
+    my ($commit) = @_;
+    my ($fhi);
+    open $fhi, '-|', 'cat-file', 'commit', $commit
+	or die "$!: cat-file commit $commit";
+    my $tree = <$fhi>;
+    close $fhi;
+    $tree =~ s/^tree //;
+    return $tree;
+}
+
+sub read_diff_tree {
+    my (@tree) = @_;
+    my ($fhi);
+    local ($_, $/);
+    $/ = "\0"; 
+    my %path;
+    open $fhi, '-|', 'diff-tree', '-r', @tree
+	or die "$!: diff-tree -r @tree";
+    while (<$fhi>) {
+	chomp;
+	if (/^\*[0-7]+->([0-7]+)\tblob\t[0-9a-f]+->([0-9a-f]{40})\t(.*)$/s) {
+	    # mode newsha path
+	    $path{$3} = [$1, $2];
+	}
+	elsif (/^\+([0-7]+)\tblob\t([0-9a-f]{40})\t(.*)$/s) {
+	    # mode newsha path
+	    $path{$3} = [$1, $2];
+	}
+	else {
+	    print STDERR "$_??";
+	}
+    }
+    close $fhi;
+    return %path;
+}
+
+sub read_show_files {
+    my ($fhi);
+    local ($_, $/);
+    $/ = "\0"; 
+    open $fhi, '-|', 'show-files', '-z'
+	or die "$!: show-files -z";
+    my (@path) = map { chomp; $_ } <$fhi>;
+    close $fhi;
+    return @path;
+}
+
+sub checkout_file {
+    my ($path, $info) = @_;
+    my (@elt) = split(/\//, $path);
+    my $j = '';
+    my $tail = pop @elt;
+    my ($fhi, $fho);
+    for (@elt) {
+	mkdir "$j$_";
+	$j = "$j$_/";
+    }
+    open $fho, '>', "$path";
+    open $fhi, '-|', 'cat-file', 'blob', $info->[1]
+	or die "$!: cat-file blob $info->[1]";
+    while (<$fhi>) {
+	print $fho $_;
+    }
+    close $fhi;
+    close $fho;
+    chmod oct("0$info->[0]"), "$path";
+}
+
+sub record_file {
+    my ($path, $info) = @_;
+    system 'update-cache', '--cacheinfo', @$info, $path;
+}
+
+sub merge_tree {
+    my ($path, $info0, $info1) = @_;
+    print STDERR "M - $path\n";
+    checkout_file(',,merge-0', $info0);
+    checkout_file(',,merge-1', $info1);
+    system 'checkout-cache', $path;
+    my ($fhi, $fho);
+    open $fhi, '-|', 'merge', '-p', ',,merge-0', $path, ',,merge-1';
+    open $fho, '>', "$path+";
+    local ($/);
+    while (<$fhi>) { print $fho $_; }
+    close $fhi;
+    close $fho;
+    unlink ',,merge-0', ',,merge-1';
+    rename "$path+", $path;
+    # There is no reason to prefer info0 over info1 but
+    # we need to pick one.
+    chmod oct("0$info0->[0]"), "$path";
+}
+
+# Find common ancestor of two trees.
+my $common = read_rev_tree(@ARGV);
+print "Common ancestor: $common\n";
+
+# Create a temporary directory and go there.
+system 'rm', '-rf', ',,merge-temp';
+for ((',,merge-temp', '.git')) { mkdir $_; chdir $_; }
+symlink "../../.git/objects", "objects";
+chdir '..';
+
+my $ancestor_tree = read_commit_tree($common);
+system 'read-tree', $ancestor_tree;
+
+my %tree0 = read_diff_tree($ancestor_tree, read_commit_tree($ARGV[0]));
+my %tree1 = read_diff_tree($ancestor_tree, read_commit_tree($ARGV[1]));
+
+my @ancestor_file = read_show_files();
+my %ancestor_file = map { $_ => 1 } @ancestor_file;
+
+for (@ancestor_file) {
+    if (! exists $tree0{$_} && ! exists $tree1{$_}) {
+	if ($full_checkout) {
+	    system 'checkout-cache', $_;
+	}
+	print STDERR "O - $_\n";
+    }
+}
+
+my %need_merge = ();
+
+for $path (keys %tree0) {
+    if (! exists $tree1{$path}) {
+	# Only changed in tree 0 --- take his version
+	print STDERR "0 - $path\n";
+	if (! exists $ancestor_file{$path}) {
+	    checkout_file($path, $tree0{$path});
+	    system 'update-cache', '--add', "$path";
+	}
+	elsif ($oneside_checkout) {
+	    checkout_file($path, $tree0{$path});
+	}
+	else {
+	    record_file($path, $tree0{$path});
+	}
+    }
+    else {
+	merge_tree($path, $tree0{$path}, $tree1{$path});
+    }
+}
+
+for $path (keys %tree1) {
+    if (! exists $tree0{$path}) {
+	# Only changed in tree 1 --- take his version
+	print STDERR "1 - $path\n";
+	if (! exists $ancestor_file{$path}) {
+	    checkout_file($path, $tree1{$path});
+	    system 'update-cache', '--add', "$path";
+	}
+	elsif ($oneside_checkout) {
+	    checkout_file($path, $tree1{$path});
+	}
+	else {
+	    record_file($path, $tree1{$path});
+	}
+    }
+}
+
+# system 'show-diff';



^ permalink raw reply

* Re: Index/hash order
From: Ingo Molnar @ 2005-04-14  7:04 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Florian Weimer, H. Peter Anvin, git
In-Reply-To: <Pine.LNX.4.58.0504131503180.4501@ppc970.osdl.org>


* Linus Torvalds <torvalds@osdl.org> wrote:

> > I've run a few tests, just to get a few numbers of the overhead
> > involved.  I used the last ~8,000 changesets from the BKCVS kernel
> > repository.  With cold cache, a checkout from cold cache takes about
> > 250 seconds on my laptop.  I don't have git numbers, but a mere copy
> > of the kernel tree needs 40 seconds.
> 
> I will bet you that a git checkout is _faster_ than a kernel source 
> tree copy. The time will be dominated by the IO costs (in particular 
> the read costs), and the IO costs are lower thanks to compression. So 
> I think that the cold-cache case will beat your 40 seconds by a clear 
> margin. It generally compresses to half the size, so 20 seconds is not 
> impossible (although seek costs would tend to stay constant, so I'd 
> expect it to be somewhere in between the two).

i'd be surprised if it was twice as fast - cache-cold linear checkouts 
are _seek_ limited, and it doesnt matter whether after a 1-2 msec 
track-to-track disk seek the DMA engine spends another 30 microseconds 
DMA-ing 60K uncompressed data instead of 30K compressed... (there are 
other factors, but this is the main thing.)

	Ingo

^ permalink raw reply

* Re: Re: Re: Remove need to untrack before tracking new branch
From: Martin Schlemmer @ 2005-04-14  6:55 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <20050413221936.GI25711@pasky.ji.cz>


[-- Attachment #1.1: Type: text/plain, Size: 5659 bytes --]

On Thu, 2005-04-14 at 00:19 +0200, Petr Baudis wrote:
> Dear diary, on Wed, Apr 13, 2005 at 02:15:37PM CEST, I got a letter
> where Martin Schlemmer <azarah@nosferatu.za.org> told me that...
> > On Wed, 2005-04-13 at 11:26 +0200, Petr Baudis wrote:
> >> > Dear diary, on Wed, Apr 13, 2005 at 10:41:12AM CEST, I got a letter
> > > where Martin Schlemmer <azarah@nosferatu.za.org> told me that...
> > > > On Wed, 2005-04-13 at 09:54 +0200, Petr Baudis wrote:
> > > > PS: not having looked deeper yet, why does fsck-cache always find
> > > > unreferenced blobs/commits (no matter what tree is tracked, they stay
> > > > the same) ?  And trying to remove them leads to more, which leads to an
> > > > empty .git/opjects/ =)  Also, leading to this, will adding an option to
> > > > remove disconnected commits/blobs from local commits (that was
> > > > disconnected with a pull) be a viable option to add?
> > > 
> > > fsck-cache is concerned only by the objects database, so all the HEADs
> > > are unreferenced commits too. This is a right thing, the HEAD tracking
> > > should stay purely in the scripts - if we want to make fsck-cache
> > > smarter about that, we should implement git fsck or something.
> > > 
> > > Killing unreferenced blobs should be safe, I think.
> > > 
> > > > First, about the 'git diff' thing I asked yesterday .. what I meant, was
> > > > should it actually output this:
> > > > 
> > > > ----
> > > > COPYING:  fe2a4177a760fd110e78788734f167bd633be8de 33
> > > > Makefile:  929aa49a3dbe683ad52094099797bc636a7949a6 33
> > > > README:  46c6a9ea48ddd1dda45ca585f49975a6869ffe51 33
> > > > ...
> > > > ----
> > > > 
> > > > Shouldn't it just show actual changes?
> > > 
> > > This is an actual change. It's just that it's a change to metadata
> > > (somewhat esotherically described by the "33"), not the file contents.
> > > 
> > > BTW, git diff does actually something completely different from git diff
> > > with any arguments. It diffs to the directory cache, not to any tree! It
> > > just wraps show-diff, which has also a different output format (not
> > > outputting "git diffs"). The worst thing is that it requires a different
> > > -p option to apply. Someone should purge this wart, I think.
> > > 
> > 
> > Check applied patch (also in the new output).
> 
> Please send patches inline and properly signed off.
> 

The new evo have a bad habit of screwing the tabs, but sure.

> > > > Also on the same note .. should 'git ci' without listed files to be
> > > > committed, really add a reference to all files as it currently do in the
> > > > commit/blob/whatever info, instead of just the changed/added files (see
> > > > the git-seperate-dir.patch you have not yet commented on for reference)?
> > > 
> > > ...
> > > 
> > 
> > Patch will also resolve this.
> 
> Your patch is bad - it removes the pure metadata changes, but you
> definitively do not want to do that! If you are annoyed by meaningless
> time changes etc, do update-cache --refresh. Ignoring mode changes is a
> pure disaster.
> 

Ahh - and there was light.  I do not have a problem with the mode
changes - its just _all_ files was shown after tracked branch was
changed.  How about below patch?

> > > > I know its in its infancy, but I am not sure on what scm you are basing
> > > > it, so not sure how things should behave.
> > > 
> > > I'm trying to base it on common sense and principle of least surprise.
> > > :-)
> > > 
> > 
> > Ok, I'll just bug you then if I am not sure on how you want something ;p
> 
> Or do it somehow and I'll bug you back if I don't like it. ;-)
> 

Ditto


----

Normalize show-diff output and make sure we only show real changes after
changing the tracked branch.

Signed-off-by: Martin Schlemmer <azarah@nosferatu.za.org>

gittrack.sh:  a9d7c3d117390787e562a0450deb14c7cbf4b565 33
--- a9d7c3d117390787e562a0450deb14c7cbf4b565/gittrack.sh
+++ uncommitted/gittrack.sh
@@ -49,6 +49,7 @@

        read-tree $(tree-id "$name")
        gitdiff.sh local "$name" | gitapply.sh
+       update-cache --refresh

 else
        [ "$tracking" ] || \
@@ -59,6 +60,7 @@
        if [ -s ".git/HEAD.local" ]; then
                gitdiff.sh "$tracking" local | gitapply.sh
                read-tree $(tree-id local)
+               update-cache --refresh

                head=$(cat .git/HEAD)
                branchhead=$(cat .git/HEAD.$tracking)
show-diff.c:  136ec315b82d10d33ff9b4517f1b8ab91e3dcabc 33
--- 136ec315b82d10d33ff9b4517f1b8ab91e3dcabc/show-diff.c
+++ uncommitted/show-diff.c
@@ -5,13 +5,18 @@
  */
 #include "cache.h"

-static void show_differences(char *name,
+static void show_differences(struct cache_entry *ce,
        void *old_contents, unsigned long long old_size)
 {
        static char cmd[1000];
+       static char sha1[41];
+       int n;
        FILE *f;

-       snprintf(cmd, sizeof(cmd), "diff -L %s -u -N  - %s", name, name);
+       for (n = 0; n < 20; n++)
+               snprintf(&(sha1[n*2]), 3, "%02x", ce->sha1[n]);
+       snprintf(cmd, sizeof(cmd), "diff -L %s/%s -L uncommitted/%s -u -N  - %s",
+               sha1, ce->name, ce->name, ce->name);
        f = popen(cmd, "w");
        if (old_size)
                fwrite(old_contents, old_size, 1, f);
@@ -98,7 +103,7 @@

                fflush(stdout);
                new = read_sha1_file(ce->sha1, type, &size);
-               show_differences(ce->name, new, size);
+               show_differences(ce, new, size);
                free(new);
        }
        return 0;


-- 
Martin Schlemmer


[-- Attachment #1.2: git-show-diff.patch --]
[-- Type: text/x-patch, Size: 1514 bytes --]

gittrack.sh:  a9d7c3d117390787e562a0450deb14c7cbf4b565 33
--- a9d7c3d117390787e562a0450deb14c7cbf4b565/gittrack.sh
+++ uncommitted/gittrack.sh
@@ -49,6 +49,7 @@
 
 	read-tree $(tree-id "$name")
 	gitdiff.sh local "$name" | gitapply.sh
+	update-cache --refresh
 
 else
 	[ "$tracking" ] || \
@@ -59,6 +60,7 @@
 	if [ -s ".git/HEAD.local" ]; then
 		gitdiff.sh "$tracking" local | gitapply.sh
 		read-tree $(tree-id local)
+		update-cache --refresh
 
 		head=$(cat .git/HEAD)
 		branchhead=$(cat .git/HEAD.$tracking)
show-diff.c:  136ec315b82d10d33ff9b4517f1b8ab91e3dcabc 33
--- 136ec315b82d10d33ff9b4517f1b8ab91e3dcabc/show-diff.c
+++ uncommitted/show-diff.c
@@ -5,13 +5,18 @@
  */
 #include "cache.h"
 
-static void show_differences(char *name,
+static void show_differences(struct cache_entry *ce,
 	void *old_contents, unsigned long long old_size)
 {
 	static char cmd[1000];
+	static char sha1[41];
+	int n;
 	FILE *f;
 
-	snprintf(cmd, sizeof(cmd), "diff -L %s -u -N  - %s", name, name);
+	for (n = 0; n < 20; n++)
+		snprintf(&(sha1[n*2]), 3, "%02x", ce->sha1[n]);
+	snprintf(cmd, sizeof(cmd), "diff -L %s/%s -L uncommitted/%s -u -N  - %s",
+		sha1, ce->name, ce->name, ce->name);
 	f = popen(cmd, "w");
 	if (old_size)
 		fwrite(old_contents, old_size, 1, f);
@@ -98,7 +103,7 @@
 
 		fflush(stdout);
 		new = read_sha1_file(ce->sha1, type, &size);
-		show_differences(ce->name, new, size);
+		show_differences(ce, new, size);
 		free(new);
 	}
 	return 0;

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: Yet another base64 patch
From: H. Peter Anvin @ 2005-04-14  6:35 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: Christopher Li, git
In-Reply-To: <425E0D62.9000401@zytor.com>

H. Peter Anvin wrote:

> 
> Actually, the subdirectory hack has the same effect, so you lose 
> regardless.  Doesn't mean that you can't construct cases where the 
> subdirectory hack doesn't win, but I maintain that those are likely to 
> be artificial.
> 

That should, of course, be "... where the subdirectory hack does win ..."

Really, the subdirectory hack is a workaround for broken filesystems, 
and we don't use those anymore.

	-hpa

^ permalink raw reply

* Re: Yet another base64 patch
From: H. Peter Anvin @ 2005-04-14  6:27 UTC (permalink / raw)
  To: Christopher Li; +Cc: git
In-Reply-To: <20050414024228.GC18655@64m.dyndns.org>

Christopher Li wrote:
> 
> But if you write a large number of random files, when htree has three
> levels index. htree will suffer on the effect that it dirty random block
> very quickly, most block get dirty only contain one or two new entries.
> Ext3 will choke on it due to the limited journal size.
> 
> While non-index directory, new entry are very compact on the blocks.
> So it end up dirty a lot less blocks, of course, lookup will suffer.
> 
> Depend on you want check out fast or write a big tree fast, you can't
> win it all.
> 

Actually, the subdirectory hack has the same effect, so you lose 
regardless.  Doesn't mean that you can't construct cases where the 
subdirectory hack doesn't win, but I maintain that those are likely to 
be artificial.

It's probably worth noting that you have to assume htree is on, since 
that's the typical default for a Linux installation, even if you use the 
subdirectory hack.

	-hpa

^ permalink raw reply

* Re: Merge with git-pasky II.
From: Paul Jackson @ 2005-04-14  6:16 UTC (permalink / raw)
  To: Christopher Li; +Cc: torvalds, pasky, git
In-Reply-To: <20050414021602.GA18655@64m.dyndns.org>

> Oh, my bad. I am not trying to start a language war here.

Neither am I - no problem what so ever. <chuckle ...>

Besides, I think we'd be on the same side.

My point was only a gentle one -- as is often the case when dealing with
the strange species called human, whether or not you can get away with
something is often a simple matter of ones attitude.

There is one Python script already in the kernel: scripts/show_delta.
But it's too small a sample to mean much.

I think first thing is "get it right."  Python is good for that
in the hands of someone who enjoys coding in it.

I wish you well.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@engr.sgi.com> 1.650.933.1373, 1.925.600.0401

^ permalink raw reply

* Re: Yet another base64 patch
From: Christopher Li @ 2005-04-14  2:42 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: git
In-Reply-To: <425E0174.4080404@zytor.com>

On Wed, Apr 13, 2005 at 10:36:52PM -0700, H. Peter Anvin wrote:
> Christopher Li wrote:
> >On Wed, Apr 13, 2005 at 09:19:48PM -0700, H. Peter Anvin wrote:
> >
> >That is not surprising due to the directory index in ext3. Htree is pretty
> >good at random access and the hashed file name distribute evenly, that is
> >the best case for htree. 
> >
> 
> Right, so by not trying to do the filesystem's job for it we actually 
> come out ahead.
>

But if you write a large number of random files, when htree has three
levels index. htree will suffer on the effect that it dirty random block
very quickly, most block get dirty only contain one or two new entries.
Ext3 will choke on it due to the limited journal size.

While non-index directory, new entry are very compact on the blocks.
So it end up dirty a lot less blocks, of course, lookup will suffer.

Depend on you want check out fast or write a big tree fast, you can't
win it all.

Chris

 

^ permalink raw reply

* Re: Yet another base64 patch
From: H. Peter Anvin @ 2005-04-14  5:36 UTC (permalink / raw)
  To: Christopher Li; +Cc: git
In-Reply-To: <20050414022413.GB18655@64m.dyndns.org>

Christopher Li wrote:
> On Wed, Apr 13, 2005 at 09:19:48PM -0700, H. Peter Anvin wrote:
> 
>>Checking out the total kernel tree (time checkout-cache -a into an empty 
>>directory):
>>
>>	Cache cold	Cache hot
>>stock	3:46.95		19.95
>>base64	5:56.20		23.74
>>flat	2:44.13		15.68
> 
>>It seems that the flat format, at least on ext3 with dircache, is 
>>actually a major performance win, and that the second level loses quite 
>>a bit.
> 
> That is not surprising due to the directory index in ext3. Htree is pretty
> good at random access and the hashed file name distribute evenly, that is
> the best case for htree. 
> 

Right, so by not trying to do the filesystem's job for it we actually 
come out ahead.

	-hpa

^ permalink raw reply

* Re: Yet another base64 patch
From: Christopher Li @ 2005-04-14  2:24 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: git
In-Reply-To: <425DEF64.60108@zytor.com>

On Wed, Apr 13, 2005 at 09:19:48PM -0700, H. Peter Anvin wrote:
> Checking out the total kernel tree (time checkout-cache -a into an empty 
> directory):
> 
> 	Cache cold	Cache hot
> stock	3:46.95		19.95
> base64	5:56.20		23.74
> flat	2:44.13		15.68


> It seems that the flat format, at least on ext3 with dircache, is 
> actually a major performance win, and that the second level loses quite 
> a bit.

That is not surprising due to the directory index in ext3. Htree is pretty
good at random access and the hashed file name distribute evenly, that is
the best case for htree. 

Chris


^ permalink raw reply

* Re: Merge with git-pasky II.
From: Christopher Li @ 2005-04-14  2:16 UTC (permalink / raw)
  To: Paul Jackson; +Cc: torvalds, pasky, git
In-Reply-To: <20050413220341.13e5ce0f.pj@engr.sgi.com>

On Wed, Apr 13, 2005 at 10:03:41PM -0700, Paul Jackson wrote:
> 
> If you have a thin skin or tend to annoy others with a bit too much
> attitude or can't pass up a good language war (which is my failing, and
> why I am responding to a discussion that I've not been involved in for
> days) then the resulting flamage could be distracting.

Oh, my bad. I am not trying to start a language war here.
That is why I am hesitated about Python.
Just try to find out the acceptability. No pushing.

Chris 


^ 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