Git development
 help / color / mirror / Atom feed
* [PATCH] Add 'stg prev' and 'stg next'
From: Hannes Eder @ 2009-08-07 10:45 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git

These commands are related to 'stg top'.  They print the patch below
resp. above the topmost patch, given that they exist.

Signed-off-by: Hannes Eder <heder@google.com>
---
Ok, I confess it's not kill feature, but Mercurial has these commands
and I use them from time to time.  Maybe somebody else finds it useful
as well, so I decided to share it. ;)

Cheers,
Hannes

 stgit/commands/next.py |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 stgit/commands/prev.py |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 96 insertions(+), 0 deletions(-)
 create mode 100644 stgit/commands/next.py
 create mode 100644 stgit/commands/prev.py

diff --git a/stgit/commands/next.py b/stgit/commands/next.py
new file mode 100644
index 0000000..c8e2599
--- /dev/null
+++ b/stgit/commands/next.py
@@ -0,0 +1,48 @@
+__copyright__ = """
+Copyright (C) 2005, Catalin Marinas <catalin.marinas@gmail.com>
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License version 2 as
+published by the Free Software Foundation.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program; if not, write to the Free Software
+Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+"""
+
+from stgit.argparse import opt
+from stgit.commands import common
+from stgit.out import out
+from stgit import argparse
+
+help = 'Print the name of the next patch'
+kind = 'stack'
+usage = ['']
+description = """
+Print the name of the next patch."""
+
+args = []
+options = [
+    opt('-b', '--branch', args = [argparse.stg_branches],
+        short = 'Use BRANCH instead of the default branch')]
+
+directory = common.DirectoryHasRepositoryLib()
+
+def func(parser, options, args):
+    """Show the name of the next patch
+    """
+    if len(args) != 0:
+        parser.error('incorrect number of arguments')
+
+    stack = directory.repository.get_stack(options.branch)
+    unapplied = stack.patchorder.unapplied
+
+    if unapplied:
+        out.stdout(unapplied[0])
+    else:
+        raise common.CmdException, 'No unapplied patches'
diff --git a/stgit/commands/prev.py b/stgit/commands/prev.py
new file mode 100644
index 0000000..79f97a8
--- /dev/null
+++ b/stgit/commands/prev.py
@@ -0,0 +1,48 @@
+__copyright__ = """
+Copyright (C) 2005, Catalin Marinas <catalin.marinas@gmail.com>
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License version 2 as
+published by the Free Software Foundation.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program; if not, write to the Free Software
+Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+"""
+
+from stgit.argparse import opt
+from stgit.commands import common
+from stgit.out import out
+from stgit import argparse
+
+help = 'Print the name of the previous patch'
+kind = 'stack'
+usage = ['']
+description = """
+Print the name of the previous patch."""
+
+args = []
+options = [
+    opt('-b', '--branch', args = [argparse.stg_branches],
+        short = 'Use BRANCH instead of the default branch')]
+
+directory = common.DirectoryHasRepositoryLib()
+
+def func(parser, options, args):
+    """Show the name of the previous patch
+    """
+    if len(args) != 0:
+        parser.error('incorrect number of arguments')
+
+    stack = directory.repository.get_stack(options.branch)
+    applied = stack.patchorder.applied
+
+    if applied and len(applied) >= 2:
+        out.stdout(applied[-2])
+    else:
+        raise common.CmdException, 'Not enough applied patches'

^ permalink raw reply related

* Droppable Git Gui in Mac OS X
From: Tobia Conforto @ 2009-08-07 10:12 UTC (permalink / raw)
  To: git

Hello

I'm using Git 1.6.3.3 on Mac OS X 10.5.7, installed using MacPorts 1.710

I noticed that Git Gui cannot be launched on an existing repository by
simply dragging the repository folder onto Git Gui's icon in the dock,
but this is a standard UI feature in OS X.

Here is a fix "Git Gui.app" (which MacPorts installs in
/opt/local/share/git-gui/lib) to enable this feature.

The change to Info.plist enables dropping folders onto the application.
The modified AppMain.tcl, before launching the application, receives the
one pending OpenDocument event and cd's to the repository folder.

-Tobia


--- Git Gui.app/Contents/Info.plist
+++ Git Gui.app/Contents/Info.plist
@@ -24,5 +24,16 @@
 	<string>GITg</string>
 	<key>CFBundleVersion</key>
 	<string>0.12.0</string>
+	<key>CFBundleDocumentTypes</key>
+	<array>
+		<dict>
+			<key>CFBundleTypeOSTypes</key>
+			<array>
+				<string>fold</string>
+			</array>
+			<key>CFBundleTypeRole</key>
+			<string>Viewer</string>
+		</dict>
+	</array>
 </dict>
 </plist>
--- Git Gui.app/Contents/Resources/Scripts/AppMain.tcl
+++ Git Gui.app/Contents/Resources/Scripts/AppMain.tcl
@@ -19,4 +19,9 @@

 unset gitexecdir gitguilib
 set argv [lrange $argv 1 end]
-source $AppMain_source
+
+proc ::tk::mac::OpenDocument {args} {
+	cd [lindex $args 0]
+}
+
+after 1 { source $AppMain_source }

^ permalink raw reply

* Problem running 'make install' as root when on mounted fs
From: Alex Blewitt @ 2009-08-07 10:08 UTC (permalink / raw)
  To: git

I just tried building and installing Git on Mac OSX. My home directory  
is NFS mounted, to which I have write (but root doesn't).

When I do make, everything compiles OK
When I do sudo make install, I get '/bin/sh: GIT-BUILD-OPTIONS:  
permission denied'

It seems that make install is re-running the GIT-BUILD-OPTIONS target,  
and that tries to write to the current directory. In my case, on an  
NFS mounted home, root doesn't have write permissions to the local dir  
and so the install step fails.

I could build on a local partition (e.g. /tmp) but I'd prefer to have  
the code on my local drive so that I can do a git update and other  
such changes subsequently. I was able to get it to work by removing  
the echo>> from the GIT-BUILD-OPTIONS step after having built the code  
the first time.

The problem basically seems to be:

install: all
all:: ... GIT-BUILD-OPTIONS

I think it would be possible to restrict the GIT-BUILD-OPTIONS to be  
generated during a build phase, so restructuring the Makefile like:

install: build
all:: build GIT-BUILD-OPTIONS
build: ...

would then mean the install phase wouldn't have to touch the GIT-BUILD- 
OPTIONS step.

Does that sound reasonable? I'm happy to test it out and submit a  
patch if people agree (and/or have opinions on what the inner target  
'build' should be called)

Alex

^ permalink raw reply

* Re: [PATCH] git-ls-files.txt: clarify what "other files" mean for --other
From: Matthieu Moy @ 2009-08-07  8:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmy6cfz9j.fsf@alter.siamese.dyndns.org>

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

> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
>> ---
>> Actually, if it was not about backward compatibility, I'd just replace
>> -o, --other with -u, --untracked and talk about "untracked" everywhere,
>> but compatibility matters more than consistancy for plumbing.
>
> "ls-files -u" talks about (u)nmerged files.

(forgot this, so -u is even less an option)

> What consistency did you have in mind?

I found no place outside git ls-files where Git talks about "other"
files. Everywhere else, it talks about Untracked files.

As for the -u option, git status/commit -u[mode] is for Untracked
files, not unmerged ones.

--
Matthieu

^ permalink raw reply

* Re: [PATCH 0/7] block-sha1: improved SHA1 hashing
From: George Spelvin @ 2009-08-07  7:36 UTC (permalink / raw)
  To: torvalds; +Cc: art.08.09, git, linux

> Basically, older P4's will I think shift one bit at a time. So while even 
> Prescott is relatively weak in the shifter department, pre-prescott 
> (Willamette and Northwood) are _really_ weak. If your P4 is one of those, 
> you really shouldn't use it to decide on optimizations.

Thanks for the warning.  The only P4 I have a login on is a Willamette, so
I guess it's a bad optimization target:

processor       : 0
vendor_id       : GenuineIntel
cpu family      : 15
model           : 1
model name      : Intel(R) Pentium(R) 4 CPU 1.60GHz
stepping        : 2
cpu MHz         : 1593.888
cache size      : 256 KB
fdiv_bug        : no
hlt_bug         : no
f00f_bug        : no
coma_bug        : no
fpu             : yes
fpu_exception   : yes
cpuid level     : 2
wp              : yes
flags           : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pebs bts
bogomips        : 3187.77
clflush size    : 64
power management:



BTW, I'm having a bit of a problem with sha1bench.  Because the number
of rounds is multiplied by 10 until it takes more than mintime, and
the watchdog timer is set to 12*mintime, if I just barely miss the
threshold, I can hit the watchdog on any code that's more than 20% slower
than the reference rfc3174 code:

# /tmp/sha1bench 
#Initializing... Rounds: 10000000, size: 625000K, time: 6.904s, speed: 88.41MB/s
#             TIME[s] SPEED[MB/s]
rfc3174         6.912       88.31
# New hash result: 0489b02aee9fbd82b0bb0cba96f8047e42f543b8
rfc3174         6.911       88.31
linus           3.002       203.3
linusas         3.253       187.7
linusas2        3.059       199.6
mozilla         10.86       56.22
mozillaas       10.33       59.09
openssl         2.145       284.5
spelvin         1.933       315.8
spelvina        1.933       315.8
nettle          2.161       282.4

I had to bump it to 20 times to be able to get past the mozilla code.

You can still have nice round numbers with smaller incements of 2x or 2.5x:

    do {
-      rounds *= 10;
+      rounds *= 2;
+      if (rounds % 9 == 4)
+        rounds += rounds/4;     /* Step 1, 2, 5, 10, 20, 50, 100... */
       uWATCHDOG(mintime*20);
       t1 = GETTIME();
       for (i=0; i<rounds; i++) {
          SHA1Input(sc, arr512b, sizeof(arr512b));
          if (i<64) {
             SHA1Result(sc, arr512b+
                     (i+arr512b[i]+arr512b[arr512b[i]%64]+
                      arr512b[63-i]+arr512b[arr512b[63-i]%64]) %
                     (sizeof(arr512b)-20));
             SHA1Reset(sc);
             }
          }
       t2 = GETTIME(); td = t2-t1;
       }
    while (td<mintime);

And yeah, my P4 is touchy:
n# /var/tmp/sha1bench 
#Initializing... Rounds: 500000, size: 31250K, time: 0.9736s, speed: 31.35MB/s
#             TIME[s] SPEED[MB/s]
rfc3174        0.9931       30.73
# New hash result: 2872616106e163ae9c7c8d12a38bef032323c844
rfc3174        0.9491       32.16
linus          0.4906        62.2
linusas        0.5799       52.62
linusas2       0.4859       62.81
mozilla         1.302       23.44
mozillaas       1.234       24.74
openssl         0.226         135
spelvin        0.2298       132.8
spelvina       0.2494       122.4
nettle         0.3687       82.78

^ permalink raw reply

* Re: [ANNOUNCE] tortoisegit 0.9.1.0
From: John Tapsell @ 2009-08-07  6:48 UTC (permalink / raw)
  To: Tim Harper
  Cc: Frank Li, git, tortoisegit-dev, tortoisegit-users,
	tortoisegit-announce, tortoisegit
In-Reply-To: <e1a5e9a00908062225y112984d9gc0486ebcda25ab57@mail.gmail.com>

How hard would it be to port this to KDE?
What would be involved?


John

^ permalink raw reply

* Re: [PATCH 0/5] Suggested for PU: revision caching system to  significantly speed up packing/walking
From: Johannes Schindelin @ 2009-08-07  6:12 UTC (permalink / raw)
  To: Sam Vilain
  Cc: Nick Edelen, Michael J Gruber, Junio C Hamano, Jeff King,
	Shawn O. Pearce, Andreas Ericsson, Christian Couder,
	git@vger.kernel.org
In-Reply-To: <4A7B95A8.2010000@vilain.net>

Hi,

On Fri, 7 Aug 2009, Sam Vilain wrote:

> Johannes Schindelin wrote:
> >> the short answer is that cache slices are totally independant of pack 
> >> files.
> >>     
> >
> > My idea with that was that you already have a SHA-1 map in the pack 
> > index, and if all you want to be able to accelerate the revision 
> > walker, you'd probably need something that adds yet another mapping, 
> > from commit to parents and tree, and from tree to sub-tree and blob 
> > (so you can avoid unpacking commit and tree objects).
> >   
> 
> Tying indexes together like that is not a good idea in the database
> world.

This is not the same index as in the database world.  It is more 
comparable with a cached view.  And there, it is generally a good idea to 
keep related things in the same cached view (with an outer join), rather 
than having two primary keys for almost every record.

> Especially as in this case as Nick mentions, the domain is subtly 
> different (ie pack vs dag). Unfortunately you just can't try to pretend 
> that they will always be the same; you can't force a full repack on 
> every ref change!

No, but you do not need that, either.  In the setting that is most likely 
the most thankful one, i.e. a git:// server, you _want_ to keep the 
repository "as packed as possible", otherwise the rev cache improvements 
will be lost in the bad packing performance anyway.

> > Still, there is some redundancy between the pack index and your cache, 
> > as you have to write out the whole list of SHA-1s all over again.  I 
> > guess it is time to look at the code instead of asking stupid 
> > questions.
> >   
> 
> "Disk is cheap" :-)

Disk I/O ain't.

(Size of the I/O caches, yaddayadda, I'm sure you get my point).

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking
From: Johannes Schindelin @ 2009-08-07  6:08 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Sam Vilain, Nick Edelen, Michael J Gruber, Junio C Hamano,
	Jeff King, Shawn O. Pearce, Andreas Ericsson, Christian Couder,
	git@vger.kernel.org
In-Reply-To: <alpine.LFD.2.00.0908070031160.16073@xanadu.home>

Hi,

On Fri, 7 Aug 2009, Nicolas Pitre wrote:

> On Fri, 7 Aug 2009, Sam Vilain wrote:
> 
> > Johannes Schindelin wrote:
> > >> the short answer is that cache slices are totally independant of 
> > >> pack files.
> > >>     
> > >
> > > My idea with that was that you already have a SHA-1 map in the pack 
> > > index, and if all you want to be able to accelerate the revision 
> > > walker, you'd probably need something that adds yet another mapping, 
> > > from commit to parents and tree, and from tree to sub-tree and blob 
> > > (so you can avoid unpacking commit and tree objects).
> > >   
> > 
> > Tying indexes together like that is not a good idea in the database 
> > world. Especially as in this case as Nick mentions, the domain is 
> > subtly different (ie pack vs dag). Unfortunately you just can't try to 
> > pretend that they will always be the same; you can't force a full 
> > repack on every ref change!
> 
> Right.  And the rev cache must work even if the repository is not 
> packed.

Umm, why?  AFAICT the principal purpose of the rev cache is to help work 
loads on, say, www.kernel.org.

I am unlikely to notice the improvements in my regular "git log" calls 
that only show a couple of pages before I quit the pager.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking
From: Johannes Schindelin @ 2009-08-07  6:05 UTC (permalink / raw)
  To: Shawn O. Pearce
  Cc: A Large Angry SCM, Nick Edelen, Michael J Gruber, Junio C Hamano,
	Jeff King, Sam Vilain, Andreas Ericsson, Christian Couder,
	git@vger.kernel.org
In-Reply-To: <20090806233739.GL1033@spearce.org>



On Thu, 6 Aug 2009, Shawn O. Pearce wrote:

> A Large Angry SCM <gitzilla@gmail.com> wrote:
> > Shawn O. Pearce wrote:
> >> Nick Edelen <sirnot@gmail.com> wrote:
> >>> Hrmm, I just realized that it dosn't actually cache paths/names...
> >>
> >> You may not need the path name, but instead the hash value that 
> >> pack-objects computes from the path name.
> >
> > Please do NOT expose the hash values. The hash used by pack-objects is 
> > an implementation detail of the heuristics used by the _current_ 
> > object packing code. It would be a real shame to have to maintain 
> > backward compatibility with it at some future date after the packing 
> > machinery has changed.
> 
> This is a local cache.  If there was a version number in the header, and 
> the hash function changes, we could just bump the version number and 
> invalidate all of the caches.
> 
> No sense in storing (and doing IO of) huge duplicate string values for 
> something where we really only need 32 bits, and where a recompute from 
> scratch only costs a minute.

FWIW it was this redundancy in duplicate (unpacked) string redundancy 
I meant, but I did a poor job at expressing myself, and consequently Nick 
did not understand what I want (and I'm on a slow connection, so I deleted 
the mail halfway through looking if there are some real answers hidden in 
the huge quoted part instead of replying).

And the fragility of the dependency to the implementation detail of the 
pack index suggests to me that my intuition that this whole thing should 
be more tightly integrated with the pack index was not totally off the 
mark.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH] clarify error message when an abbreviated non-existent  commit was specified
From: Junio C Hamano @ 2009-08-07  5:36 UTC (permalink / raw)
  To: Tim Harper; +Cc: git
In-Reply-To: <e1a5e9a00908062217q4bd1ecafm5fd5e060aecfa467@mail.gmail.com>

Tim Harper <timcharper@gmail.com> writes:

>> diff --git a/parse-options.c b/parse-options.c
>> index 3b71fbb..95eb1c4 100644
>> --- a/parse-options.c
>> +++ b/parse-options.c
>> @@ -615,7 +615,7 @@ int parse_opt_with_commit(const struct option *opt, const char *arg, int unset)
>>        if (!arg)
>>                return -1;
>>        if (get_sha1(arg, sha1))
>> -               return error("malformed object name %s", arg);
>> +               return error("malformed object name or no such commit: %s", arg);
>>        commit = lookup_commit_reference(sha1);
>>        if (!commit)
>>                return error("no such commit %s", arg);
>> --
>> 1.6.4
>
> Does nobody think this is a good idea?

Probably people don't care enough.  I certainly didn't pay much attention
to the discussion on a rather trivial patch that was not yet signed off.

I'd probably write along this line instead, if I cared enough.  

	if (get_sha1(arg, sha1) ||
            !(commit = lookup_commit_reference(sha1)))
		return error("no such commit: %s", arg);

I think the important part of the message is that whatever the user gave
us when we expected to see a string that names a commit was not a commit;
it is immaterial if the failure was because an abbreviated hexadecimal
form was mistyped (get_sha1() would fail in this case) or because a tag
that points at a non commit, e.g. "v2.6.11-tree", was given (l-c-r will
fail in that case).

Giving two different messages depending on the nature of an error will
help debugging parse_opt_with_commit(), but that benefit is secondary.

^ permalink raw reply

* Re: [PATCH 02/13] Use an external program to implement fetching with curl
From: Daniel Barkalow @ 2009-08-07  5:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git, Johan Herland
In-Reply-To: <7v63d06wjq.fsf@alter.siamese.dyndns.org>

On Thu, 6 Aug 2009, Junio C Hamano wrote:

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > On Wed, 5 Aug 2009, Johannes Schindelin wrote:
> >
> >> Ooops, I missed this part.  How about making git-remote-https and 
> >> git-remote-ftp hardlinks to git-remote-http?
> >
> > Sure. Is "ln ... || ln -s ... || cp ..." the right way to do this 
> > cross-platform?
> 
> I've queued the first three from this series to 'next' (and the rest to
> 'pu'), as I wanted to give wider testing to the smaller footprint git with
> the libcurl-less-ness they bring in, with Linus's standalone SHA-1.
> 
> Since then two fix-ups (adding git-remote-* to .gitignore and the
> dependency fix to git-http-fetch) were posted to the list, which I also
> rebased in to the series, making the total number of patches merged to
> 'next' from the series 5.
>
> If there are major changes/rewrites/redesign in the remaining part of the
> series that are only in 'pu', please feel free to either send incrementals
> or replacements.

Great. Johannes had a bunch of suggestions for making it less error-prone 
to extend, which I hope to get to this weekend.

> I do not think I've seen any issues raised but unresolved against the part
> from this series already in 'next', other than that this builds three
> copies of git-remote-* programs based on libcurl.  I'll rebase a patch
> like this in after the Makefile fixup ae209bd (http-fetch: Fix Makefile
> dependancies, 2009-08-06) and queue the result to 'next'.

I think there were remaining improvements (e.g., transport-helper.c could 
write commands with strbufs), but nothing that couldn't just as well be 
applied afterwards.

> I suspect that the "install" target may need a patch similar to this one,
> though...

Ah, yes, avoiding the duplication in the build directory isn't a big win 
if the installed location doesn't get links.

> -- >8 --
> Subject: Makefile: do not link three copies of git-remote-* programs
> 
> Instead, link only one and make the rest hardlinks/copies, like we do for
> the built-ins. 
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> 
>  Makefile |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 2900057..38924f2 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1256,6 +1256,7 @@ ifndef V
>  	QUIET_LINK     = @echo '   ' LINK $@;
>  	QUIET_BUILT_IN = @echo '   ' BUILTIN $@;
>  	QUIET_GEN      = @echo '   ' GEN $@;
> +	QUIET_LNCP     = @echo '   ' LN/CP $@;
>  	QUIET_SUBDIR0  = +@subdir=
>  	QUIET_SUBDIR1  = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
>  			 $(MAKE) $(PRINT_DIR) -C $$subdir
> @@ -1494,10 +1495,16 @@ git-http-push$X: revision.o http.o http-push.o $(GITLIBS)
>  	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
>  		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
>  
> -git-remote-http$X git-remote-https$X git-remote-ftp$X: remote-curl.o http.o http-walker.o $(GITLIBS)
> +git-remote-http$X: remote-curl.o http.o http-walker.o $(GITLIBS)
>  	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
>  		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
>  
> +git-remote-https$X git-remote-ftp$X: git-remote-http$X
> +	$(QUIET_LNCP)$(RM) $@ && \
> +	ln git-remote-http$X $@ 2>/dev/null || \
> +	ln -s  git-remote-http$X $@ 2>/dev/null || \
> +	cp git-remote-http$X $@

Maybe "$<" for "git-remote-http$X", since that would make the rule portion 
generic.

^ permalink raw reply

* Re: [PATCH v2] Re: mailinfo: allow individual e-mail files as input
From: Junio C Hamano @ 2009-08-07  5:25 UTC (permalink / raw)
  To: Nicolas Sebrecht
  Cc: Brandon Casey, Junio C Hamano, giuseppe.bilotta, git,
	Brandon Casey
In-Reply-To: <20090807043929.GJ12924@vidovic>

Nicolas Sebrecht <nicolas.s.dev@gmx.fr> writes:

> It really seems like the argument has precedence to the redirection
> I couldn't find any reference to this case in POSIX and I guess that the
> behaviour may differ between implementations of sed.
> Perhaps somebody could tell us if our hesitation is justified (or not)?

If you give file arguments, standard input is irrelevant to sed.  I bet you
do:

	$ sed -e something file

all the time, and you rely on the command not to get stuck after reading
the file because now it wants to continue reading from its stdin, which is
connected to your terminal.  Have you ever worried about having to type ^D
after every such invocation of sed?  I bet you never have ;-)

Also, your _preferred_ alternative

	{ read l1; read l2; read l3;
          ( echo "$l1"; echo "$l2"; echo "$l3"; cat | sed )
	} <"$1"

is not just simply inefficient, but is actively wrong.  The "read" can
butcher your input.  For example, try:

	$ echo "a b " | ( read l1; echo "<$l1>" )

^ permalink raw reply

* Re: [ANNOUNCE] tortoisegit 0.9.1.0
From: Tim Harper @ 2009-08-07  5:25 UTC (permalink / raw)
  To: Frank Li
  Cc: git, tortoisegit-dev, tortoisegit-users, tortoisegit-announce,
	tortoisegit
In-Reply-To: <1976ea660908050700u9b16a8ci169825b121f02cb9@mail.gmail.com>

On Wed, Aug 5, 2009 at 8:00 AM, Frank Li<lznuaa@gmail.com> wrote:
> http://tortoisegit.googlecode.com/files/TortoiseGit-0.9.1.0-64bit.msi
> http://tortoisegit.googlecode.com/files/TortoiseGit-0.9.1.0-32bit.msi
>
> = Release 0.9.1.0 =
> == Features ==
>
>  * Add Sync Dialog like TortoiseHg.
>   Put pull, fetch, push, apply patch and email patch together. You
> can Know what change pull and push. Show changed file list.
>
>  * Enhance Rebase dialog. Add force rebase checkbox. Disable start
> button when no item rebase. Don't show merge commit
>
>  * Add post action button for rebase dialog. Such as send patch by email
>   After rebase, you can click button to send patch email directly.
>
>  * Add  launch rebase option at fetch dialog.
>
>  * Improve push dialog.
>   Default settings from local repositories pushing to remote
> repository Choose track remote and remote branch! Add short-cut at
> push dialog
>   Add "F5" refresh remote and branch info at push dialog
>
>  * Add Clean Untracked version type
>   User can choose clean untracked file, clean ignore file, clean all.
>
>  * Enhancement: "Git Clone" from SVN repository with additional start
> revision number option (-r xxx:HEAD)
>
>  * Improve Commit dialog
>   Add recent message back to context menu.
>   The "Message" field of the Commit dialog should have a shortcut key
> (Alt-M is a good choice)
>   Make "Whole project" directory checked by default when the user
> commits in the root of the project.
>   When using "Commit" to also add files, if you forget to check the
> new files and press "Ok", you get the dialog "Nothing Commit" and then
> the whole Commit dialog closes. Keep the dialog open after this
> message.
>
>
>  * Improve setting dialog
>   Settings: Git -> Remote: When creating the first remote, the
> "Remote" should have "origin" as default.
>   Fix setting dialog remote page tab order
>   Push: When you press "Manage" under Destinations, the Settings
> dialog opening should have "Git -> Remote" selected by default.
>
> == Bug Fix ==
>  * Fixed issue #124: Incorrect Date header in patch e-mail
>  * Fixed issue #122: Garbage text in "Git Command Progress" /
> suspected buffer overflow Improve commit speed when many added files.
>  * Fixed issue #121: Refresh in "Check for modifications" dialog
> duplicates added state entries when new repository
>  * Fixed issue #71: (TortoiseProc problem)Icon Overlays don't work in
> root of drive When m_CurrentDir =C:\, not C:,  pathlist calulate
> wrong.
>  * Fixed issue #116: SVN Rebase doesn't work
>  * Fixed issue #115: Windows XP: Initial "Git Clone..." from SVN
> Repository doesn't work
>  * Fix "ESC" = "push" when after commit and Add Alt-P  for Push
>  * Fixed issue #111: Undo Add does not work (keep added file) and
> enable "F5" at revert dialog
>  * Fixed issue #109: clone on bare local repository fails Clear trail
> slash \ or/
>  * Fixed issue #104: Doubleclicking changed submodule dir in Check For
> Modifications dlg, crashes TGit Fix log dialog double click submodule
> problem
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordo
mo info at  http://vger.kernel.org/majordomo-info.html
>

Frank Li,

Thank you for your work on TortoiseGit.  I feel more and more
confident recommending git to my winders friends because of it.

Tim

^ permalink raw reply

* Re: [PATCH] clarify error message when an abbreviated non-existent  commit was specified
From: Tim Harper @ 2009-08-07  5:17 UTC (permalink / raw)
  To: git
In-Reply-To: <1249588435-23400-1-git-send-email-timcharper@gmail.com>

On Thu, Aug 6, 2009 at 1:53 PM, Tim Harper<timcharper@gmail.com> wrote:
> When running the command 'git branch --contains efabdfb' on a repository that doesn't yet have efabdfb, git reports: "malformed object name efabdfb". To the uninitiated, this makes little sense (as far as they are concerned, efabdfb is perfectly formed).
>
> This commit changes the message to "malformed object name or no such commit: efabdfb"
> ---
>  parse-options.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/parse-options.c b/parse-options.c
> index 3b71fbb..95eb1c4 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -615,7 +615,7 @@ int parse_opt_with_commit(const struct option *opt, const char *arg, int unset)
>        if (!arg)
>                return -1;
>        if (get_sha1(arg, sha1))
> -               return error("malformed object name %s", arg);
> +               return error("malformed object name or no such commit: %s", arg);
>        commit = lookup_commit_reference(sha1);
>        if (!commit)
>                return error("no such commit %s", arg);
> --
> 1.6.4
>
>

Does nobody think this is a good idea?

^ permalink raw reply

* Re: [PATCH 02/13] Use an external program to implement fetching with curl
From: Junio C Hamano @ 2009-08-07  5:08 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: Johannes Schindelin, git, Johan Herland
In-Reply-To: <alpine.LNX.2.00.0908051145250.2147@iabervon.org>

Daniel Barkalow <barkalow@iabervon.org> writes:

> On Wed, 5 Aug 2009, Johannes Schindelin wrote:
>
>> Ooops, I missed this part.  How about making git-remote-https and 
>> git-remote-ftp hardlinks to git-remote-http?
>
> Sure. Is "ln ... || ln -s ... || cp ..." the right way to do this 
> cross-platform?

I've queued the first three from this series to 'next' (and the rest to
'pu'), as I wanted to give wider testing to the smaller footprint git with
the libcurl-less-ness they bring in, with Linus's standalone SHA-1.

Since then two fix-ups (adding git-remote-* to .gitignore and the
dependency fix to git-http-fetch) were posted to the list, which I also
rebased in to the series, making the total number of patches merged to
'next' from the series 5.

If there are major changes/rewrites/redesign in the remaining part of the
series that are only in 'pu', please feel free to either send incrementals
or replacements.

I do not think I've seen any issues raised but unresolved against the part
from this series already in 'next', other than that this builds three
copies of git-remote-* programs based on libcurl.  I'll rebase a patch
like this in after the Makefile fixup ae209bd (http-fetch: Fix Makefile
dependancies, 2009-08-06) and queue the result to 'next'.

I suspect that the "install" target may need a patch similar to this one,
though...

-- >8 --
Subject: Makefile: do not link three copies of git-remote-* programs

Instead, link only one and make the rest hardlinks/copies, like we do for
the built-ins. 

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 Makefile |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 2900057..38924f2 100644
--- a/Makefile
+++ b/Makefile
@@ -1256,6 +1256,7 @@ ifndef V
 	QUIET_LINK     = @echo '   ' LINK $@;
 	QUIET_BUILT_IN = @echo '   ' BUILTIN $@;
 	QUIET_GEN      = @echo '   ' GEN $@;
+	QUIET_LNCP     = @echo '   ' LN/CP $@;
 	QUIET_SUBDIR0  = +@subdir=
 	QUIET_SUBDIR1  = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
 			 $(MAKE) $(PRINT_DIR) -C $$subdir
@@ -1494,10 +1495,16 @@ git-http-push$X: revision.o http.o http-push.o $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
 		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
 
-git-remote-http$X git-remote-https$X git-remote-ftp$X: remote-curl.o http.o http-walker.o $(GITLIBS)
+git-remote-http$X: remote-curl.o http.o http-walker.o $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
 		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
 
+git-remote-https$X git-remote-ftp$X: git-remote-http$X
+	$(QUIET_LNCP)$(RM) $@ && \
+	ln git-remote-http$X $@ 2>/dev/null || \
+	ln -s  git-remote-http$X $@ 2>/dev/null || \
+	cp git-remote-http$X $@
+
 $(LIB_OBJS) $(BUILTIN_OBJS): $(LIB_H)
 $(patsubst git-%$X,%.o,$(PROGRAMS)) git.o: $(LIB_H) $(wildcard */*.h)
 builtin-revert.o wt-status.o: wt-status.h

^ permalink raw reply related

* Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking
From: Nicolas Pitre @ 2009-08-07  4:42 UTC (permalink / raw)
  To: Nick Edelen
  Cc: Johannes Schindelin, Michael J Gruber, Junio C Hamano, Jeff King,
	Sam Vilain, Shawn O. Pearce, Andreas Ericsson, Christian Couder,
	git@vger.kernel.org
In-Reply-To: <c77435a80908061330h2461012at8b877970cab4906b@mail.gmail.com>

On Thu, 6 Aug 2009, Nick Edelen wrote:

> Hrmm, I just realized that it dosn't actually cache paths/names...
> This obviously has no bearing on its use in packing, but I should
> either add that in or restrict usage in non-packing-related walks.
> Weird how things like that escape you.

Actually it is really the packing related walk that would benefit the 
most from this work.  The "counting objects" phase of a clone may take 
quite a while with some repositories.  Most other operations don't care 
as much because the rev walk is done incrementally whereas the packing 
operation needs to perform it all up front.


Nicolas

^ permalink raw reply

* [PATCH v2] Re: mailinfo: allow individual e-mail files as input
From: Nicolas Sebrecht @ 2009-08-07  4:39 UTC (permalink / raw)
  To: Brandon Casey
  Cc: Junio C Hamano, Nicolas Sebrecht, giuseppe.bilotta, git,
	Brandon Casey
In-Reply-To: <7GvFnE4br-8WnXmtoea9V1LPY-qshCw6arPr6H40SRG59-b7YcpTsw@cipher.nrlssc.navy.mil>

The 06/08/09, Brandon Casey wrote:

> The "former", or Junio's original patch, effectively has this form:
> 
>    {
>       sed "$1"
>    } < "$1"
> 
> Without reading closely enough, I thought it looked like this:
> 
>    {
>       sed
>    } < "$1"
> 
> Since I didn't study the sed statement closely enough, I assumed that it was
> operating on the remaining portion of the patch email that was redirected to
> the block on stdin.  I missed the fact that the file name was supplied to
> it.  My comment was that I found it strange (and maybe unintuitive, or maybe
> it's just me) that "$1" was piped on stdin and it was supplied as an
> argument to sed.

Thinking to this a bit more, I tend to think that your intention to get
rid of the "$1" argument of sed is the right thing to do.

It really seems like the argument has precedence to the redirection

  _but_

I couldn't find any reference to this case in POSIX and I guess that the
behaviour may differ between implementations of sed. I don't know.
Perhaps somebody could tell us if our hesitation is justified (or not)?

Finally and to prevent strange behaviours, I would write

  {
    real l1
    real l2
    real l3
    {
      echo "$l1"
      echo "$l2"
      echo "$l3"
      cat
    } | sed
  } < "$1"

instead of

  {
    real l1
    real l2
    real l3
    sed "$1"
  } < "$1"

because the latter may contain either the content of the whole file
(coming from the argument) or the content of the file _whithout_ the
first three lines (coming from the redirection '<' amputated by the
'read' statements).

Junio?

-- 
Nicolas Sebrecht

^ permalink raw reply

* Re: [PATCH 0/5] Suggested for PU: revision caching system to significantly speed up packing/walking
From: Nicolas Pitre @ 2009-08-07  4:35 UTC (permalink / raw)
  To: Sam Vilain
  Cc: Johannes Schindelin, Nick Edelen, Michael J Gruber,
	Junio C Hamano, Jeff King, Shawn O. Pearce, Andreas Ericsson,
	Christian Couder, git@vger.kernel.org
In-Reply-To: <4A7B95A8.2010000@vilain.net>

On Fri, 7 Aug 2009, Sam Vilain wrote:

> Johannes Schindelin wrote:
> >> the short answer is that cache slices are totally independant of pack 
> >> files.
> >>     
> >
> > My idea with that was that you already have a SHA-1 map in the pack index, 
> > and if all you want to be able to accelerate the revision walker, you'd 
> > probably need something that adds yet another mapping, from commit to 
> > parents and tree, and from tree to sub-tree and blob (so you can avoid 
> > unpacking commit and tree objects).
> >   
> 
> Tying indexes together like that is not a good idea in the database
> world. Especially as in this case as Nick mentions, the domain is subtly
> different (ie pack vs dag). Unfortunately you just can't try to pretend
> that they will always be the same; you can't force a full repack on
> every ref change!

Right.  And the rev cache must work even if the repository is not 
packed. So pack index and rev caching are orthogonal things and are best 
kept separate on disk.

How big this cache might get would be interesting indeed.


Nicolas

^ permalink raw reply

* Re: Performance issue of 'git branch'
From: Jeff King @ 2009-08-07  4:21 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Junio C Hamano, Git Mailing List, Carlos R. Mafra,
	Daniel Barkalow, Johannes Schindelin
In-Reply-To: <alpine.LFD.2.01.0907241505400.3960@localhost.localdomain>

On Fri, Jul 24, 2009 at 03:13:07PM -0700, Linus Torvalds wrote:

> Subject: [PATCH] git-http-fetch: not a builtin
> 
> We should really try to avoid having a dependency on the curl libraries
> for the core 'git' executable. It adds huge overheads, for no advantage.
> 
> This splits up git-http-fetch so that it isn't built-in.  We still do
> end up linking with curl for the git binary due to the transport.c http
> walker, but that's at least partially an independent issue.
>
> [...]
>
> +git-http-fetch$X: revision.o http.o http-push.o $(GITLIBS)
> +	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
> +		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)

Err, this seems to horribly break git-http-fetch (see if you can spot
the logic error in dependencies). Patch is below.

Nobody noticed, I expect, because nothing in git _uses_ http-fetch
anymore, now that git-clone is no longer a shell script. I only noticed
because it tried to build http-push on one of my NO_EXPAT machines.

It might be an interesting exercise to dust off the old shell scripts
once in a while and see if they still pass their original tests while
running on top of a more modern git. It would test that we haven't
broken the plumbing interfaces.

-- >8 --
Subject: [PATCH] Makefile: build http-fetch against http-fetch.o

As opposed to http-push.o. We can also drop EXPAT_LIBEXPAT,
since fetch does not need it.

This appears to be a bad cut-and-paste in commit 1088261f.

Signed-off-by: Jeff King <peff@peff.net>
---
 Makefile |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 97d904b..d6362d3 100644
--- a/Makefile
+++ b/Makefile
@@ -1502,9 +1502,9 @@ http.o http-walker.o http-push.o: http.h
 
 http.o http-walker.o: $(LIB_H)
 
-git-http-fetch$X: revision.o http.o http-push.o $(GITLIBS)
+git-http-fetch$X: revision.o http.o http-fetch.o http-walker.o $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
-		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
+		$(LIBS) $(CURL_LIBCURL)
 git-http-push$X: revision.o http.o http-push.o $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
 		$(LIBS) $(CURL_LIBCURL) $(EXPAT_LIBEXPAT)
-- 
1.6.4.117.g6056d.dirty

^ permalink raw reply related

* Re: [PATCH 0/7] block-sha1: improved SHA1 hashing
From: Artur Skawina @ 2009-08-07  4:16 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.2.01.0908061909310.3390@localhost.localdomain>

Linus Torvalds wrote:
> 
> Here's the plain "linus" baseline (ie the "Do register rotation in cpp") 
> thing, with the fixed "E += TEMP .." thing):

> 	linus          0.4018       151.9

> and here it is with your patch:

> 	linus          0.4653       131.2

> (ok, so the numbers aren't horribly stable, but the "plain linus" thing 
> consistently outperforms here - and underperforms with your patch).

Well, I'd be surprised if one C version would always be the winner on
every single cpu; that 13% loss[1] I think would be an acceptable compromise,
if the goal is to have one implementation that does reasonably well on all
cpus.

That's why i asked how the change did on nehalem; if it's a measurable loss
on anything modern (core2+), then of course the P4s must suffer; and one
could always blame the compiler ;)
It's not like the difference in sha1 overhead will be noticeable in normal
git use.

artur

[1] I suspect the old gcc is a factor (4.0.4 does <100M/s here).

^ permalink raw reply

* Re: What's in git.git (Aug 2009, #01; Wed, 05)
From: Junio C Hamano @ 2009-08-07  3:55 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Brandon Casey, Nicolas Sebrecht, git
In-Reply-To: <20090807123346.6117@nanako3.lavabit.com>

Nanako Shiraishi <nanako3@lavabit.com> writes:

> And Nicolas Sebrecht has been working with Junio to implement an
> enhancement to add support for the "individual piece of email" format.

I'll squash patches 1+2/3 from Brandon into a single change after
retitling it to read "am" not "mailinfo", and queue the result, together
with patch 3/3.

Thanks, Brandon and Nicolas.

^ permalink raw reply

* Re: [PATCH 2/3] mailinfo: allow individual e-mail files as input
From: Junio C Hamano @ 2009-08-07  3:37 UTC (permalink / raw)
  To: Brandon Casey; +Cc: ni.s, giuseppe.bilotta, git, Brandon Casey
In-Reply-To: <COrzR9ThNBy5SQ7chsXyUB30jVGIijxZQ3LI9L_y7Ab5vWcDcy_HolvjjuHTC7DHI9ntV-eR_v0@cipher.nrlssc.navy.mil>

Brandon Casey <casey@nrlssc.navy.mil> writes:

> You'll notice that I changed your grep -E to an egrep and dropped the -e.

That's the right thing to do.  I was re-reading Nicolas Sebrecht's series
and checked with  "git grep -e 'egrep ' -e 'grep .*-E '" to reach the same
conclusion.  Thanks.

^ permalink raw reply

* Re: What's in git.git (Aug 2009, #01; Wed, 05)
From: Nanako Shiraishi @ 2009-08-07  3:33 UTC (permalink / raw)
  To: Brandon Casey; +Cc: Junio C Hamano, git
In-Reply-To: <MEhvdM_GHnyaFj9ZU3lxKS47vmOk5BKslGm0FxkE_lg0SQT5Zx6KhA@cipher.nrlssc.navy.mil>

Quoting Brandon Casey <brandon.casey.ctr@nrlssc.navy.mil>

> Junio C Hamano wrote:
>> The 1.6.4 release seems to have been quite solid, and there is no
>> brown-paper-bag bugfixes on 'maint' yet ;-).
>
> Found one.
>
> I didn't realize the whole git-am discussion did _not_ result in a
> fix being applied.  But git-am will currently refuse to apply any
> patch from email that does not have "From " or "From: " in the first
> three lines of the email.  For those of us whose mail servers prepend
> many lines of the form:
>
> Received: from XXX ([XXX]) by XXX with Microsoft SMTPSVC(6.0.3790.2825);
> 	 Tue, 14 Jul 2009 07:24:06 -0500

According to an already hashed out discussion, that isn't a mbox format that has been supported, so it isn't even a bug. For details, see e.g.

    http://thread.gmane.org/gmane.comp.version-control.git/123338/focus=123355

And Nicolas Sebrecht has been working with Junio to implement an enhancement to add support for the "individual piece of email" format.

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

^ permalink raw reply

* Re: [PATCH 5/5] full integration of rev-cache into git's revision walker, completed test suite
From: Sam Vilain @ 2009-08-07  3:22 UTC (permalink / raw)
  To: Nick Edelen
  Cc: Junio C Hamano, Johannes Schindelin, Jeff King, Shawn O. Pearce,
	Andreas Ericsson, Christian Couder, git@vger.kernel.org
In-Reply-To: <op.ux8i7ceotdk399@sirnot.private>

Nick Edelen wrote:
> This last patch provides a working integration of rev-cache into the revision 
> walker, along with some touch-ups:
>   

"This last patch" is redundant.  You can just write "Integrate rev-cache
into the revision walker;"

>  - integration into revision walker and list-objects
>  - addition of 'unique' field to commit objects, optionally initialized in 
>    rev-cache with the objects introduced in that commit
>  - tweak of object generation to take advantage of the 'unique' field
>  - more fluid handling of damaged cache slices
>  - numerous tests for both features from the previous patch, and the 
> integration's integrity
>   

Does it make sense to split this integration up or stage it somehow?

> 'Integration' is rather broad -- a more detailed description follows for each 
> aspect:
>  - rev-cache
> the traversal mechanism is updated to handle many of the non-prune options 
> rev-list does (date limiting, slop-handling, etc.), and is adjusted to allow 
> for non-fatal cache-traversal failures.
>
>  - revision walker
> both limited and unlimited traversal attempt to use the cache when possible, 
> smoothly falling back if it's not.
>
>  - list-objects
> object listing does not recurse into cached trees, and has been adjusted to 
> guarantee commit-tag-tree-blob ordering.
>   

Especially given the length of this part of the commit message.

Sam

^ permalink raw reply

* Re: [PATCH 1/5] revision caching documentation: man page and technical discussion
From: Sam Vilain @ 2009-08-07  3:08 UTC (permalink / raw)
  To: Nick Edelen
  Cc: Junio C Hamano, Johannes Schindelin, Jeff King, Shawn O. Pearce,
	Andreas Ericsson, Christian Couder, git@vger.kernel.org
In-Reply-To: <op.ux8i6lq9tdk399@sirnot.private>

Nick Edelen wrote:
> Before any code is introduced the full documentation is put forth.  This 
> provides a man page for the porcelain, and a technical doc in technical/.  The 
> latter describes the API, and discusses rev-cache's design, file format and 
> mechanics.
>
> Signed-off-by: Nick Edelen <sirnot@gmail.com>
>
> ---
>  Documentation/rev-cache.txt           |   51 +++++
>  Documentation/technical/rev-cache.txt |  336 +++++++++++++++++++++++++++++++++
>  2 files changed, 387 insertions(+), 0 deletions(-)
>   

A couple of minor nits here:

1. Documentation/rev-cache.txt should be
Documentation/git-rev-cache.txt, because it documents that command.

2. there are many whitespace errors:

wilber:~/src/git$ git am \[PATCH\ 1_5\]\ revision\ caching\
documentation\:\ man\ page\ and\ technical\ discussion.eml
Applying: revision caching documentation: man page and technical discussion
/home/samv/src/git/.git/rebase-apply/patch:15: trailing whitespace.
A front end for the rev-cache API is provided with the builtin utility
/home/samv/src/git/.git/rebase-apply/patch:16: trailing whitespace.
`rev-cache`.  It is mainly intended for cache slice generation and
maintenance,
/home/samv/src/git/.git/rebase-apply/patch:17: trailing whitespace.
but can also walk commits within a slice.  At the moment it is not
particularly
/home/samv/src/git/.git/rebase-apply/patch:34: trailing whitespace.
`\--stdin`:: Also read commit ids from stdin (seperated by newline,
`\--not`
/home/samv/src/git/.git/rebase-apply/patch:51: trailing whitespace.
`\--all`:: Include all objects in repository.  Will only traverse as of
cache
warning: squelched 166 whitespace errors
warning: 171 lines add whitespace errors.

Be sure to check the patch doesn't do that...

> diff --git a/Documentation/rev-cache.txt b/Documentation/rev-cache.txt
> new file mode 100755
> index 0000000..64bd051
> --- /dev/null
> +++ b/Documentation/rev-cache.txt
> @@ -0,0 +1,51 @@
> +rev-cache porcelain
> +===================
> +
> +A front end for the rev-cache API is provided with the builtin utility 
> +`rev-cache`.  It is mainly intended for cache slice generation and maintenance, 
> +but can also walk commits within a slice.  At the moment it is not particularly 
> +advanced, but is sufficient for repository administration.
>   

That last sentence is a bit unnecessarily self-doubting; "It currently
provides basic functionality" would be fine.

> +
> +It's general syntax is:
> +
> +`git-rev-cache COMMAND [options] [<commit-id>...]`
> +
> +With the commands:
>   

You should use the same structure and headings of this file as the other
man pages which are found under Documentation/git-*.txt

> +
> +`add`::
> +	Add revisions to the cache.  Reads commit ids from stdin, formatted as:
> +	`END END ... \--not START START ...`
>   

Really, it's reading a revision list; might be better to call it that. 
Can it accept the same options as git-rev-list here or just a list of
starting points/tips?  And "END" and "START" are still backwards to the
rest of git core!

> ++
> +Options:
> +
> +`\--all`:: Include all heads as ends.
> +`\--fresh`:: Exclude everything already in a cache slice.
>   

If you use the paragraph layout used by other commands you can explain a
little bit more about what you mean here.  By "Cache Slice" do you mean
"Revision Cache"?  Does that --fresh imply that all of the revisions
which were not indexed will automatically get indexed?

> +`\--stdin`:: Also read commit ids from stdin (seperated by newline, `\--not` 
> +also valid).
> +`\--legs`:: Ensure branch has no "dangling" starts (ie. is self-contained).
>   

The --legs term is a little too 'cute', is there a better way to
describe this?  And "starts" is not a real word in that context; if you
are using a technical term, you should define it..

> +`\--noobjects`:: Don't include non-commit objects.
> +
> +`walk`::
> +	Walk a cache slice based on set of commits; formatted as add.
>   

So, this works like 'rev-list' ?

"Formatted" is wrong there I think; do you mean it 'accepts the same
arguments as add' ?

> ++
> +Options:
> +
> +`\--objects`:: Include non-commit objects in traversal.
> +
>   

Which was the default?  --objects or --no-objects?

> +`fuse`::
> +	Coagulate several cache slices into a single large slice.
>   

Coagulate?  You mean, the revision caches will stop being liquid and go
gluggy, like a pool of blood clotting?

How about "combine" :-) - and the option might be better called
something simple like that, too.

> ++
> +Options:
> +
> +`\--all`:: Include all objects in repository.  Will only traverse as of cache 
> +ends if this is not specified.
>   

That sentence doesn't quite read well to me.

> +`\--noobjects`:: Don't include non-commit objects.
> +`\--ignore-size[=N]`:: Do not fuse slices of file size >= `N`.  If `N` is not 
> +given the cutoff size defaults to ~5MB.
> +
> +`index`::
> +	Regenerate the cache index.
> +
> +
> +For an explanation of the API and its inner workings, see 
> +link:technical/rev-cache.txt[technical info on rev-cache]
> diff --git a/Documentation/technical/rev-cache.txt b/Documentation/technical/rev-cache.txt
> new file mode 100755
> index 0000000..e95ec89
> --- /dev/null
> +++ b/Documentation/technical/rev-cache.txt
> @@ -0,0 +1,336 @@
> +rev-cache
> +=========
> +
> +The revision cache API ('rev-cache') provides a method for efficiently storing 
> +and accessing commit branch sections.  Such branch slices are defined by a 
> +series of top (interesting) and bottom (uninteresting) commits.  Each slice 
> +contains, per commit:
> +
> +* All intra-slice topological relations, encoded into path "channels" (see 
> +  'Mechanics' for full explanation).
> +* Object meta-data: type, SHA-1, size, date (for commits).
> +* Objects introduced in that commit, relative to slice (ie. only for non-start 
> +  commits).
> +
> +Storage data structures are not exported, in part to keep git's global scope 
> +clean, but largely because they're pretty much useless outside of rev-cache.
> +
> +The API
> +-------
> +
> +The API for 'rev-cache' is quite simple.  You can find the function prototypes 
> +in `revision.h`.
> +
> +Data Structures
> +~~~~~~~~~~~~~~~
> +
> +The `rev_cache_info` struct holds all the options and flags for the API.
> +
> +----
> +struct rev_cache_info {
> +        /* generation flags */
> +        unsigned objects : 1, 
> +                legs : 1, 
> +                make_index : 1;
> +        
> +        /* traversal flags */
> +        unsigned save_unique : 1, 
> +                add_to_pending : 1;
> +        
> +        /* fuse options */
> +        unsigned int ignore_size;
> +};
> +----
> +
> +The fields:
> +
> +`objects`:: Add non-commit objects to slice.
> +`legs`:: Ensure bottoms have no childrens.
>   

"children" is already plural and needs no 's'

> +`make_index`:: Integrate newly-made slice into index.
> +`save_unique`:: Load unique non-commit objects into `unique` field of each 
> +`commit` object.
>   

Unique how?  Do you mean, that they were introduced in this slice and
not reachable from any of the bottom/end commits?

> +`add_to_pending`:: Append unique non-commit objects to the `pending` object 
> +list in the passed `rev_info` instance.
>   

That doesn't make much sense to me either.  What does that mean?  Well,
I'll read on.

> +`ignore_size`:: If non-zero, ignore slices with size greater or equal to this.
>   

What will this ignoring mean?

> +Functions
> +~~~~~~~~~
> +
> +`init_rci`::
> +
> +        Initiate a `rev_cache_info` struct to default options.  
> +
> +`make_cache_slice`::
> +
> +        Create a cache based on an a `rev_info` instance or `commit_list` s of 
> +        "tops" and "bottoms" (defaulting to latter if `rev_info` pointer is 
> +        NULL), copying the cache SHA-1 into a passed pointer if non-zero.  A 
> +        `rev_cache_info` struct pointer can be passed to set options, while 
> +        passing NULL will set default options.  A last parameter can 
> +        optionally recieve the final cache hash.
> +
> +`make_cache_index`::
> +
> +        Add a slice to the cache index.  Requires a file descriptor, the cache 
> +        hash and the file size.  Note that this is normally called by 
> +        `make_cache_slice` under the `make_index` option.
> +
> +`get_cache_slice`::
> +
> +        Given a commit SHA-1 `get_cache_slice` will search the slice index and 
> +        return, if found, the cache-identifying SHA-1.
> +
> +`traverse_cache_slice`::
> +
> +        Traverse a specified cache slice based on:
> +
> +        * `rev_cache_info` instance (optional)
> +        * cache SHA-1 identifier
> +        * `rev_info` instance
> +        * a starting commit and commit work list
> +        * date of oldest encountered interesting commit
> +        * current `slop` (this and above mainly used in integration with 
> +          revision walker)
>   

Hmm what's a 'slop' ?

> +        
> ++
> +The output is sent to a FILO `commit_list` "queue", while any bottom commits 
> +are passed back into the work list.  If the walk is not contained within the 
> +slice, commit boundaries are also inserted into "work".
> +
> +`tops_from_slices`::
> +
> +        Will mark all top-commits in the specified cache slices with a given 
> +        flag, and add them to the rev pending list.  Will include all if no 
> +        slices are specified.
> +
> +`coagulate_cache_slices`::
> +
> +        Generate a slice based on the passed `rev_info` instance, replacing all 
> +        encountered slices with one (larger) slice.  The `ignore_size` field in 
> +        `rci`, if non-zero, will dictate which cache slice sizes to ignore in 
> +        both traversal and replacement.
> +
> +`regenerate_index`::
> +
> +        Remake cache index.
> +
> +Example Usage
> +-------------
> +
> +A few examples to demonstrate usage:
> +
> +.Creating a slice
> +----
> +/* pretend you're a porcelain for rev-cache reading from the command line */
> +struct rev_info revs;
> +struct rev_cache_info rci;
> +
> +init_revisions(&revs, 0);
> +init_rci(&rci);
> +
> +flags = 0;
> +for (i = 1; i < argc; i++) {
> +        if (!strcmp(argv[i], "--not"))
> +                flags ^= UNINTERESTING;
> +        else if(!strcmp(argv[i], "--fresh"))
> +                starts_from_slices(&revs, UNINTERESTING, 0, 0);
> +        else
> +                handle_revision_arg(argv[i], &revs, flags, 1);
> +}
> +
> +/* we want to explicitly set certain options */
> +rci.objects = 0;
> +
> +if (!make_cache_slice(&rci, &revs, 0, 0, cache_sha1))
> +        printf("made slice!  it's called %s\n", sha1_to_hex(cache_sha1));
>   

Heh, normally it's acceptable to let examples in documentation not be
complete working programs :)

> +----
> +
> +.Traversing a slice
> +----
> +/* let's say you're walking the tree with a 'work' list of current heads and a 
> + * FILO output list 'out' */
> +out = 0;
> +outp = &out;
> +
> +while (work) {
> +        struct commit *commit = pop_commit(&work);
> +        struct object *object = &commit->object;
> +        unsigned char *cache_sha1;
> +        
> +        if (cache_sha1 = get_cache_slice(object->sha1)) {
> +                /* note that this will instatiate any topo-relations 
> +                 * as it goes */
> +                if (traverse_cache_slice(&revs, cache_sha1, 
> +                        commit, 0, 0, /* use defaults */
> +                        &outp, &work) < 0)
> +                        die("I'm overreacting to a non-fatal cache error");
> +        } else {
> +                struct commit_list *parents = commit->parents;
> +                
> +                while (parents) {
> +                        struct commit *p = parents->item;
> +                        struct object *po = &p->object;
> +                        
> +                        parents = parents->next;
> +                        if (po->flags & UNINTERESTING)
> +                                continue;
> +                        
> +                        if (object->flags & UNINTERESTING)
> +                                po->flags |= UNINTERESTING;
> +                        else if (po->flags & SEEN)
> +                                continue;
> +                        
> +                        if (!po->parsed)
> +                                parse_commit(p);
> +                        insert_by_date(p, &work);
> +                }
> +                
> +                if (object->flags & (SEEN | UNINTERESTING) == 0)
> +                        outp = &commit_list_insert(commit, outp);
> +                object->flags |= SEEN;
> +        }
> +}
> +----
>   

Ok, good - perhaps needs a comment or two - not much, it's already long.

> +
> +Some Internals
> +--------------
> +
> +Although you really don't need to know anything about how rev-cache actually 
> +does its magic shizz, a bit of background may go a long way if you're wading 
> +through the source.
>   

"does its work" will do nicely..

> +
> +File Formats
> +~~~~~~~~~~~~
> +
> +A slice has a basic fixed-size header, followed by a certain number of object 
> +entries.  Commits are sorted in topo-order, and each commit entry is followed 
> +by the objects added in that commit.
>   

Starting from the top or the bottom?  And it might pay to clarify which
objects are included or not; ie objects reachable from "bottom" commits
or not.

> +
> +----
> +         -- +--------------------------------+
> +header      | object number, etc...          |
> +         -- +--------------------------------+
> +commit      | commit info                    |
> +entry       | path data                      |
> +            +--------------------------------+
> +            | tree/blob info                 |
> +            +--------------------------------+
> +            | tree/blob info                 |
> +            +--------------------------------+
> +            | ...                            |
> +         -- +--------------------------------+
> +commit      | commit info                    |
> +entry       | path data                      |
> +            +--------------------------------+
> +            | tree/blob info                 |
> +            +--------------------------------+
> +            | ...                            |
> +         -- +--------------------------------+
> +...         ...                               
> +            +--------------------------------+
> +----
> +
> +The index is somewhat similar to pack-file indexes, containing a fanout table 
> +and a list of index entries sorted by hash.
> +
> +----
> +         -- +--------------------------------+
> +header      | object #, cache #, etc.        |
> +         -- +--------------------------------+
> +cache       | SHA-1                          |
> +sha1s       | ...                            |
> +         -- +--------------------------------+
> +fanout      | fanout[0x00]                   |
> +table       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +            | fanout[0xff]                   |
> +         -- +--------------------------------+
> +index       | entry SHA-1                    |
> +entries     | cache sha1 index               |
> +            +--------------------------------+
> +            |                                |
> +            ...                               
> +            +--------------------------------+
> +----
> +
> +All the relavant structures are readily accessible in `rev-cache.c`
> +
>   

Ok right so which is the on-disk container format?  The index or the
slice?  I'm a little confused..

> +Mechanics
> +~~~~~~~~~
> +
> +The most important part of rev-cache is its method of encoding topological 
> +relations.  To ensure fluid traversal and reconstruction,

Is it still fluid after coagulation?  ;-)

>  commits are related 
> +through high-level "streams"/"channels" rather than individual 
> +interconnections.  Intuitively, rev-cache stores history the way gitk shows it: 
> +commits strung up on lines, which interconnect at merges and branches.
> +
> +Each commit is associated to a given channel/path via a 'path id', and 
> +variable-length fields govern which paths (if any) are closed or opened at that 
> +object.  This means that topo-data can be preserved in only a few bytes extra 
> +per object entry.  Other information stored per entry is the sha-1 hash, type, 
> +date, size, and status in cache slice.  Here is format of an object entry, both 
> +on-disk and in-memory:
> +
> +----
> +struct object_entry {
> +        unsigned type : 3;
> +        unsigned is_end : 1;
> +        unsigned is_start : 1;
> +        unsigned uninteresting : 1;
> +        unsigned include : 1;
> +        unsigned flags : 1;
> +        unsigned char sha1[20];
> +        
> +        unsigned merge_nr : 6;
> +        unsigned split_nr : 7;
> +        unsigned size_size : 3;
> +        
> +	uint32_t date;
> +	uint16_t path;
> +        
> +        /* merge paths */
> +        /* split paths */
> +        /* size */
> +};
> +----
> +
> +An explanation of each field:
> +
> +`type`:: Object type
> +`is_end`:: The commit has some parents outside the cache slice (all if slice 
> +has legs)
> +`is_start`:: The commit has no children in cache slice
> +`uninteresting`:: Run-time flag, used in traversal
> +`include`:: Run-time flag, used in traversal (initialization)
> +`flags`:: Currently unused, extra bit
> +`sha`:: Object SHA-1 hash
> +
> +`merge_nr`:: The number of paths the current channel diverges into; the current 
> +path ends upon any merge.
> +`split_nr`:: The number of paths this commit ends; used on both merging and 
> +branching.
> +`size_size`:: Number of bytes the object size takes up.
> +
> +merge paths:: The path IDs (16-bit) that are to be created.
> +split paths:: The path IDs (16-bit) that are to be ended.
> +size:: The size split into the minimum number of bytes.
> +
> +Each path ID refers to an index in a 'path array', which stores the current 
> +status (eg. active, interestingness) of each channel.
> +
> +Due to topo-relations and boundary tracking, all of a commit's parents must be 
> +encountered before the path is reallocated.  This is achieved by using a 
> +counter system per merge: starting at the parent number, the counter is 
> +decremented as each parent is encountered (dictated by 'split paths'); at 0 the 
> +path is cleared.
> +
> +Boundary tracking is necessary because non-commits are stored relative to the 
> +commit in which they were introduced.  If a series of commits is not included 
> +in the output, the last interesting commit must be parsed manually to ensure 
> +all objects are accounted for.
> +
> +To prevent list-objects from recursing into trees that we've already taken care 
> +of, the flag `FACE_VALUE` is introduced.  An object with this flag is not 
> +explored (= "taken at face value"), significantly reducing I/O and processing 
> +time.
> +
> +(NSE)
>   

tl;dr but that's ok it's magic shizz.

Anyway looking nice ... see what I can say about the next patches.

sam

^ 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