threads / patch / 14240

patch, 8 partsMinor push fixups

Subject: [JGIT PATCH 0/8] Minor push fixups

## tl;dr

9 messages between Jul 1, 2008 and Jul 1, 2008. Diffs are folded; open one to read it.

replies: 8people: 1as markdown or json

Shawn O. Pearce· Jul 1, 2008, 03:03 UTC · lore

A (small by recent standards) series to fix some issues with push in both the dumb and smart transports.

We also can now package jgit as a stand-alone shell+JAR combination; I install this into my path and have been favoring it over C Git for a few days now.

--
Shawn O. Pearce (8):
  Correct thin pack completion in IndexPack to handle some bundles
  Delete reflog when deleting ref during dumb transport push
  Refuse to create or delete funny ref names over dumb transports
  Shorten progress message text from PackWriter
  Correctly name the stderr redirection thread for local transport
  Support 'git upload-pack' and 'git receive-pack' over SSH
  Pack jgit into a portable single file command line utility
  Don't try to pack 0{40} during push of delete and update
 .gitignore                                         |    1 +
 jgit                                               |   37 ----------------
 jgit.sh                                            |   45 ++++++++++++++++++++
 make_jgit.sh                                       |   26 +++++++++++
 .../src/org/spearce/jgit/lib/PackWriter.java       |    4 +-
 .../jgit/transport/BasePackPushConnection.java     |    6 ++-
 .../src/org/spearce/jgit/transport/IndexPack.java  |   11 ++++-
 .../spearce/jgit/transport/TransportGitSsh.java    |    8 +++-
 .../org/spearce/jgit/transport/TransportLocal.java |    6 +-
 .../spearce/jgit/transport/WalkPushConnection.java |   14 ++++++
 .../jgit/transport/WalkRemoteObjectDatabase.java   |   13 ++++++
 11 files changed, 124 insertions(+), 47 deletions(-)
 create mode 100644 .gitignore
 delete mode 100755 jgit
 create mode 100755 jgit.sh
 create mode 100755 make_jgit.sh
Shawn O. Pearce· Jul 1, 2008, 03:03 UTC · re: Shawn O. Pearce · lore

[JGIT PATCH 1/8] Correct thin pack completion in IndexPack to handle some bundles

Recently I saw a bundle with a chain of deltas to base objects as A->B->C, where A was the delta depending on the base B. In this pack all of the objects used OBJ_REF_DELTA to link to their base and C was not in the pack (it was assumed to be in the repository).

Because of the ordering of the ObjectIds for B and C jgit tried to resolve A's delta base by pulling from the repository, as B was not found in the pack file. The reason B was not found was because it was waiting in the queue (to be processed next) and we did not know what B's ObjectId was.

By skipping objects whose ObjectLoader's aren't found we should be able to resolve those objects later when their delta base does get resolved in this pack. However by the end of of this loop we must have no more objects depending on something by ObjectId, as that is an indication that the local repository is missing the objects we must have to complete this thin pack.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/transport/IndexPack.java  |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)
Show changes to org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java +9 −2
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 24a0577..29d99db 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
@@ -55,6 +55,7 @@ import java.util.zip.Deflater;
 import java.util.zip.Inflater;
 
 import org.spearce.jgit.errors.CorruptObjectException;
+import org.spearce.jgit.errors.MissingObjectException;
 import org.spearce.jgit.lib.BinaryDelta;
 import org.spearce.jgit.lib.Constants;
 import org.spearce.jgit.lib.InflaterCache;
@@ -399,9 +400,10 @@ public class IndexPack {
 
 		final Deflater def = new Deflater(Deflater.DEFAULT_COMPRESSION, false);
 		long end = packOut.length() - 20;
-		while (!baseById.isEmpty()) {
-			final ObjectId baseId = baseById.keySet().iterator().next();
+		for (final ObjectId baseId : new ArrayList<ObjectId>(baseById.keySet())) {
 			final ObjectLoader ldr = repo.openObject(baseId);
+			if (ldr == null)
+				continue;
 			final byte[] data = ldr.getBytes();
 			final int typeCode = ldr.getType();
 			final PackedObjectInfo oe;
@@ -419,6 +421,11 @@ public class IndexPack {
 		}
 		def.end();
 
+		if (!baseById.isEmpty()) {
+			final ObjectId need = baseById.keySet().iterator().next();
+			throw new MissingObjectException(need, "delta base");
+		}
+
 		fixHeaderFooter();
 	}
 
-- 
1.5.6.74.g8a5e
Shawn O. Pearce· Jul 1, 2008, 03:03 UTC · re: Shawn O. Pearce · lore

[JGIT PATCH 2/8] Delete reflog when deleting ref during dumb transport push

If we remove a ref we must also remove the reflog.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../spearce/jgit/transport/WalkPushConnection.java |    7 +++++++
 .../jgit/transport/WalkRemoteObjectDatabase.java   |   13 +++++++++++++
 2 files changed, 20 insertions(+), 0 deletions(-)
Show changes to 2 files +20 −0

org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java, org.spearce.jgit/src/org/spearce/jgit/transport/WalkRemoteObjectDatabase.java

diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java
index f63bedd..5450b84 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java
@@ -280,6 +280,13 @@ class WalkPushConnection extends BaseConnection implements PushConnection {
 				u.setMessage(e.getMessage());
 			}
 		}
+
+		try {
+			dest.deleteRefLog(u.getRemoteName());
+		} catch (IOException e) {
+			u.setStatus(Status.REJECTED_OTHER_REASON);
+			u.setMessage(e.getMessage());
+		}
 	}
 
 	private void updateCommand(final RemoteRefUpdate u) {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkRemoteObjectDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkRemoteObjectDatabase.java
index 1e13b33..b99144a 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkRemoteObjectDatabase.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkRemoteObjectDatabase.java
@@ -269,6 +269,19 @@ abstract class WalkRemoteObjectDatabase {
 	}
 
 	/**
+	 * Delete a reflog from the remote repository.
+	 * 
+	 * @param name
+	 *            name of the ref within the ref space, for example
+	 *            <code>refs/heads/pu</code>.
+	 * @throws IOException
+	 *             deletion is not supported, or deletion failed.
+	 */
+	void deleteRefLog(final String name) throws IOException {
+		deleteFile("../logs/" + name);
+	}
+
+	/**
 	 * Overwrite (or create) a loose ref in the remote repository.
 	 * <p>
 	 * This method creates any missing parent directories, if necessary.
-- 
1.5.6.74.g8a5e
Shawn O. Pearce· Jul 1, 2008, 03:04 UTC · re: Shawn O. Pearce · lore

[JGIT PATCH 3/8] Refuse to create or delete funny ref names over dumb transports

If the RemoteRefUpdate requests us to create a remote name of just "master" (say due to a bug in the push command line tool) we would do just that, creating "$project.git/master" on the remote side. This is not a valid ref for Git and confusion ensues when C Git tries to operate on the same repository.

Normally these sorts of bad refs are blocked by git-receive-pack on the remote side, but here we don't have that so we must do the blocking as part of the push connection.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../spearce/jgit/transport/WalkPushConnection.java |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)
Show changes to org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java +7 −0
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java
index 5450b84..e11b85a 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java
@@ -129,6 +129,13 @@ class WalkPushConnection extends BaseConnection implements PushConnection {
 		//
 		final List<RemoteRefUpdate> updates = new ArrayList<RemoteRefUpdate>();
 		for (final RemoteRefUpdate u : refUpdates.values()) {
+			final String n = u.getRemoteName();
+			if (!n.startsWith("refs/") || !Repository.isValidRefName(n)) {
+				u.setStatus(Status.REJECTED_OTHER_REASON);
+				u.setMessage("funny refname");
+				continue;
+			}
+
 			if (AnyObjectId.equals(ObjectId.zeroId(), u.getNewObjectId()))
 				deleteCommand(u);
 			else
-- 
1.5.6.74.g8a5e
Shawn O. Pearce· Jul 1, 2008, 03:04 UTC · re: Shawn O. Pearce · lore

[JGIT PATCH 4/8] Shorten progress message text from PackWriter

There are traditionally three phases to a pack algorithm:
 the enumeration phase, commonly called "Counting objects"
 the delta search phase, commonly called "Compressing objects"
 the output phase, commonly called "Writing objects"

Our prior messages were too long for the text progress display and caused the progress meters to not line up vertically on the console. The shorter (and more traditional) text does fit into the column structure used by the text progress display.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/lib/PackWriter.java       |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Show changes to org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java +2 −2
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java b/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java
index 9a427da..0439656 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/PackWriter.java
@@ -102,7 +102,7 @@ public class PackWriter {
 	 *
 	 * @see #preparePack(Collection, Collection, boolean, boolean)
 	 */
-	public static final String COUNTING_OBJECTS_PROGRESS = "Counting objects to pack";
+	public static final String COUNTING_OBJECTS_PROGRESS = "Counting objects";
 
 	/**
 	 * Title of {@link ProgressMonitor} task used during searching for objects
@@ -110,7 +110,7 @@ public class PackWriter {
 	 *
 	 * @see #writePack(OutputStream)
 	 */
-	public static final String SEARCHING_REUSE_PROGRESS = "Searching for delta and object reuse";
+	public static final String SEARCHING_REUSE_PROGRESS = "Compressing objects";
 
 	/**
 	 * Title of {@link ProgressMonitor} task used during writing out pack
-- 
1.5.6.74.g8a5e
Shawn O. Pearce· Jul 1, 2008, 03:04 UTC · re: Shawn O. Pearce · lore

[JGIT PATCH 5/8] Correctly name the stderr redirection thread for local transport

When we open git-receive-pack for the local transport case we still create a thread to redirect any error messages to the System.err console. In this case the thread should be named after the command we are running.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../org/spearce/jgit/transport/TransportLocal.java |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
Show changes to org.spearce.jgit/src/org/spearce/jgit/transport/TransportLocal.java +3 −3
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportLocal.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportLocal.java
index 761d1b8..b112267 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportLocal.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportLocal.java
@@ -96,7 +96,7 @@ class TransportLocal extends PackTransport {
 		try {
 			final Process proc = Runtime.getRuntime().exec(
 					new String[] { cmd, "." }, null, remoteGitDir);
-			new StreamRewritingThread(proc.getErrorStream()).start();
+			new StreamRewritingThread(cmd, proc.getErrorStream()).start();
 			return proc;
 		} catch (IOException err) {
 			throw new TransportException(uri, err.getMessage(), err);
@@ -158,8 +158,8 @@ class TransportLocal extends PackTransport {
 	class StreamRewritingThread extends Thread {
 		private final InputStream in;
 
-		StreamRewritingThread(final InputStream in) {
-			super("JGit " + getOptionUploadPack() + " Errors");
+		StreamRewritingThread(final String cmd, final InputStream in) {
+			super("JGit " + cmd + " Errors");
 			this.in = in;
 		}
 
-- 
1.5.6.74.g8a5e
Shawn O. Pearce· Jul 1, 2008, 03:04 UTC · re: Shawn O. Pearce · lore

[JGIT PATCH 6/8] Support 'git upload-pack' and 'git receive-pack' over SSH

Within the next 6 months C git clients will begin asking remote servers for 'git $command' rather than 'git-$command' when using the SSH transport. This change is to allow the C git programs to be removed from the user's $PATH, leaving only the git wrapper binary.

For the first 6 months after C git 1.6.0 gets released clients will continue to ask for 'git-$command' but users may change that behavior by specifically asking for 'git $command' in remote.$name.uploadpack or remote.$name.receivepack. Later clients (including jgit) will change to ask for 'git $command' by default.

If we are asking for 'git $command' we cannot quote this as a single command with a space in the path. We split on the whitespace and quote both sides (if necessary) to protect the strings from the shell.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../spearce/jgit/transport/TransportGitSsh.java    |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)
Show changes to org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java +7 −1
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java
index caf531d..1bbdf04 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java
@@ -182,7 +182,13 @@ class TransportGitSsh extends PackTransport {
 				path = (uri.getPath().substring(1));
 
 			final StringBuilder cmd = new StringBuilder();
-			sqMinimal(cmd, exe);
+			final int gitspace = exe.indexOf("git ");
+			if (gitspace >= 0) {
+				sqMinimal(cmd, exe.substring(0, gitspace + 3));
+				cmd.append(' ');
+				sqMinimal(cmd, exe.substring(gitspace + 4));
+			} else
+				sqMinimal(cmd, exe);
 			cmd.append(' ');
 			sqAlways(cmd, path);
 			channel.setCommand(cmd.toString());
-- 
1.5.6.74.g8a5e
Shawn O. Pearce· Jul 1, 2008, 03:04 UTC · re: Shawn O. Pearce · lore

[JGIT PATCH 7/8] Pack jgit into a portable single file command line utility

On UNIX (and even Cygwin) we can package the entire jgit library into a single JAR file and prefix that JAR with a shell script to start the JVM, passing in the script/JAR file as the only classpath. This trick works because the table of contents for the ZIP file is at the end of the stream, and the JVM tends to only do random access through that table when it looks up classes for loading.

A tiny shell script called make_jgit.sh compiles everything with the command line javac and packages it into the portable jgit shell script, making it possible to build and use jgit without touching either Eclipse or Maven.

Currently this is quite easy to implement as we have only one support library (JSch), but things may get more complicated if we add another JAR to our classpath.

With this I can install JGit into my personal account and make use of it on a daily basis from the command line:

	./make_jgit.sh
	mv jgit ~/bin
	jgit push ...
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .gitignore   |    1 +
 jgit         |   37 -------------------------------------
 jgit.sh      |   45 +++++++++++++++++++++++++++++++++++++++++++++
 make_jgit.sh |   26 ++++++++++++++++++++++++++
 4 files changed, 72 insertions(+), 37 deletions(-)
 create mode 100644 .gitignore
 delete mode 100755 jgit
 create mode 100755 jgit.sh
 create mode 100755 make_jgit.sh
Show changes to 4 files +72 −37

.gitignore, jgit, jgit.sh, make_jgit.sh

diff --git a/.gitignore b/.gitignore
new file mode 100644
index 0000000..0763732
--- /dev/null
+++ b/.gitignore
@@ -0,0 +1 @@
+/jgit
diff --git a/jgit b/jgit
deleted file mode 100755
index be7df7e..0000000
--- a/jgit
+++ /dev/null
@@ -1,37 +0,0 @@
-#!/bin/sh
-
-jgit_home=`dirname $0`
-cp="$jgit_home/org.spearce.jgit/bin"
-cp="$cp:$jgit_home/org.spearce.jgit/lib/jsch-0.1.37.jar"
-unset jgit_home
-java_args=
-
-# Cleanup paths for Cygwin.
-#
-case "`uname`" in
-CYGWIN*)
-	cp=`cygpath --windows --mixed --path "$cp"`
-	;;
-Darwin)
-	if test -e /System/Library/Frameworks/JavaVM.framework
-	then
-		java_args='
-			-Dcom.apple.mrj.application.apple.menu.about.name=JGit
-			-Dcom.apple.mrj.application.growbox.intrudes=false
-			-Dapple.laf.useScreenMenuBar=true
-			-Xdock:name=JGit
-		'
-	fi
-	;;
-esac
-
-CLASSPATH="$cp"
-export CLASSPATH
-
-java=java
-if test -n "$JAVA_HOME"
-then
-	java="$JAVA_HOME/bin/java"
-fi
-
-exec "$java" $java_args org.spearce.jgit.pgm.Main "$@"
diff --git a/jgit.sh b/jgit.sh
new file mode 100755
index 0000000..84927bc
--- /dev/null
+++ b/jgit.sh
@@ -0,0 +1,45 @@
+#!/bin/sh
+
+if [ "@@use_self@@" = "1" ]
+then
+	this_script=`which "$0" 2>/dev/null`
+	[ $? -gt 0 -a -f "$0" ] && this_script="$0"
+	cp=$this_script
+else
+	jgit_home=`dirname $0`
+	cp="$jgit_home/org.spearce.jgit/bin"
+	cp="$cp:$jgit_home/org.spearce.jgit/lib/jsch-0.1.37.jar"
+	unset jgit_home
+	java_args=
+fi
+
+# Cleanup paths for Cygwin.
+#
+case "`uname`" in
+CYGWIN*)
+	cp=`cygpath --windows --mixed --path "$cp"`
+	;;
+Darwin)
+	if test -e /System/Library/Frameworks/JavaVM.framework
+	then
+		java_args='
+			-Dcom.apple.mrj.application.apple.menu.about.name=JGit
+			-Dcom.apple.mrj.application.growbox.intrudes=false
+			-Dapple.laf.useScreenMenuBar=true
+			-Xdock:name=JGit
+		'
+	fi
+	;;
+esac
+
+CLASSPATH="$cp"
+export CLASSPATH
+
+java=java
+if test -n "$JAVA_HOME"
+then
+	java="$JAVA_HOME/bin/java"
+fi
+
+exec "$java" $java_args org.spearce.jgit.pgm.Main "$@"
+exit 1
diff --git a/make_jgit.sh b/make_jgit.sh
new file mode 100755
index 0000000..3889dca
--- /dev/null
+++ b/make_jgit.sh
@@ -0,0 +1,26 @@
+#!/bin/sh
+
+PATH=$JAVA_HOME/bin:$PATH
+O=jgit
+T=".temp$$.$O"
+
+rm -f $O
+rm -rf $T $O+ org.spearce.jgit/bin2
+cp org.spearce.jgit/lib/jsch-0.1.37.jar $T &&
+mkdir org.spearce.jgit/bin2 &&
+(cd org.spearce.jgit/src &&
+ find . -name \*.java -type f |
+ xargs javac \
+	-source 1.5 \
+	-target 1.5 \
+	-g \
+	-d ../bin2 \
+	-cp ../lib/jsch-0.1.37.jar) &&
+jar uf $T -C org.spearce.jgit/bin2 . &&
+jar uf $T -C org.spearce.jgit META-INF &&
+sed s/@@use_self@@/1/ jgit.sh >$O+ &&
+cat $T >>$O+ &&
+chmod 555 $O+ &&
+mv $O+ $O &&
+echo "Created $O." &&
+rm -rf $T $O+ org.spearce.jgit/bin2
-- 
1.5.6.74.g8a5e
Shawn O. Pearce· Jul 1, 2008, 03:04 UTC · re: Shawn O. Pearce · lore

[JGIT PATCH 8/8] Don't try to pack 0{40} during push of delete and update

`jgit push origin :refs/heads/die refs/heads/master` tries to pack 0{40} to delete branch "die" while also packing the objects needed to update branch master. Since "die" is being removed we do not want to pack any objects for it, as there is nothing to pack.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../jgit/transport/BasePackPushConnection.java     |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
Show changes to org.spearce.jgit/src/org/spearce/jgit/transport/BasePackPushConnection.java +4 −2
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackPushConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackPushConnection.java
index 7ae3aa7..784a578 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackPushConnection.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackPushConnection.java
@@ -169,8 +169,10 @@ class BasePackPushConnection extends BasePackConnection implements
 
 		for (final Ref r : getRefs())
 			remoteObjects.add(r.getObjectId());
-		for (final RemoteRefUpdate r : refUpdates.values())
-			newObjects.add(r.getNewObjectId());
+		for (final RemoteRefUpdate r : refUpdates.values()) {
+			if (!ObjectId.zeroId().equals(r.getNewObjectId()))
+				newObjects.add(r.getNewObjectId());
+		}
 
 		writer.preparePack(newObjects, remoteObjects, thinPack, true);
 		writer.writePack(out);
-- 
1.5.6.74.g8a5e

← back to recent threads