Git development
 help / color / mirror / Atom feed
* [JGIT PATCH v2] Rename RevTag.getName() to RevTag.getTagName()
From: Shawn O. Pearce @ 2009-08-12 20:51 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <200908122237.37148.robin.rosenberg.lists@dewire.com>

The method getName() conflicts semantically with the method name()
we have inherited from our base class, ObjectId.  It is a rather
unfortunate turn of events that for performance reasons we wind up
subclassing what should be a property of this class, but since we
do that we need to pay attention to the methods declared on our
base class.

We want to use getName() to be a mirror of name() on AnyObjectId,
as it has a more JavaBeans style feel to the accessing of that
particular value.  So, rename getTagName() so it doesn't wind up
conflicting with the SHA-1 hex formatted string.

Noticed-by: Alex Blewitt <alex.blewitt@gmail.com>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
 > You forgot the rename of the method here, and all uses of it.

 Quite right.  #@*!! Eclipse.  I thought I refactored that method,
 but I guess it didn't actually do the work, and I failed to read
 the diff closely enough to notice.  *sigh* That's what I get for
 trying to quickly bang out a "simple" change.

 .../org/spearce/jgit/revwalk/RevTagParseTest.java  |    8 ++++----
 .../src/org/spearce/jgit/revwalk/RevTag.java       |   10 +++++-----
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevTagParseTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevTagParseTest.java
index 66bc901..9f91154 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevTagParseTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/revwalk/RevTagParseTest.java
@@ -75,7 +75,7 @@ private void testOneType(final int typeCode) throws Exception {
 
 		c = new RevTag(id("9473095c4cb2f12aefe1db8a355fe3fafba42f67"));
 		assertNull(c.getObject());
-		assertNull(c.getName());
+		assertNull(c.getTagName());
 
 		c.parseCanonical(rw, b.toString().getBytes("UTF-8"));
 		assertNotNull(c.getObject());
@@ -117,15 +117,15 @@ public void testParseAllFields() throws Exception {
 
 		c = new RevTag(id("9473095c4cb2f12aefe1db8a355fe3fafba42f67"));
 		assertNull(c.getObject());
-		assertNull(c.getName());
+		assertNull(c.getTagName());
 
 		c.parseCanonical(rw, body.toString().getBytes("UTF-8"));
 		assertNotNull(c.getObject());
 		assertEquals(treeId, c.getObject().getId());
 		assertSame(rw.lookupTree(treeId), c.getObject());
 
-		assertNotNull(c.getName());
-		assertEquals(name, c.getName());
+		assertNotNull(c.getTagName());
+		assertEquals(name, c.getTagName());
 		assertEquals("", c.getFullMessage());
 
 		final PersonIdent cTagger = c.getTaggerIdent();
diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java
index 2fab266..204e9b1 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java
@@ -56,7 +56,7 @@
 
 	private byte[] buffer;
 
-	private String name;
+	private String tagName;
 
 	/**
 	 * Create a new tag reference.
@@ -96,7 +96,7 @@ void parseCanonical(final RevWalk walk, final byte[] rawTag)
 
 		int p = pos.value += 4; // "tag "
 		final int nameEnd = RawParseUtils.nextLF(rawTag, p) - 1;
-		name = RawParseUtils.decode(Constants.CHARSET, rawTag, p, nameEnd);
+		tagName = RawParseUtils.decode(Constants.CHARSET, rawTag, p, nameEnd);
 
 		if (walk.isRetainBody())
 			buffer = rawTag;
@@ -186,7 +186,7 @@ public final String getShortMessage() {
 	 * @return parsed tag.
 	 */
 	public Tag asTag(final RevWalk walk) {
-		return new Tag(walk.db, this, name, buffer);
+		return new Tag(walk.db, this, tagName, buffer);
 	}
 
 	/**
@@ -203,8 +203,8 @@ public final RevObject getObject() {
 	 * 
 	 * @return name of the tag, according to the tag header.
 	 */
-	public final String getName() {
-		return name;
+	public final String getTagName() {
+		return tagName;
 	}
 
 	final void disposeBody() {
-- 
1.6.4.225.gb589e

^ permalink raw reply related

* Re: [PATCH] Re: [TRIVIAL] Documentation: merge: one <remote> is required
From: Paul Bolle @ 2009-08-12 20:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nicolas Sebrecht, git
In-Reply-To: <7vhbwcydtn.fsf@alter.siamese.dyndns.org>

On Wed, 2009-08-12 at 13:31 -0700, Junio C Hamano wrote:
> Paul Bolle <pebolle@tiscali.nl> writes:
> I do not have any objection to make sure that we uniformly use ellipses
> for one-or-more (and enclose them in [] if we want zero-or-more).  Are
> these two that your patch touched the only ones that need fixing?

I guess not (these two were the ones that annoyed me enough to write
patches when I read them in order to learn more about the commands they
described).

What is the best way to fix small issues like this: one set of patches
that tries to fix as much as possible in one go or a stream of little
patches whenever similar issues are found in one of the documents?

^ permalink raw reply

* Re: git-gui translation pushed
From: Jimmy Angelakos @ 2009-08-12 20:29 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20090810154429.GU1033@spearce.org>

On Mon, 2009-08-10 at 08:44 -0700, Shawn O. Pearce wrote:
> Jimmy Angelakos <vyruss@hellug.gr> wrote:
> > I have pushed a Greek translation for git-gui on repo.or.cz's
> > git-gui-i18n (mob branch).
> 
> Thanks, I have cherry-picked this through to master.  The charset
> was set wrong in the files, it was "CHARSET", which isn't a valid
> charset.  I modified to UTF-8 before applying.  I suspect something
> is up with your PO editor.
> 

Thanks for mentioning that, I'll check it out. It's either something
wrong with Lokalize or my mistake.

Regards
Jimmy

^ permalink raw reply

* Re: [JGIT PATCH] Rename RevTag.getName() to RevTag.getTagName()
From: Robin Rosenberg @ 2009-08-12 20:37 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <1250105956-17795-1-git-send-email-spearce@spearce.org>

onsdag 12 augusti 2009 21:39:16 skrev "Shawn O. Pearce" <spearce@spearce.org>:
> The method getName() conflicts semantically with the method name()
> we have inherited from our base class, ObjectId.  It is a rather
> unfortunate turn of events that for performance reasons we wind up
> subclassing what should be a property of this class, but since we
> do that we need to pay attention to the methods declared on our
> base class.
> 
> We want to use getName() to be a mirror of name() on AnyObjectId,
> as it has a more JavaBeans style feel to the accessing of that
> particular value.  So, rename getTagName() so it doesn't wind up
> conflicting with the SHA-1 hex formatted string.

> @@ -204,7 +204,7 @@ public final RevObject getObject() {
>  	 * @return name of the tag, according to the tag header.
>  	 */
>  	public final String getName() {
> -		return name;
> +		return tagName;
>  	}
>  
>  	final void disposeBody() {

You forgot the rename of the method here, and all uses of it.

-- robin

^ permalink raw reply

* Re: git index: how does it work?
From: Junio C Hamano @ 2009-08-12 20:31 UTC (permalink / raw)
  To: Shaun Cutts; +Cc: git
In-Reply-To: <436D5ED1-2F0E-4227-AC4A-3A5FD16B2DCF@cuttshome.net>

Shaun Cutts <shaun@cuttshome.net> writes:

> Are renames being tracked by the index, and is there a more basic
> interface than "status" to query about them?

No.  Index, nor git in general, never records renames.  git records
contents, not content changes.  The index records a state, so does the
tree object pointed at by the HEAD commit.

When you ask for "status", git will notice that you have lost a file, and
added a new one, between these two states, by comparing them.  The
contents of these lost files and added files are then compared, and ones
with similar contents are paired up.

That way, you do not have to use "git mv A B" to "rename" A to B.  You can
just as well "mv A B; git rm A; git add B", and get the same outcome,
exactly because git does not record renames.

Instead, we track them by deducing that you renamed from the result.

The tree-vs-index comparison "git status" does to figure all this out is
"git diff-index -M --cached HEAD".

As it should be obvious from the above description,

	git diff-index -M --cached HEAD -- A

is *NOT* the way for you to ask about "possible renames of A".  You need
to run the diff for the whole tree without path limitation so that you can
pair deletions and creations up in order to deduce renames.

^ permalink raw reply

* Re: [PATCH] Re: [TRIVIAL] Documentation: merge: one <remote> is required
From: Junio C Hamano @ 2009-08-12 20:31 UTC (permalink / raw)
  To: Paul Bolle; +Cc: Nicolas Sebrecht, git
In-Reply-To: <1250074578.7545.2.camel@localhost.localdomain>

Paul Bolle <pebolle@tiscali.nl> writes:

> Confusingly, as far as I can see, the manpages of the following commands
> seem to use the "one or more" meaning:
>     git merge-base
> ...
> ("git mv" uses both meanings in its synopsis. The two "git tag"
> invocations seem to do nothing with zero arguments and do not return an
> error.)
>
> If the above commands really use the "one or more" meaning, that would
> mean both versions are used in the documentation. I'd say it would be
> better to stick to one meaning throughout the manpages.

You are absolutely right.  We would want consistency.

I do not have any objection to make sure that we uniformly use ellipses
for one-or-more (and enclose them in [] if we want zero-or-more).  Are
these two that your patch touched the only ones that need fixing?

^ permalink raw reply

* Re: [PATCH 5/8] Add a config option for remotes to specify a foreign vcs
From: Junio C Hamano @ 2009-08-12 20:30 UTC (permalink / raw)
  To: Jakub Narebski
  Cc: Jeff King, Johannes Schindelin, Bert Wesarg, Daniel Barkalow, git,
	Brian Gernhardt
In-Reply-To: <m363ctpedr.fsf@localhost.localdomain>

Jakub Narebski <jnareb@gmail.com> writes:

> Jeff King <peff@peff.net> writes:
>
>>   1. Is there some other syntax that _doesn't_ have this breakage
>>      but that similarly helps the "vast majority of Git users".
>
> Well, proposed possible syntax was:
> 1. <vcs>:<repository location>
> ...
> 2. <vcs>::<repository location>
> ...
> 3. <vcs>+<repository location>
>
>    e.g.
>
>      svn+http://svn.example.com/project
>
>    but
>
>      http+svn://svn.example.com/project
>      svn+path/to/repo

I do not think these are valid examples to demonstrate that 3 is bad.

We do not have (and we will not create) "http+svn://" native transport, so
the former can only mean "Feed 'svn://svn.example.com/project' to the vcs
helper whose name is 'http'".  Similarly I do not see any way to read the
latter other than "Feed 'path/to/repo' to 'svn' vcs helper".

We do have a pair of synonyms "git+ssh://foo" and "ssh+git://bar" that
could make the use of '+' ambiguous.  They could be feeding 'ssh://foo'
and 'git://bar' to 'git' and 'ssh' vcs helpers respectively, but

 (1) they are not even advertised in Documentation/ anywhere as far as I
     can see; and

 (2) these are the only two existing ones that are misdesigned, and we can
     easily special case them to keep backward compatibility.

Double-colon (your 2) is also workable.  It probably is slightly better
than plus because it does not have to grandfather "git+ssh" and "ssh+git"
and that would be beneficial for requiring less complexity in both code
(i.e. special case logic) and more importantly in mental burden to the end
users (i.e. '::' would stand out more than '+' and clearly different from
traditional git URLs in all cases).

As Jeff said (your 1.), a single colon ':' has a rather bad ambiguity
between <vcs> and hostname part in the existing scp-style repository
naming.

^ permalink raw reply

* Re: Tie a CVS-aware app into GIT?
From: Robin Rosenberg @ 2009-08-12 20:30 UTC (permalink / raw)
  To: david.hagood; +Cc: git
In-Reply-To: <b988339dcd1bf764f0da46db763552d8.squirrel@localhost>

onsdag 12 augusti 2009 20:01:45 skrev david.hagood@gmail.com:
> I have an application (closed source, unfortunately) that can use CVS to
> manage its files (specifically, Enterprise Architect by Sparx).
> 
> I'd rather use GIT to manage the files, but EA doesn't "speak" git - just
> CVS and Subversion (and Microsoft's SCC protocol, but...)
> 
> Are there any programs which
> 1) accept the same command line parameters as CVS or Subversion
> and
> 2) access a GIT repository.
> 
> git-svn is almost exactly NOT what I need, as it accepts "git" type
> command line parameters and access a SVN repo, rather than accepting SVN
> command line parameters and accessing a GIT repo.
> 
> Now, obviously, such a tool wouldn't have access to all the things that
> GIT can do, and that is NOT what I am expecting - what I want is just to
> enable EA to do the things it needs to do, namely adding/removing/moving
> files from a repo. Branching, commits, and so on can be done manually.
> 
> I've thought about git-cvsserver as a solution, but I don't know if it can
> be run on a local machine to access a local repository.

What makes you there would be an issue here?

-- robin

^ permalink raw reply

* Re: [ANNOUNCE] Scumd
From: Jakub Narebski @ 2009-08-12 20:26 UTC (permalink / raw)
  To: Michael Gaffney; +Cc: git
In-Reply-To: <4A8309D9.8070008@gmail.com>

Michael Gaffney <mr.gaffo@gmail.com> writes:

>     This is an initial announcement of SCuMD, a pure java git sshd
> daemon. The source is at git://github.com/gaffo/scumd. The impetus
> behind SCuMD is to provide a highly configurable git daemon which can
> authenticate and authorize off of flat files, databases, LDAP, web
> services, or any other resource you can think of. SCuMD's other goal
> is to remove the need to serve off of normal sshd which some find to
> be a security risk on the open Internet. Currently SCuMD supports LDAP
> as the authentication module but coding other modules is quite simple.
>
>     I would welcome any feedback including a better name. SCuMD stands
> for SCM Daemon.

Could you add information about this tool to Git Wiki:
http://git.or.cz/gitwiki/InterfacesFrontendsAndTools
perhaps below entry for gitosis?

TIA
-- 
Jakub Narebski

Git User's Survey 2009
http://tinyurl.com/GitSurvey2009

^ permalink raw reply

* [PATCH 3/3] block-sha1: support for architectures with memory alignment restrictions
From: Nicolas Pitre @ 2009-08-12 19:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, git


This is needed on architectures with poor or non-existent unaligned memory
support and/or no fast byte swap instruction (such as ARM) by using byte
accesses to memory and shifting the result together.

This also makes the code portable, therefore the byte access methods are
the defaults.  Any architecture that properly supports unaligned word
accesses in hardware simply has to enable the alternative methods.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---
 block-sha1/sha1.c |   32 ++++++++++++++++++++++++++++++--
 1 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/block-sha1/sha1.c b/block-sha1/sha1.c
index 67c9bd0..d3121f7 100644
--- a/block-sha1/sha1.c
+++ b/block-sha1/sha1.c
@@ -60,6 +60,34 @@
   #define setW(x, val) (W(x) = (val))
 #endif
 
+/*
+ * Performance might be improved if the CPU architecture is OK with
+ * unaligned 32-bit loads and a fast ntohl() is available.
+ * Otherwise fall back to byte loads and shifts which is portable,
+ * and is faster on architectures with memory alignment issues.
+ */
+
+#if defined(__i386__) || defined(__x86_64__)
+
+#define get_be32(p)	ntohl(*(unsigned int *)(p))
+#define put_be32(p, v)	do { *(unsigned int *)(p) = htonl(v); } while (0)
+
+#else
+
+#define get_be32(p)	( \
+	(*((unsigned char *)(p) + 0) << 24) | \
+	(*((unsigned char *)(p) + 1) << 16) | \
+	(*((unsigned char *)(p) + 2) <<  8) | \
+	(*((unsigned char *)(p) + 3) <<  0) )
+#define put_be32(p, v)	do { \
+	unsigned int __v = (v); \
+	*((unsigned char *)(p) + 0) = __v >> 24; \
+	*((unsigned char *)(p) + 1) = __v >> 16; \
+	*((unsigned char *)(p) + 2) = __v >>  8; \
+	*((unsigned char *)(p) + 3) = __v >>  0; } while (0)
+
+#endif
+
 /* This "rolls" over the 512-bit array */
 #define W(x) (array[(x)&15])
 
@@ -67,7 +95,7 @@
  * Where do we get the source from? The first 16 iterations get it from
  * the input data, the next mix it from the 512-bit array.
  */
-#define SHA_SRC(t) htonl(data[t])
+#define SHA_SRC(t) get_be32(data + t)
 #define SHA_MIX(t) SHA_ROL(W(t+13) ^ W(t+8) ^ W(t+2) ^ W(t), 1)
 
 #define SHA_ROUND(t, input, fn, constant, A, B, C, D, E) do { \
@@ -245,5 +273,5 @@ void blk_SHA1_Final(unsigned char hashout[20], blk_SHA_CTX *ctx)
 
 	/* Output hash */
 	for (i = 0; i < 5; i++)
-		((unsigned int *)hashout)[i] = htonl(ctx->H[i]);
+		put_be32(hashout + i*4, ctx->H[i]);
 }
-- 
1.6.4.189.g282fa

^ permalink raw reply related

* Re: [JGIT PATCH 1/1] Fix for Repository.stripWorkDir when using partial paths
From: Robin Rosenberg @ 2009-08-12 19:47 UTC (permalink / raw)
  To: Adam W. Hawks; +Cc: git
In-Reply-To: <4A821167.6030107@writeme.com>

onsdag 12 augusti 2009 02:48:39 skrev "Adam W. Hawks" <awhawks@writeme.com>:
> 
> From ef993e633cdcb1dddda5e71db1b62306df7ce83f Mon Sep 17 00:00:00 2001
> Date: Tue, 11 Aug 2009 20:02:56 -0400
> 
> When you call stripWorkDir with a relative path
> you can get a string out of bounds error.
> 
> This change fixes that problem by using the absolute paths
> of the file instead of its relative name.
> 
> Signed-off-by: Adam W. Hawks <awhawks@writeme.com>
> ---
>  .../src/org/spearce/jgit/lib/Repository.java       |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
> index 468cf4c..a68817b 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
> @@ -1036,7 +1036,7 @@ public static boolean isValidRefName(final String refName) {
>  	 * @return normalized repository relative path
>  	 */
>  	public static String stripWorkDir(File wd, File f) {
> -		String relName = f.getPath().substring(wd.getPath().length() + 1);
> +		String relName = f.getAbsolutePath().substring(wd.getPath().length() + 1);
>  		relName = relName.replace(File.separatorChar, '/');
>  		return relName;
>  	}

Why not convert both paths? A trickier issue is that getAbsolutePath is very slow when
the path is not absolute. I don't think we will always need to normalize in order to
fix this. A few unit tests to show the cases solved would help.

-- robin

^ permalink raw reply

* [PATCH 2/3] block-sha1: split the different "hacks" to be individually selected
From: Nicolas Pitre @ 2009-08-12 19:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, git


This is to make it easier for them to be selected individually depending
on the architecture instead of the other way around i.e. having each
architecture select a list of hacks up front.  That makes for clearer
documentation as well.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---
 block-sha1/sha1.c |   23 ++++++++++++++++++-----
 1 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/block-sha1/sha1.c b/block-sha1/sha1.c
index c3f1ae5..67c9bd0 100644
--- a/block-sha1/sha1.c
+++ b/block-sha1/sha1.c
@@ -11,10 +11,16 @@
 
 #if defined(__i386__) || defined(__x86_64__)
 
+/*
+ * Force usage of rol or ror by selecting the one with the smaller constant.
+ * It _can_ generate slightly smaller code (a constant of 1 is special), but
+ * perhaps more importantly it's possibly faster on any uarch that does a
+ * rotate with a loop.
+ */
+
 #define SHA_ASM(op, x, n) ({ unsigned int __res; __asm__(op " %1,%0":"=r" (__res):"i" (n), "0" (x)); __res; })
 #define SHA_ROL(x,n)	SHA_ASM("rol", x, n)
 #define SHA_ROR(x,n)	SHA_ASM("ror", x, n)
-#define SMALL_REGISTER_SET
 
 #else
 
@@ -24,9 +30,6 @@
 
 #endif
 
-/* This "rolls" over the 512-bit array */
-#define W(x) (array[(x)&15])
-
 /*
  * If you have 32 registers or more, the compiler can (and should)
  * try to change the array[] accesses into registers. However, on
@@ -43,13 +46,23 @@
  * Ben Herrenschmidt reports that on PPC, the C version comes close
  * to the optimized asm with this (ie on PPC you don't want that
  * 'volatile', since there are lots of registers).
+ *
+ * On ARM we get the best code generation by forcing a full memory barrier
+ * between each SHA_ROUND, otherwise gcc happily get wild with spilling and
+ * the stack frame size simply explode and performance goes down the drain.
  */
-#ifdef SMALL_REGISTER_SET
+
+#if defined(__i386__) || defined(__x86_64__)
   #define setW(x, val) (*(volatile unsigned int *)&W(x) = (val))
+#elif defined(__arm__)
+  #define setW(x, val) do { W(x) = (val); __asm__("":::"memory"); } while (0)
 #else
   #define setW(x, val) (W(x) = (val))
 #endif
 
+/* This "rolls" over the 512-bit array */
+#define W(x) (array[(x)&15])
+
 /*
  * Where do we get the source from? The first 16 iterations get it from
  * the input data, the next mix it from the 512-bit array.
-- 
1.6.4.189.g282fa

^ permalink raw reply related

* [PATCH 1/3] block-sha1: move code around
From: Nicolas Pitre @ 2009-08-12 19:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, git


Move the code around so specific architecture hacks are defined first.
Also make one line comments actually one line.  No code change.

Signed-off-by: Nicolas Pitre <nico@cam.org>
---
 block-sha1/sha1.c |  129 ++++++++++++++++++++++++----------------------------
 1 files changed, 60 insertions(+), 69 deletions(-)

diff --git a/block-sha1/sha1.c b/block-sha1/sha1.c
index 304cd04..c3f1ae5 100644
--- a/block-sha1/sha1.c
+++ b/block-sha1/sha1.c
@@ -9,74 +9,6 @@
 
 #include "sha1.h"
 
-/* Hash one 64-byte block of data */
-static void blk_SHA1Block(blk_SHA_CTX *ctx, const unsigned int *data);
-
-void blk_SHA1_Init(blk_SHA_CTX *ctx)
-{
-	ctx->size = 0;
-
-	/* Initialize H with the magic constants (see FIPS180 for constants)
-	 */
-	ctx->H[0] = 0x67452301;
-	ctx->H[1] = 0xefcdab89;
-	ctx->H[2] = 0x98badcfe;
-	ctx->H[3] = 0x10325476;
-	ctx->H[4] = 0xc3d2e1f0;
-}
-
-
-void blk_SHA1_Update(blk_SHA_CTX *ctx, const void *data, unsigned long len)
-{
-	int lenW = ctx->size & 63;
-
-	ctx->size += len;
-
-	/* Read the data into W and process blocks as they get full
-	 */
-	if (lenW) {
-		int left = 64 - lenW;
-		if (len < left)
-			left = len;
-		memcpy(lenW + (char *)ctx->W, data, left);
-		lenW = (lenW + left) & 63;
-		len -= left;
-		data += left;
-		if (lenW)
-			return;
-		blk_SHA1Block(ctx, ctx->W);
-	}
-	while (len >= 64) {
-		blk_SHA1Block(ctx, data);
-		data += 64;
-		len -= 64;
-	}
-	if (len)
-		memcpy(ctx->W, data, len);
-}
-
-
-void blk_SHA1_Final(unsigned char hashout[20], blk_SHA_CTX *ctx)
-{
-	static const unsigned char pad[64] = { 0x80 };
-	unsigned int padlen[2];
-	int i;
-
-	/* Pad with a binary 1 (ie 0x80), then zeroes, then length
-	 */
-	padlen[0] = htonl(ctx->size >> 29);
-	padlen[1] = htonl(ctx->size << 3);
-
-	i = ctx->size & 63;
-	blk_SHA1_Update(ctx, pad, 1+ (63 & (55 - i)));
-	blk_SHA1_Update(ctx, padlen, 8);
-
-	/* Output hash
-	 */
-	for (i = 0; i < 5; i++)
-		((unsigned int *)hashout)[i] = htonl(ctx->H[i]);
-}
-
 #if defined(__i386__) || defined(__x86_64__)
 
 #define SHA_ASM(op, x, n) ({ unsigned int __res; __asm__(op " %1,%0":"=r" (__res):"i" (n), "0" (x)); __res; })
@@ -136,7 +68,7 @@ void blk_SHA1_Final(unsigned char hashout[20], blk_SHA_CTX *ctx)
 #define T_40_59(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, ((B&C)+(D&(B^C))) , 0x8f1bbcdc, A, B, C, D, E )
 #define T_60_79(t, A, B, C, D, E) SHA_ROUND(t, SHA_MIX, (B^C^D) ,  0xca62c1d6, A, B, C, D, E )
 
-static void blk_SHA1Block(blk_SHA_CTX *ctx, const unsigned int *data)
+static void blk_SHA1_Block(blk_SHA_CTX *ctx, const unsigned int *data)
 {
 	unsigned int A,B,C,D,E;
 	unsigned int array[16];
@@ -243,3 +175,62 @@ static void blk_SHA1Block(blk_SHA_CTX *ctx, const unsigned int *data)
 	ctx->H[3] += D;
 	ctx->H[4] += E;
 }
+
+void blk_SHA1_Init(blk_SHA_CTX *ctx)
+{
+	ctx->size = 0;
+
+	/* Initialize H with the magic constants (see FIPS180 for constants) */
+	ctx->H[0] = 0x67452301;
+	ctx->H[1] = 0xefcdab89;
+	ctx->H[2] = 0x98badcfe;
+	ctx->H[3] = 0x10325476;
+	ctx->H[4] = 0xc3d2e1f0;
+}
+
+void blk_SHA1_Update(blk_SHA_CTX *ctx, const void *data, unsigned long len)
+{
+	int lenW = ctx->size & 63;
+
+	ctx->size += len;
+
+	/* Read the data into W and process blocks as they get full */
+	if (lenW) {
+		int left = 64 - lenW;
+		if (len < left)
+			left = len;
+		memcpy(lenW + (char *)ctx->W, data, left);
+		lenW = (lenW + left) & 63;
+		len -= left;
+		data += left;
+		if (lenW)
+			return;
+		blk_SHA1_Block(ctx, ctx->W);
+	}
+	while (len >= 64) {
+		blk_SHA1_Block(ctx, data);
+		data += 64;
+		len -= 64;
+	}
+	if (len)
+		memcpy(ctx->W, data, len);
+}
+
+void blk_SHA1_Final(unsigned char hashout[20], blk_SHA_CTX *ctx)
+{
+	static const unsigned char pad[64] = { 0x80 };
+	unsigned int padlen[2];
+	int i;
+
+	/* Pad with a binary 1 (ie 0x80), then zeroes, then length */
+	padlen[0] = htonl(ctx->size >> 29);
+	padlen[1] = htonl(ctx->size << 3);
+
+	i = ctx->size & 63;
+	blk_SHA1_Update(ctx, pad, 1+ (63 & (55 - i)));
+	blk_SHA1_Update(ctx, padlen, 8);
+
+	/* Output hash */
+	for (i = 0; i < 5; i++)
+		((unsigned int *)hashout)[i] = htonl(ctx->H[i]);
+}
-- 
1.6.4.189.g282fa

^ permalink raw reply related

* [JGIT PATCH (RESEND) 1/4] Avoid unnecessary stat when scanning packs in the objects directory
From: Shawn O. Pearce @ 2009-08-12 19:45 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1250106323-19408-1-git-send-email-spearce@spearce.org>

We only care that the pack name exists in the directory at this stage
of processing.  Performing a isFile() test against the pack file name
is likely to be slower than checking an in-memory HashSet, since the
OS call has to actually check the inode to determine whether or not
the name is a file, or some other node type.  Since we already have
forced the OS to give us a complete listing of the paths, its just
faster to consult that existing list.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/lib/ObjectDirectory.java  |   49 +++++++++++--------
 1 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
index 7cc459c..4419f9c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
@@ -41,14 +41,16 @@
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.FileReader;
-import java.io.FilenameFilter;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.atomic.AtomicReference;
 
 import org.spearce.jgit.errors.PackMismatchException;
@@ -339,11 +341,23 @@ private static boolean inList(final PackFile[] list, final PackFile pack) {
 
 	private PackFile[] scanPacksImpl(final PackFile[] old) {
 		final Map<String, PackFile> forReuse = reuseMap(old);
-		final String[] idxList = listPackIdx();
-		final List<PackFile> list = new ArrayList<PackFile>(idxList.length);
-		for (final String indexName : idxList) {
+		final Set<String> names = listPackDirectory();
+		final List<PackFile> list = new ArrayList<PackFile>(names.size() >> 2);
+		for (final String indexName : names) {
+			// Must match "pack-[0-9a-f]{40}.idx" to be an index.
+			//
+			if (indexName.length() != 49 || !indexName.endsWith(".idx"))
+				continue;
+
 			final String base = indexName.substring(0, indexName.length() - 4);
 			final String packName = base + ".pack";
+			if (!names.contains(packName)) {
+				// Sometimes C Git's HTTP fetch transport leaves a
+				// .idx file behind and does not download the .pack.
+				// We have to skip over such useless indexes.
+				//
+				continue;
+			}
 
 			final PackFile oldPack = forReuse.remove(packName);
 			if (oldPack != null) {
@@ -352,14 +366,6 @@ private static boolean inList(final PackFile[] list, final PackFile pack) {
 			}
 
 			final File packFile = new File(packDirectory, packName);
-			if (!packFile.isFile()) {
-				// Sometimes C Git's HTTP fetch transport leaves a
-				// .idx file behind and does not download the .pack.
-				// We have to skip over such useless indexes.
-				//
-				continue;
-			}
-
 			final File idxFile = new File(packDirectory, indexName);
 			list.add(new PackFile(idxFile, packFile));
 		}
@@ -401,16 +407,17 @@ private static boolean inList(final PackFile[] list, final PackFile pack) {
 		return forReuse;
 	}
 
-	private String[] listPackIdx() {
+	private Set<String> listPackDirectory() {
 		packDirectoryLastModified = packDirectory.lastModified();
-		final String[] idxList = packDirectory.list(new FilenameFilter() {
-			public boolean accept(final File baseDir, final String n) {
-				// Must match "pack-[0-9a-f]{40}.idx" to be an index.
-				return n.length() == 49 && n.endsWith(".idx")
-						&& n.startsWith("pack-");
-			}
-		});
-		return idxList != null ? idxList : new String[0];
+		final String[] nameList = packDirectory.list();
+		if (nameList == null)
+			return Collections.emptySet();
+		final Set<String> nameSet = new HashSet<String>(nameList.length << 1);
+		for (final String name : nameList) {
+			if (name.startsWith("pack-"))
+				nameSet.add(name);
+		}
+		return nameSet;
 	}
 
 	@Override
-- 
1.6.4.225.gb589e

^ permalink raw reply related

* [JGIT PATCH (RESEND) 4/4] Fix racy condition when a repository is repacked
From: Shawn O. Pearce @ 2009-08-12 19:45 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1250106323-19408-4-git-send-email-spearce@spearce.org>

If the filesystem clock granularity is sufficiently large enough it
is possible for a repacking program such as `git repack` to change
the same directory more than once within the same modification time.

If JGit were to scan the directory between changes in the same
clock step it will never see the later edits, because the directory
modification time has not changed.

Instead we now keep track of the last time we read the directory.
If an object cannot be found on disk and the pack directory's last
modified time is less than 2 minutes since the last time we read
the directory's contents, we scan it again looking for changes.

Worst case scenario, JGit will list the pack directory once for
each requested missing object, until the directory has aged at
least 2 minutes.  Most repositories modify this directory only a
few times a week, so this is not an undue burden on the host.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/lib/ObjectDirectory.java  |   60 +++++++++++++++++---
 1 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
index 0bb3c01..859824d 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
@@ -66,7 +66,7 @@
  * {@link PackFile}s.
  */
 public class ObjectDirectory extends ObjectDatabase {
-	private static final PackList NO_PACKS = new PackList(-1, new PackFile[0]);
+	private static final PackList NO_PACKS = new PackList(-1, -1, new PackFile[0]);
 
 	private final File objects;
 
@@ -273,7 +273,7 @@ private void insertPack(final PackFile pf) {
 			final PackFile[] newList = new PackFile[1 + oldList.length];
 			newList[0] = pf;
 			System.arraycopy(oldList, 0, newList, 1, oldList.length);
-			n = new PackList(o.lastModified, newList);
+			n = new PackList(o.lastRead, o.lastModified, newList);
 		} while (!packList.compareAndSet(o, n));
 	}
 
@@ -290,7 +290,7 @@ private void removePack(final PackFile deadPack) {
 			final PackFile[] newList = new PackFile[oldList.length - 1];
 			System.arraycopy(oldList, 0, newList, 0, j);
 			System.arraycopy(oldList, j + 1, newList, j, newList.length - j);
-			n = new PackList(o.lastModified, newList);
+			n = new PackList(o.lastRead, o.lastModified, newList);
 		} while (!packList.compareAndSet(o, n));
 		deadPack.close();
 	}
@@ -324,6 +324,7 @@ private PackList scanPacks(final PackList original) {
 
 	private PackList scanPacksImpl(final PackList old) {
 		final Map<String, PackFile> forReuse = reuseMap(old);
+		final long lastRead = System.currentTimeMillis();
 		final long lastModified = packDirectory.lastModified();
 		final Set<String> names = listPackDirectory();
 		final List<PackFile> list = new ArrayList<PackFile>(names.size() >> 2);
@@ -362,18 +363,18 @@ private PackList scanPacksImpl(final PackList old) {
 		// return the same collection.
 		//
 		if (!foundNew && lastModified == old.lastModified && forReuse.isEmpty())
-			return old;
+			return old.updateLastRead(lastRead);
 
 		for (final PackFile p : forReuse.values()) {
 			p.close();
 		}
 
 		if (list.isEmpty())
-			return new PackList(lastModified, NO_PACKS.packs);
+			return new PackList(lastRead, lastModified, NO_PACKS.packs);
 
 		final PackFile[] r = list.toArray(new PackFile[list.size()]);
 		Arrays.sort(r, PackFile.SORT);
-		return new PackList(lastModified, r);
+		return new PackList(lastRead, lastModified, r);
 	}
 
 	private static Map<String, PackFile> reuseMap(final PackList old) {
@@ -449,19 +450,62 @@ private ObjectDatabase openAlternate(final String location)
 	}
 
 	private static final class PackList {
+		/** Last wall-clock time the directory was read. */
+		volatile long lastRead;
+
 		/** Last modification time of {@link ObjectDirectory#packDirectory}. */
 		final long lastModified;
 
 		/** All known packs, sorted by {@link PackFile#SORT}. */
 		final PackFile[] packs;
 
-		PackList(final long lastModified, final PackFile[] packs) {
+		private boolean cannotBeRacilyClean;
+
+		PackList(final long lastRead, final long lastModified,
+				final PackFile[] packs) {
+			this.lastRead = lastRead;
 			this.lastModified = lastModified;
 			this.packs = packs;
+			this.cannotBeRacilyClean = notRacyClean(lastRead);
+		}
+
+		private boolean notRacyClean(final long read) {
+			return read - lastModified > 2 * 60 * 1000L;
+		}
+
+		PackList updateLastRead(final long now) {
+			if (notRacyClean(now))
+				cannotBeRacilyClean = true;
+			lastRead = now;
+			return this;
 		}
 
 		boolean tryAgain(final long currLastModified) {
-			return lastModified < currLastModified;
+			// Any difference indicates the directory was modified.
+			//
+			if (lastModified != currLastModified)
+				return true;
+
+			// We have already determined the last read was far enough
+			// after the last modification that any new modifications
+			// are certain to change the last modified time.
+			//
+			if (cannotBeRacilyClean)
+				return false;
+
+			if (notRacyClean(lastRead)) {
+				// Our last read should have marked cannotBeRacilyClean,
+				// but this thread may not have seen the change. The read
+				// of the volatile field lastRead should have fixed that.
+				//
+				return false;
+			}
+
+			// We last read this directory too close to its last observed
+			// modification time. We may have missed a modification. Scan
+			// the directory again, to ensure we still see the same state.
+			//
+			return true;
 		}
 	}
 }
-- 
1.6.4.225.gb589e

^ permalink raw reply related

* [JGIT PATCH (RESEND) 3/4] Don't create new pack lists if the directory hasn't changed
From: Shawn O. Pearce @ 2009-08-12 19:45 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1250106323-19408-3-git-send-email-spearce@spearce.org>

If we scan the directory and it hasn't changed since the last time we
scanned it, there is no reason to build a new PackList and update the
volatile reference.  This just generates unnecessary garbage and may
make it more difficult to detect an unmodified directory.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/lib/ObjectDirectory.java  |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
index 5b28207..0bb3c01 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
@@ -260,10 +260,8 @@ protected ObjectLoader openObject2(final WindowCursor curs,
 	@Override
 	protected boolean tryAgain1() {
 		final PackList old = packList.get();
-		if (old.tryAgain(packDirectory.lastModified())) {
-			scanPacks(old);
-			return true;
-		}
+		if (old.tryAgain(packDirectory.lastModified()))
+			return old != scanPacks(old);
 		return false;
 	}
 
@@ -317,6 +315,8 @@ private PackList scanPacks(final PackList original) {
 					return o;
 				}
 				n = scanPacksImpl(o);
+				if (n == o)
+					return n;
 			} while (!packList.compareAndSet(o, n));
 			return n;
 		}
@@ -327,6 +327,7 @@ private PackList scanPacksImpl(final PackList old) {
 		final long lastModified = packDirectory.lastModified();
 		final Set<String> names = listPackDirectory();
 		final List<PackFile> list = new ArrayList<PackFile>(names.size() >> 2);
+		boolean foundNew = false;
 		for (final String indexName : names) {
 			// Must match "pack-[0-9a-f]{40}.idx" to be an index.
 			//
@@ -352,8 +353,17 @@ private PackList scanPacksImpl(final PackList old) {
 			final File packFile = new File(packDirectory, packName);
 			final File idxFile = new File(packDirectory, indexName);
 			list.add(new PackFile(idxFile, packFile));
+			foundNew = true;
 		}
 
+		// If we did not discover any new files, the modification time was not
+		// changed, and we did not remove any files, then the set of files is
+		// the same as the set we were given. Instead of building a new object
+		// return the same collection.
+		//
+		if (!foundNew && lastModified == old.lastModified && forReuse.isEmpty())
+			return old;
+
 		for (final PackFile p : forReuse.values()) {
 			p.close();
 		}
-- 
1.6.4.225.gb589e

^ permalink raw reply related

* [JGIT PATCH (RESEND) 2/4] Make ObjectDirectory last modified time atomically updated with list
From: Shawn O. Pearce @ 2009-08-12 19:45 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <1250106323-19408-2-git-send-email-spearce@spearce.org>

By moving the last modified time and the PackFile[] into the same
object and using that object as the atomic access unit we ensure
that the two values change atomically for all readers.

This refactoring step prepares the code so we can later fix a race
condition where the objects directory is modified multiple times in
the same filesystem clock window, but a reader scanned the directory
during that same clock window.

A nice cleanup performed here is to eliminate the null pointer value
from the packList reference field.  Instead of getting a null we
get NO_PACKS, which uses an "impossible" last modified time of -1,
forcing any caller to scan the directory the first time a pack
is required.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/lib/ObjectDirectory.java  |  126 ++++++++++----------
 1 files changed, 63 insertions(+), 63 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
index 4419f9c..5b28207 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
@@ -66,7 +66,7 @@
  * {@link PackFile}s.
  */
 public class ObjectDirectory extends ObjectDatabase {
-	private static final PackFile[] NO_PACKS = {};
+	private static final PackList NO_PACKS = new PackList(-1, new PackFile[0]);
 
 	private final File objects;
 
@@ -76,9 +76,7 @@
 
 	private final File alternatesFile;
 
-	private final AtomicReference<PackFile[]> packList;
-
-	private volatile long packDirectoryLastModified;
+	private final AtomicReference<PackList> packList;
 
 	/**
 	 * Initialize a reference to an on-disk object directory.
@@ -91,8 +89,7 @@ public ObjectDirectory(final File dir) {
 		infoDirectory = new File(objects, "info");
 		packDirectory = new File(objects, "pack");
 		alternatesFile = new File(infoDirectory, "alternates");
-
-		packList = new AtomicReference<PackFile[]>();
+		packList = new AtomicReference<PackList>(NO_PACKS);
 	}
 
 	/**
@@ -116,13 +113,10 @@ public void create() throws IOException {
 
 	@Override
 	public void closeSelf() {
-		PackFile[] packs = packList.get();
-		if (packs != null) {
-			packList.set(null);
-			for (final PackFile p : packs) {
-				p.close();
-			}
-		}
+		final PackList packs = packList.get();
+		packList.set(NO_PACKS);
+		for (final PackFile p : packs.packs)
+			p.close();
 	}
 
 	/**
@@ -176,7 +170,7 @@ public String toString() {
 
 	@Override
 	protected boolean hasObject1(final AnyObjectId objectId) {
-		for (final PackFile p : packs()) {
+		for (final PackFile p : packList.get().packs) {
 			try {
 				if (p.hasObject(objectId)) {
 					return true;
@@ -196,9 +190,9 @@ protected boolean hasObject1(final AnyObjectId objectId) {
 	@Override
 	protected ObjectLoader openObject1(final WindowCursor curs,
 			final AnyObjectId objectId) throws IOException {
-		PackFile[] pList = packs();
+		PackList pList = packList.get();
 		SEARCH: for (;;) {
-			for (final PackFile p : pList) {
+			for (final PackFile p : pList.packs) {
 				try {
 					final PackedObjectLoader ldr = p.get(curs, objectId);
 					if (ldr != null) {
@@ -224,9 +218,9 @@ protected ObjectLoader openObject1(final WindowCursor curs,
 	void openObjectInAllPacks1(final Collection<PackedObjectLoader> out,
 			final WindowCursor curs, final AnyObjectId objectId)
 			throws IOException {
-		PackFile[] pList = packs();
+		PackList pList = packList.get();
 		SEARCH: for (;;) {
-			for (final PackFile p : pList) {
+			for (final PackFile p : pList.packs) {
 				try {
 					final PackedObjectLoader ldr = p.get(curs, objectId);
 					if (ldr != null) {
@@ -265,8 +259,8 @@ protected ObjectLoader openObject2(final WindowCursor curs,
 
 	@Override
 	protected boolean tryAgain1() {
-		final PackFile[] old = packList.get();
-		if (packDirectoryLastModified < packDirectory.lastModified()) {
+		final PackList old = packList.get();
+		if (old.tryAgain(packDirectory.lastModified())) {
 			scanPacks(old);
 			return true;
 		}
@@ -274,57 +268,46 @@ protected boolean tryAgain1() {
 	}
 
 	private void insertPack(final PackFile pf) {
-		PackFile[] o, n;
+		PackList o, n;
 		do {
-			o = packs();
-			n = new PackFile[1 + o.length];
-			n[0] = pf;
-			System.arraycopy(o, 0, n, 1, o.length);
+			o = packList.get();
+			final PackFile[] oldList = o.packs;
+			final PackFile[] newList = new PackFile[1 + oldList.length];
+			newList[0] = pf;
+			System.arraycopy(oldList, 0, newList, 1, oldList.length);
+			n = new PackList(o.lastModified, newList);
 		} while (!packList.compareAndSet(o, n));
 	}
 
 	private void removePack(final PackFile deadPack) {
-		PackFile[] o, n;
+		PackList o, n;
 		do {
 			o = packList.get();
-			if (o == null || !inList(o, deadPack)) {
-				break;
 
-			} else if (o.length == 1) {
-				n = NO_PACKS;
+			final PackFile[] oldList = o.packs;
+			final int j = indexOf(oldList, deadPack);
+			if (j < 0)
+				break;
 
-			} else {
-				n = new PackFile[o.length - 1];
-				int j = 0;
-				for (final PackFile p : o) {
-					if (p != deadPack) {
-						n[j++] = p;
-					}
-				}
-			}
+			final PackFile[] newList = new PackFile[oldList.length - 1];
+			System.arraycopy(oldList, 0, newList, 0, j);
+			System.arraycopy(oldList, j + 1, newList, j, newList.length - j);
+			n = new PackList(o.lastModified, newList);
 		} while (!packList.compareAndSet(o, n));
 		deadPack.close();
 	}
 
-	private static boolean inList(final PackFile[] list, final PackFile pack) {
-		for (final PackFile p : list) {
-			if (p == pack) {
-				return true;
-			}
+	private static int indexOf(final PackFile[] list, final PackFile pack) {
+		for (int i = 0; i < list.length; i++) {
+			if (list[i] == pack)
+				return i;
 		}
-		return false;
-	}
-
-	private PackFile[] packs() {
-		PackFile[] r = packList.get();
-		if (r == null)
-			r = scanPacks(null);
-		return r;
+		return -1;
 	}
 
-	private PackFile[] scanPacks(final PackFile[] original) {
+	private PackList scanPacks(final PackList original) {
 		synchronized (packList) {
-			PackFile[] o, n;
+			PackList o, n;
 			do {
 				o = packList.get();
 				if (o != original) {
@@ -333,14 +316,15 @@ private static boolean inList(final PackFile[] list, final PackFile pack) {
 					//
 					return o;
 				}
-				n = scanPacksImpl(o != null ? o : NO_PACKS);
+				n = scanPacksImpl(o);
 			} while (!packList.compareAndSet(o, n));
 			return n;
 		}
 	}
 
-	private PackFile[] scanPacksImpl(final PackFile[] old) {
+	private PackList scanPacksImpl(final PackList old) {
 		final Map<String, PackFile> forReuse = reuseMap(old);
+		final long lastModified = packDirectory.lastModified();
 		final Set<String> names = listPackDirectory();
 		final List<PackFile> list = new ArrayList<PackFile>(names.size() >> 2);
 		for (final String indexName : names) {
@@ -374,17 +358,17 @@ private static boolean inList(final PackFile[] list, final PackFile pack) {
 			p.close();
 		}
 
-		if (list.isEmpty()) {
-			return NO_PACKS;
-		}
+		if (list.isEmpty())
+			return new PackList(lastModified, NO_PACKS.packs);
+
 		final PackFile[] r = list.toArray(new PackFile[list.size()]);
 		Arrays.sort(r, PackFile.SORT);
-		return r;
+		return new PackList(lastModified, r);
 	}
 
-	private static Map<String, PackFile> reuseMap(final PackFile[] old) {
+	private static Map<String, PackFile> reuseMap(final PackList old) {
 		final Map<String, PackFile> forReuse = new HashMap<String, PackFile>();
-		for (final PackFile p : old) {
+		for (final PackFile p : old.packs) {
 			if (p.invalid()) {
 				// The pack instance is corrupted, and cannot be safely used
 				// again. Do not include it in our reuse map.
@@ -408,7 +392,6 @@ private static boolean inList(final PackFile[] list, final PackFile pack) {
 	}
 
 	private Set<String> listPackDirectory() {
-		packDirectoryLastModified = packDirectory.lastModified();
 		final String[] nameList = packDirectory.list();
 		if (nameList == null)
 			return Collections.emptySet();
@@ -454,4 +437,21 @@ private ObjectDatabase openAlternate(final String location)
 		}
 		return new ObjectDirectory(objdir);
 	}
+
+	private static final class PackList {
+		/** Last modification time of {@link ObjectDirectory#packDirectory}. */
+		final long lastModified;
+
+		/** All known packs, sorted by {@link PackFile#SORT}. */
+		final PackFile[] packs;
+
+		PackList(final long lastModified, final PackFile[] packs) {
+			this.lastModified = lastModified;
+			this.packs = packs;
+		}
+
+		boolean tryAgain(final long currLastModified) {
+			return lastModified < currLastModified;
+		}
+	}
 }
-- 
1.6.4.225.gb589e

^ permalink raw reply related

* [JGIT PATCH (RESEND) 0/4] Fix race condition on loading pack files
From: Shawn O. Pearce @ 2009-08-12 19:45 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git

I had sent this series while you were on vacation, so I assume it
just got lost in the shuffle and am resending.

This series tries to fix a "racy git" problem when reading pack
files in a repository; its possible that the directory is modified,
read and modified again in the same filesystem timestamp, resulting
in the reader not noticing the second modification.  This may cause
a reader to lose access to objects on disk.

Shawn O. Pearce (4):
  Avoid unnecessary stat when scanning packs in the objects directory
  Make ObjectDirectory last modified time atomically updated with list
  Don't create new pack lists if the directory hasn't changed
  Fix racy condition when a repository is repacked

 .../src/org/spearce/jgit/lib/ObjectDirectory.java  |  235 ++++++++++++-------
 1 files changed, 148 insertions(+), 87 deletions(-)

^ permalink raw reply

* Re: git index: how does it work?
From: Björn Steinbrink @ 2009-08-12 19:39 UTC (permalink / raw)
  To: Shaun Cutts; +Cc: Sverre Rabbelier, Git List
In-Reply-To: <410DBF0B-80A2-4235-B566-B622CA976DA0@cuttshome.net>

[Please don't top-post, fixed that for you...]

On 2009.08.12 20:45:48 +0200, Shaun Cutts wrote:
> On Aug 12, 2009, at 7:47 PM, Sverre Rabbelier wrote:
> >Heya,
> >
> >On Wed, Aug 12, 2009 at 04:52, Shaun Cutts<shaun@cuttshome.net> wrote:
> >>Are renames being tracked by the index, and is there a more
> >>basic interface
> >>than "status" to query about them?
> >
> >Nope, git never explicitly tracks renames. Try this:
> >$ mv foo bar
> >$ git rm --cached foo
> >$ git add bar
> >$ git status
> >
> >It'll tell you that you renamed foo to bar, even if you never
> >executed 'git mv'.
> >
> >This is because git does rename _detection_, that is, it'll notice
> >that you have another file with (almost) the same contents, so it
> >assumes you did a rename.
>
> Aha ---
> 
> that explains it, then.
> 
> Is there a lower-level interface to rename detection than via
> "status"? And... um... hmmm.... how does it work? The hash codes
> don't help for "almost" the same. Is there an approximate string
> matching algorithm built in somewhere?

Roughly, it works like this:
The files are split into small chunks, those are hashed, and if there
are chunks with the same hash in both files, those chunks are treated as
being common to both files. The more the files have in common, the
higher the similarity score. See estimate_similarity() for details.

As the splitting also happens at newlines this has some interesting
effects, for example, you can completely reorder the lines in a file
after renaming it, and git will still detect it as a rename.

Björn

^ permalink raw reply

* [JGIT PATCH] Rename RevTag.getName() to RevTag.getTagName()
From: Shawn O. Pearce @ 2009-08-12 19:39 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git

The method getName() conflicts semantically with the method name()
we have inherited from our base class, ObjectId.  It is a rather
unfortunate turn of events that for performance reasons we wind up
subclassing what should be a property of this class, but since we
do that we need to pay attention to the methods declared on our
base class.

We want to use getName() to be a mirror of name() on AnyObjectId,
as it has a more JavaBeans style feel to the accessing of that
particular value.  So, rename getTagName() so it doesn't wind up
conflicting with the SHA-1 hex formatted string.

Noticed-by: Alex Blewitt <alex.blewitt@gmail.com>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/revwalk/RevTag.java       |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java
index 2fab266..51ff49b 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevTag.java
@@ -56,7 +56,7 @@
 
 	private byte[] buffer;
 
-	private String name;
+	private String tagName;
 
 	/**
 	 * Create a new tag reference.
@@ -96,7 +96,7 @@ void parseCanonical(final RevWalk walk, final byte[] rawTag)
 
 		int p = pos.value += 4; // "tag "
 		final int nameEnd = RawParseUtils.nextLF(rawTag, p) - 1;
-		name = RawParseUtils.decode(Constants.CHARSET, rawTag, p, nameEnd);
+		tagName = RawParseUtils.decode(Constants.CHARSET, rawTag, p, nameEnd);
 
 		if (walk.isRetainBody())
 			buffer = rawTag;
@@ -186,7 +186,7 @@ public final String getShortMessage() {
 	 * @return parsed tag.
 	 */
 	public Tag asTag(final RevWalk walk) {
-		return new Tag(walk.db, this, name, buffer);
+		return new Tag(walk.db, this, tagName, buffer);
 	}
 
 	/**
@@ -204,7 +204,7 @@ public final RevObject getObject() {
 	 * @return name of the tag, according to the tag header.
 	 */
 	public final String getName() {
-		return name;
+		return tagName;
 	}
 
 	final void disposeBody() {
-- 
1.6.4.225.gb589e

^ permalink raw reply related

* Re: [EGIT PATCH] Provide a more JavaBeans-style 'getName' accessor for the id Signed-off-by: Alex Blewitt <alex.blewitt@gmail.com>
From: Robin Rosenberg @ 2009-08-12 19:09 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Alex Blewitt, git@vger.kernel.org
In-Reply-To: <20090812142049.GA1033@spearce.org>

onsdag 12 augusti 2009 16:20:49 skrev "Shawn O. Pearce" <spearce@spearce.org>:
> Alex Blewitt <alex.blewitt@gmail.com> wrote:
> > On 10 Aug 2009, at 21:59, "Shawn O." <spearce@spearce.org> wrote:
> >>
> >>>> ./org/spearce/jgit/revwalk/RevTag.java:206: getName() in
> >>>> org.spearce.jgit.revwalk.RevTag cannot override getName() in
> >>>> org.spearce.jgit.lib.AnyObjectId; overridden method is final
> >>
> >> I can't apply this patch because getName() on RevTag is already
> >> defined with a different meaning.  :-(
> >
> > That sounds dangerous. We now have a .name() and a .getName() with  
> > different semantics. Can we not change the RevTag method name to  
> > something else so that we dont have an inconsistency?
> 
> Good point.  We didn't think that RevTag.getName method through
> very well.  Rename it to getTagName() ?

Ok with me.

-- robin

^ permalink raw reply

* Re: git index: how does it work?
From: Shaun Cutts @ 2009-08-12 18:45 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: Git List
In-Reply-To: <fabb9a1e0908121047jf7e6cf1n757cde626c79830c@mail.gmail.com>

Aha ---

that explains it, then.

Is there a lower-level interface to rename detection than via  
"status"? And... um... hmmm.... how does it work? The hash codes don't  
help for "almost" the same. Is there an approximate string matching  
algorithm built in somewhere?

Thanks,

-- Shaun

On Aug 12, 2009, at 7:47 PM, Sverre Rabbelier wrote:

> Heya,
>
> On Wed, Aug 12, 2009 at 04:52, Shaun Cutts<shaun@cuttshome.net> wrote:
>> Are renames being tracked by the index, and is there a more basic  
>> interface
>> than "status" to query about them?
>
> Nope, git never explicitly tracks renames. Try this:
> $ mv foo bar
> $ git rm --cached foo
> $ git add bar
> $ git status
>
> It'll tell you that you renamed foo to bar, even if you never  
> executed 'git mv'.
>
> This is because git does rename _detection_, that is, it'll notice
> that you have another file with (almost) the same contents, so it
> assumes you did a rename.
>
> -- 
> Cheers,
>
> Sverre Rabbelier
>

^ permalink raw reply

* Re: Tie a CVS-aware app into GIT?
From: Martin Langhoff @ 2009-08-12 18:34 UTC (permalink / raw)
  To: david.hagood; +Cc: git
In-Reply-To: <b988339dcd1bf764f0da46db763552d8.squirrel@localhost>

On Wed, Aug 12, 2009 at 2:01 PM, <david.hagood@gmail.com> wrote:
> I've thought about git-cvsserver as a solution, but I don't know if it can
> be run on a local machine to access a local repository.

Even though the docs say you can't (well, you should have to run it
over pserver) ISTR there is a way to get the cvs client to instantiate
a server using the CVS_SERVER envvar even for local ops.

Worth checking in the more detailed CVS user guide.



m
-- 
 martin.langhoff@gmail.com
 martin@laptop.org -- School Server Architect
 - ask interesting questions
 - don't get distracted with shiny stuff  - working code first
 - http://wiki.laptop.org/go/User:Martinlanghoff

^ permalink raw reply

* [ANNOUNCE] Scumd
From: Michael Gaffney @ 2009-08-12 18:28 UTC (permalink / raw)
  To: git

Greetings,
    This is an initial announcement of SCuMD, a pure java git sshd 
daemon. The source is at git://github.com/gaffo/scumd. The impetus 
behind SCuMD is to provide a highly configurable git daemon which can 
authenticate and authorize off of flat files, databases, LDAP, web 
services, or any other resource you can think of. SCuMD's other goal is 
to remove the need to serve off of normal sshd which some find to be a 
security risk on the open Internet. Currently SCuMD supports LDAP as the 
authentication module but coding other modules is quite simple.
    I would welcome any feedback including a better name. SCuMD stands 
for SCM Daemon.

Thanks,
    Mike Gaffney

^ permalink raw reply

* Tie a CVS-aware app into GIT?
From: david.hagood @ 2009-08-12 18:01 UTC (permalink / raw)
  To: git

I have an application (closed source, unfortunately) that can use CVS to
manage its files (specifically, Enterprise Architect by Sparx).

I'd rather use GIT to manage the files, but EA doesn't "speak" git - just
CVS and Subversion (and Microsoft's SCC protocol, but...)

Are there any programs which
1) accept the same command line parameters as CVS or Subversion
and
2) access a GIT repository.

git-svn is almost exactly NOT what I need, as it accepts "git" type
command line parameters and access a SVN repo, rather than accepting SVN
command line parameters and accessing a GIT repo.

Now, obviously, such a tool wouldn't have access to all the things that
GIT can do, and that is NOT what I am expecting - what I want is just to
enable EA to do the things it needs to do, namely adding/removing/moving
files from a repo. Branching, commits, and so on can be done manually.

I've thought about git-cvsserver as a solution, but I don't know if it can
be run on a local machine to access a local repository.

I'd appreciate any help on this.

^ 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