threads / discuss / 19258

[JGIT] exception while pushing if no objects/pack directory exists in origin repo

Subject: [JGIT] exception while pushing if no objects/pack directory exists in origin repo

## tl;dr

4 messages between May 9, 2009 and May 10, 2009.

replies: 3people: 3as markdown or json

Mark Struberg· May 9, 2009, 20:40 UTC · lore
Hi!

I have problems with JGit pushing to an old origin repo in my TCK tests of maven-scm-providers-git.

This 'old' bare repository does not have an object/pack folder, it only has unpacked objects lying around. I created it in 2007-11 with git-1.5.3 so this is actually not so old, so I'm not sure why there is no pack folder. Maybe that's caused by some 'prune empty directories' and stuff, I really don't know.

If I use jgit to push to this repo it crashes because it cannot write to the pack folder. If I mkdir the object/pack folder in my origin repo, everything runs smooth.

The test git repo checked-in in my git repo ;)
http://github.com/struberg/maven-scm-providers-git/tree/82e3021576e18a09fc04e1a5ac58368f598167b2/maven-scm-provider-jgit/src/test/resources/repository
The Exception I get when running my TCK test:
org.apache.maven.scm.ScmException: JGit checkin failure!
	at
org.apache.maven.scm.provider.git.jgit.command.checkin.JGitCheckInCommand.executeCheckInCommand(JGitCheckInCommand.java:98)
	at
org.apache.maven.scm.command.checkin.AbstractCheckInCommand.executeCommand(AbstractCheckInCommand.java:53)
	at org.apache.maven.scm.command.AbstractCommand.execute(AbstractCommand.java:59)
	at
org.apache.maven.scm.provider.git.AbstractGitScmProvider.executeCommand(AbstractGitScmProvider.java:339)
	at
org.apache.maven.scm.provider.git.AbstractGitScmProvider.checkin(AbstractGitScmProvider.java:265)
	at org.apache.maven.scm.provider.AbstractScmProvider.checkIn(AbstractScmProvider.java:342)
	at org.apache.maven.scm.provider.AbstractScmProvider.checkIn(AbstractScmProvider.java:326)
	at org.apache.maven.scm.manager.AbstractScmManager.checkIn(AbstractScmManager.java:374)
	at
org.apache.maven.scm.tck.command.checkin.CheckInCommandTckTest.testCheckInCommandTest(CheckInCommandTckTest.java:74)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:597)
	at junit.framework.TestCase.runTest(TestCase.java:164)
	at junit.framework.TestCase.runBare(TestCase.java:130)
	at junit.framework.TestResult$1.protect(TestResult.java:106)
	at junit.framework.TestResult.runProtected(TestResult.java:124)
	at junit.framework.TestResult.run(TestResult.java:109)
	at junit.framework.TestCase.run(TestCase.java:120)
	at junit.framework.TestSuite.runTest(TestSuite.java:230)
	at junit.framework.TestSuite.run(TestSuite.java:225)
	at
org.eclipse.jdt.internal.junit.runner.junit3.JUnit3TestReference.run(JUnit3TestReference.java:130)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:460)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:673)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:386)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:196)
Caused by: org.spearce.jgit.errors.TransportException:
file:///home/msx/develop/java/maven2/maven-scm/maven-scm-providers/maven-scm-providers-git/maven-scm-provider-jgit/target/scm-test/repository:
error occurred during unpacking on the remote end: error Cannot move pack to
/home/msx/develop/java/maven2/maven-scm/maven-scm-providers/maven-scm-providers-git/maven-scm-provider-jgit/target/scm-test/repository/objects/pack/pack-737c284d140709e1182fa00fa1a20d4e370cf2d5.pack
	at
org.spearce.jgit.transport.BasePackPushConnection.readStatusReport(BasePackPushConnection.java:217)
	at org.spearce.jgit.transport.BasePackPushConnection.doPush(BasePackPushConnection.java:134)
	at org.spearce.jgit.transport.BasePackPushConnection.push(BasePackPushConnection.java:100)
	at org.spearce.jgit.transport.PushProcess.execute(PushProcess.java:127)
	at org.spearce.jgit.transport.Transport.push(Transport.java:713)
	at org.spearce.jgit.simple.SimpleRepository.push(SimpleRepository.java:504)
	at org.spearce.jgit.simple.SimpleRepository.push(SimpleRepository.java:436)
	at
org.apache.maven.scm.provider.git.jgit.command.checkin.JGitCheckInCommand.executeCheckInCommand(JGitCheckInCommand.java:66)
	... 26 more

Not sure if I only have a 'broken' repo (everything runs smooth on the shell) or if this is a thing which should get fixed in JGit...

txs and LieGrue, strub
Shawn O. Pearce· May 10, 2009, 21:30 UTC · re: Mark Struberg · lore

[PATCH] Create $GIT_DIR/objects/pack if missing during renameAndOpenPack

Its OK for the objects/pack directory to be missing, an empty repository doesn't have to have it. C Git's `git repack` will automatically create the directory if it is not present, so we need to do the same behavior here.

When the directory doesn't exist, we try to create it, but we may have creation fail if another concurrent thread/process also made the directory at the same time, hence we test once more after creation failure before throwing the exception.

Found-by: Mark Struberg <struberg@yahoo.de>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
  Mark Struberg <struberg@yahoo.de> wrote:
  > 
  > If I use jgit to push to this repo it crashes because it cannot write to the pack folder. If I
  > mkdir the object/pack folder in my origin repo, everything runs smooth.
  
  Yea, that's legal.  This patch should fix it.
  
 .../src/org/spearce/jgit/transport/IndexPack.java  |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
index b2bcbb7..1eb40d4 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
@@ -1037,6 +1037,14 @@ public PackLock renameAndOpenPack(final String lockMessage)
 		final File finalIdx = new File(packDir, "pack-" + name + ".idx");
 		final PackLock keep = new PackLock(finalPack);
 
+		if (!packDir.exists() && !packDir.mkdir() && !packDir.exists()) {
+			// The objects/pack directory isn't present, and we are unable
+			// to create it. There is no way to move this pack in.
+			//
+			cleanupTemporaryFiles();
+			throw new IOException("Cannot create " + packDir.getAbsolutePath());
+		}
+
 		if (finalPack.exists()) {
 			// If the pack is already present we should never replace it.
 			//

-- 
1.6.3.195.gad81
Robin Rosenberg· May 10, 2009, 22:59 UTC · re: Shawn O. Pearce · lore

Re: [PATCH] Create $GIT_DIR/objects/pack if missing during renameAndOpenPack

söndag 10 maj 2009 23:30:46 skrev "Shawn O. Pearce" <spearce@spearce.org>:
Show 9 quoted lines
> diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
> index b2bcbb7..1eb40d4 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
> @@ -1037,6 +1037,14 @@ public PackLock renameAndOpenPack(final String lockMessage)
>  		final File finalIdx = new File(packDir, "pack-" + name + ".idx");
>  		final PackLock keep = new PackLock(finalPack);
>  
> +		if (!packDir.exists() && !packDir.mkdir() && !packDir.exists()) {
Is there a lot to gain by this instead of just if (!packDir.mkdir() && !packDir.exists()) ?
-- robin
Shawn O. Pearce· May 10, 2009, 23:03 UTC · re: Robin Rosenberg · lore

Re: [PATCH] Create $GIT_DIR/objects/pack if missing during renameAndOpenPack

Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:
Show 12 quoted lines
> s?ndag 10 maj 2009 23:30:46 skrev "Shawn O. Pearce" <spearce@spearce.org>:
> > diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
> > index b2bcbb7..1eb40d4 100644
> > --- a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
> > +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
> > @@ -1037,6 +1037,14 @@ public PackLock renameAndOpenPack(final String lockMessage)
> >  		final File finalIdx = new File(packDir, "pack-" + name + ".idx");
> >  		final PackLock keep = new PackLock(finalPack);
> >  
> > +		if (!packDir.exists() && !packDir.mkdir() && !packDir.exists()) {
> 
> Is there a lot to gain by this instead of just if (!packDir.mkdir() && !packDir.exists()) ?

No, not really. But usually the directory exists, its rare when it doesn't.

We probably just wasted more CPU time in our MUAs debating this than it really costs in the long run. I'm too #@!** lazy to go back and amend this patch. But you can amend it if it really bothers you to do the "!exists && !mkdir && !exists".

-- 
Shawn.

← back to recent threads