{"thread":{"id":"14367","subject":"[JGIT PATCH 1/4] Avoid deadlock while fetching from local repository","startedAt":"2008-07-09T04:15:27Z","lastAt":"2008-07-09T04:15:31Z","messageCount":5,"participants":["Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"82680","messageId":"1215576931-4174-1-git-send-email-spearce@spearce.org","threadId":"14367","inReplyTo":null,"subject":"[JGIT PATCH 0/4] Misc. push transport fixes","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-07-09T04:15:27Z","receivedAt":"2008-07-09T04:15:27Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Few minor bugs related to our push implementation.  A deadlock\nis fixed in the local fetch case and pack generation to include\nannotated tags is also fixed.\n\nWe also now do essentially \"git init\" on the remote repository\nif we are creating it over a dumb transport during the first push\nrequest made to it.\n\n\nShawn O. Pearce (4):\n  Avoid deadlock while fetching from local repository\n  Fix pushing of annotated tags to actually include the tag object\n  Automatically initialize a new dumb repository during push\n  Use a singleton for the NullProgressMonitor implementation\n\n .../org/spearce/jgit/lib/NullProgressMonitor.java  |    3 +\n .../src/org/spearce/jgit/revwalk/ObjectWalk.java   |   13 ++++-\n .../spearce/jgit/transport/BasePackConnection.java |    7 ++-\n .../jgit/transport/BasePackFetchConnection.java    |    1 +\n .../jgit/transport/BasePackPushConnection.java     |    1 +\n .../spearce/jgit/transport/WalkPushConnection.java |   54 ++++++++++++++++++++\n .../src/org/spearce/jgit/util/TemporaryBuffer.java |    2 +-\n 7 files changed, 77 insertions(+), 4 deletions(-)\n"},{"id":"82678","messageId":"1215576931-4174-2-git-send-email-spearce@spearce.org","threadId":"14367","inReplyTo":"1215576931-4174-1-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH 1/4] Avoid deadlock while fetching from local repository","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-07-09T04:15:28Z","receivedAt":"2008-07-09T04:15:28Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"We cannot send a packet line end command to git-upload-pack after we\nhave sent our want list and obtained a pack back from it.  Once the\nwant list has ended git-upload-pack wants no further data sent to\nit, and attempting to write more may cause us to deadlock as the\npipe won't accept the data.\n\nNot sending the packet line end if we don't send a want list is a\n(minor) protocol error.  To avoid these protocol errors (which\nmay display on stderr from git-upload-pack) we still send the end\nduring close if we have not yet sent a want list.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n .../spearce/jgit/transport/BasePackConnection.java |    7 ++++++-\n .../jgit/transport/BasePackFetchConnection.java    |    1 +\n .../jgit/transport/BasePackPushConnection.java     |    1 +\n 3 files changed, 8 insertions(+), 1 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackConnection.java\nindex a878f01..7dc4620 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackConnection.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackConnection.java\n@@ -83,6 +83,9 @@ abstract class BasePackConnection extends BaseConnection {\n \t/** Packet line encoder around {@link #out}. */\n \tprotected PacketLineOut pckOut;\n \n+\t/** Send {@link PacketLineOut#end()} before closing {@link #out}? */\n+\tprotected boolean outNeedsEnd;\n+\n \t/** Capability tokens advertised by the remote side. */\n \tprivate final Set<String> remoteCapablities = new HashSet<String>();\n \n@@ -99,6 +102,7 @@ abstract class BasePackConnection extends BaseConnection {\n \n \t\tpckIn = new PacketLineIn(in);\n \t\tpckOut = new PacketLineOut(out);\n+\t\toutNeedsEnd = true;\n \t}\n \n \tprotected void readAdvertisedRefs() throws TransportException {\n@@ -195,7 +199,8 @@ abstract class BasePackConnection extends BaseConnection {\n \tpublic void close() {\n \t\tif (out != null) {\n \t\t\ttry {\n-\t\t\t\tpckOut.end();\n+\t\t\t\tif (outNeedsEnd)\n+\t\t\t\t\tpckOut.end();\n \t\t\t\tout.close();\n \t\t\t} catch (IOException err) {\n \t\t\t\t// Ignore any close errors.\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java\nindex 04a91bf..12b36f2 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java\n@@ -253,6 +253,7 @@ abstract class BasePackFetchConnection extends BasePackConnection implements\n \t\t\tpckOut.writeString(line.toString());\n \t\t}\n \t\tpckOut.end();\n+\t\toutNeedsEnd = false;\n \t\treturn !first;\n \t}\n \ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackPushConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackPushConnection.java\nindex 784a578..6d95eaf 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackPushConnection.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackPushConnection.java\n@@ -148,6 +148,7 @@ class BasePackPushConnection extends BasePackConnection implements\n \t\tif (monitor.isCancelled())\n \t\t\tthrow new TransportException(uri, \"push cancelled\");\n \t\tpckOut.end();\n+\t\toutNeedsEnd = false;\n \t}\n \n \tprivate String enableCapabilties() {\n-- \n1.5.6.74.g8a5e\n"},{"id":"82679","messageId":"1215576931-4174-3-git-send-email-spearce@spearce.org","threadId":"14367","inReplyTo":"1215576931-4174-2-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH 2/4] Fix pushing of annotated tags to actually include the tag object","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-07-09T04:15:29Z","receivedAt":"2008-07-09T04:15:29Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"When we pushed an annotated tag to a remote repository jgit was\nfailing to include the annotated tag object itself as ObjectWalk\nnextObject() failed to pop the object out of the internal pending\nqueue and return it for inclusion in the generated pack file.\n\nWe need to use two different flags for SEEN and IN_PENDING as a blob\nor tree object can be SEEN during a TreeWalk, yet may also have been\ninserted into the pending object list.  SEEN means we have output\nthe object to the caller, IN_PENDING means we have placed it into\nthe pending object list.  Together these prevent duplicates in the\nqueue and duplicate return values.\n\nNoticed by Robin while trying to push an annotated tag:\n\n  $ git tag -m foo X\n  $ git rev-parse X\n  49aa0e93621...\n\n  $ jgit push somerepo refs/tags/X\n  error: unpack should have generated 49aa0e93621...\n  To somerepo.git\n   ! [remote rejected] X -> X (bad pack)\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n .../src/org/spearce/jgit/revwalk/ObjectWalk.java   |   13 +++++++++++--\n 1 files changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/ObjectWalk.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/ObjectWalk.java\nindex 6a5b857..1ba21eb 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/ObjectWalk.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/ObjectWalk.java\n@@ -66,6 +66,15 @@ import org.spearce.jgit.treewalk.TreeWalk;\n  * commits that are returned first.\n  */\n public class ObjectWalk extends RevWalk {\n+\t/**\n+\t * Indicates a non-RevCommit is in {@link #pendingObjects}.\n+\t * <p>\n+\t * We can safely reuse {@link RevWalk#REWRITE} here for the same value as it\n+\t * is only set on RevCommit and {@link #pendingObjects} never has RevCommit\n+\t * instances inserted into it.\n+\t */\n+\tprivate static final int IN_PENDING = RevWalk.REWRITE;\n+\n \tprivate final TreeWalk treeWalk;\n \n \tprivate BlockObjQueue pendingObjects;\n@@ -361,8 +370,8 @@ public class ObjectWalk extends RevWalk {\n \t}\n \n \tprivate void addObject(final RevObject o) {\n-\t\tif ((o.flags & SEEN) == 0) {\n-\t\t\to.flags |= SEEN;\n+\t\tif ((o.flags & IN_PENDING) == 0) {\n+\t\t\to.flags |= IN_PENDING;\n \t\t\tpendingObjects.add(o);\n \t\t}\n \t}\n-- \n1.5.6.74.g8a5e\n"},{"id":"82681","messageId":"1215576931-4174-4-git-send-email-spearce@spearce.org","threadId":"14367","inReplyTo":"1215576931-4174-3-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH 3/4] Automatically initialize a new dumb repository during push","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-07-09T04:15:30Z","receivedAt":"2008-07-09T04:15:30Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"If we are pushing into a repository which has no refs and no packs\nwe can reasonably assume the repository is brand new.  In such\ncases we must ensure there is a valid HEAD and config file in the\nrepository, otherwise Git clients reading the repository may not\nbe able to recognize its contents correctly.\n\nBy default we try to create HEAD from refs/heads/master, if that\nis among the branches being pushed by the user.  This mirrors what\ngit-init would have initialized HEAD to be.  However if master was\nnot among the branches pushed then another branch is selected as a\nreasonable default.\n\nWe initialize .git/config to a very bare configuration, listing\nonly the repository version.  We avoid setting up core.filemode as\nwe cannot genrally detect if the remote side understands executable\nmode bits.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n .../spearce/jgit/transport/WalkPushConnection.java |   54 ++++++++++++++++++++\n 1 files changed, 54 insertions(+), 0 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java\nindex e11b85a..bb5a653 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkPushConnection.java\n@@ -48,6 +48,7 @@ import java.util.TreeMap;\n \n import org.spearce.jgit.errors.TransportException;\n import org.spearce.jgit.lib.AnyObjectId;\n+import org.spearce.jgit.lib.Constants;\n import org.spearce.jgit.lib.ObjectId;\n import org.spearce.jgit.lib.PackWriter;\n import org.spearce.jgit.lib.ProgressMonitor;\n@@ -151,6 +152,12 @@ class WalkPushConnection extends BaseConnection implements PushConnection {\n \t\tfor (final RemoteRefUpdate u : updates)\n \t\t\tupdateCommand(u);\n \n+\t\t// Is this a new repository? If so we should create additional\n+\t\t// metadata files so it is properly initialized during the push.\n+\t\t//\n+\t\tif (!updates.isEmpty() && isNewRepository())\n+\t\t\tcreateNewRepository(updates);\n+\n \t\tif (!packedRefUpdates.isEmpty()) {\n \t\t\ttry {\n \t\t\t\tdest.writePackedRefs(newRefs.values());\n@@ -307,4 +314,51 @@ class WalkPushConnection extends BaseConnection implements PushConnection {\n \t\t\tu.setMessage(e.getMessage());\n \t\t}\n \t}\n+\n+\tprivate boolean isNewRepository() {\n+\t\treturn getRefsMap().isEmpty() && packNames != null\n+\t\t\t\t&& packNames.isEmpty();\n+\t}\n+\n+\tprivate void createNewRepository(final List<RemoteRefUpdate> updates)\n+\t\t\tthrows TransportException {\n+\t\ttry {\n+\t\t\tfinal String ref = \"ref: \" + pickHEAD(updates) + \"\\n\";\n+\t\t\tfinal byte[] bytes = ref.getBytes(Constants.CHARACTER_ENCODING);\n+\t\t\tdest.writeFile(\"../HEAD\", bytes);\n+\t\t} catch (IOException e) {\n+\t\t\tthrow new TransportException(uri, \"cannot create HEAD\", e);\n+\t\t}\n+\n+\t\ttry {\n+\t\t\tfinal String config = \"[core]\\n\"\n+\t\t\t\t\t+ \"\\trepositoryformatversion = 0\\n\";\n+\t\t\tfinal byte[] bytes = config.getBytes(Constants.CHARACTER_ENCODING);\n+\t\t\tdest.writeFile(\"../config\", bytes);\n+\t\t} catch (IOException e) {\n+\t\t\tthrow new TransportException(uri, \"cannot create config\", e);\n+\t\t}\n+\t}\n+\n+\tprivate static String pickHEAD(final List<RemoteRefUpdate> updates) {\n+\t\t// Try to use master if the user is pushing that, it is the\n+\t\t// default branch and is likely what they want to remain as\n+\t\t// the default on the new remote.\n+\t\t//\n+\t\tfor (final RemoteRefUpdate u : updates) {\n+\t\t\tfinal String n = u.getRemoteName();\n+\t\t\tif (n.equals(Constants.HEADS_PREFIX + \"/\" + Constants.MASTER))\n+\t\t\t\treturn n;\n+\t\t}\n+\n+\t\t// Pick any branch, under the assumption the user pushed only\n+\t\t// one to the remote side.\n+\t\t//\n+\t\tfor (final RemoteRefUpdate u : updates) {\n+\t\t\tfinal String n = u.getRemoteName();\n+\t\t\tif (n.startsWith(Constants.HEADS_PREFIX + \"/\"))\n+\t\t\t\treturn n;\n+\t\t}\n+\t\treturn updates.get(0).getRemoteName();\n+\t}\n }\n-- \n1.5.6.74.g8a5e\n"},{"id":"82682","messageId":"1215576931-4174-5-git-send-email-spearce@spearce.org","threadId":"14367","inReplyTo":"1215576931-4174-4-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH 4/4] Use a singleton for the NullProgressMonitor implementation","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-07-09T04:15:31Z","receivedAt":"2008-07-09T04:15:31Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"No reason to create a new instance every time we need to shove\na null monitor into a variable because someone passed us null.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n .../org/spearce/jgit/lib/NullProgressMonitor.java  |    3 +++\n .../src/org/spearce/jgit/util/TemporaryBuffer.java |    2 +-\n 2 files changed, 4 insertions(+), 1 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/NullProgressMonitor.java b/org.spearce.jgit/src/org/spearce/jgit/lib/NullProgressMonitor.java\nindex de75b90..191dc00 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/NullProgressMonitor.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/NullProgressMonitor.java\n@@ -42,6 +42,9 @@ package org.spearce.jgit.lib;\n  * A NullProgressMonitor does not report progress anywhere.\n  */\n public class NullProgressMonitor implements ProgressMonitor {\n+\t/** Immutable instance of a null progress monitor. */\n+\tpublic static final NullProgressMonitor INSTANCE = new NullProgressMonitor();\n+\n \tpublic void start(int totalTasks) {\n \t\t// Do not report.\n \t}\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/util/TemporaryBuffer.java b/org.spearce.jgit/src/org/spearce/jgit/util/TemporaryBuffer.java\nindex 20db580..d597c38 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/util/TemporaryBuffer.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/util/TemporaryBuffer.java\n@@ -200,7 +200,7 @@ public class TemporaryBuffer extends OutputStream {\n \tpublic void writeTo(final OutputStream os, ProgressMonitor pm)\n \t\t\tthrows IOException {\n \t\tif (pm == null)\n-\t\t\tpm = new NullProgressMonitor();\n+\t\t\tpm = NullProgressMonitor.INSTANCE;\n \t\tif (blocks != null) {\n \t\t\t// Everything is in core so we can stream directly to the output.\n \t\t\t//\n-- \n1.5.6.74.g8a5e\n"}]}