{"thread":{"id":"18198","subject":"[JGit] Push to new Amazon S3 does not work? (\"funny refname\")","startedAt":"2009-03-07T16:05:02Z","lastAt":"2009-03-09T21:29:00Z","messageCount":8,"participants":["Daniel Cheng","Robin Rosenberg","Shawn O. Pearce"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"107308","messageId":"ff6a9c820903070805m34f792dard6b17d2029e41dfe@mail.gmail.com","threadId":"18198","inReplyTo":null,"subject":"[JGit] Push to new Amazon S3 does not work? (\"funny refname\")","fromName":"Daniel Cheng","fromEmail":"j16sdiz+freenet@gmail.com","sentAt":"2009-03-07T16:05:02Z","receivedAt":"2009-03-07T16:05:02Z","isPatch":false,"sender":{"key":"j16sdiz+freenet@gmail.com","avatar":"https://gravatar.com/avatar/3e796e8a156ee86e305bfc1fbe01608302554ee3b5fa7eb8d1213b8877bfcabd?d=mp&s=160"},"body":"Pushing to new Amazon S3 repository does not work.\nIt say \"funny refname\" without pushing anything:\n\n<<<<<<<<<\n$ jgit push s3 master\nTo amazon-s3://0NQ4APQ8R7S6HQ65TWR2@egitsdiz/1.git\n ! [remote rejected] master -> master (funny refname)\n$ s3cmd la\n DIR s3://egitsdiz/1.git/\n$\n>>>>>>>>>\n\nAny idea what's happening here?\n\n\nThe code is in WalkPushConnection.java line 137:\n<<<<<<<<<\n134 final List updates = new ArrayList();\n135 for (final RemoteRefUpdate u : refUpdates.values()) {\n136 final String n = u.getRemoteName();\n137 if (!n.startsWith(\"refs/\") || !Repository.isValidRefName(n)) {\n138 u.setStatus(Status.REJECTED_OTHER_REASON);\n139 u.setMessage(\"funny refname\");\n140 continue;\n141 }\n>>>>>>>>>\n\nu.getRemoteName() gives \"master\" here.\nRemoving n.startsWith(\"refs/\") would generate a bad `packed-refs`\nfile in later code.\nI tried to fix this, but failed to do so without breaking GitSsh transports\n"},{"id":"107317","messageId":"200903071850.38045.robin.rosenberg.lists@dewire.com","threadId":"18198","inReplyTo":"ff6a9c820903070805m34f792dard6b17d2029e41dfe@mail.gmail.com","subject":"Re: [JGit] Push to new Amazon S3 does not work? (\"funny refname\")","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg.lists@dewire.com","sentAt":"2009-03-07T17:50:37Z","receivedAt":"2009-03-07T17:50:37Z","isPatch":false,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"lördag 07 mars 2009 17:05:02 skrev Daniel Cheng :\n> Pushing to new Amazon S3 repository does not work.\n> It say \"funny refname\" without pushing anything:\n> \n> <<<<<<<<<\n> $ jgit push s3 master\n> To amazon-s3://0NQ4APQ8R7S6HQ65TWR2@egitsdiz/1.git\n> ! [remote rejected] master -> master (funny refname)\n> $ s3cmd la\n> DIR s3://egitsdiz/1.git/\n> $\n> >>>>>>>>>\n> \n> Any idea what's happening here?\n> \n> \n> The code is in WalkPushConnection.java line 137:\n> <<<<<<<<<\n> 134 final List updates = new ArrayList();\n> 135 for (final RemoteRefUpdate u : refUpdates.values()) {\n> 136 final String n = u.getRemoteName();\n> 137 if (!n.startsWith(\"refs/\") || !Repository.isValidRefName(n)) {\n> 138 u.setStatus(Status.REJECTED_OTHER_REASON);\n> 139 u.setMessage(\"funny refname\");\n> 140 continue;\n> 141 }\n> >>>>>>>>>\n> \n> u.getRemoteName() gives \"master\" here.\n> Removing n.startsWith(\"refs/\") would generate a bad `packed-refs`\n> file in later code.\n> I tried to fix this, but failed to do so without breaking GitSsh transports\n\nThis is not specific to s3. It seems jgit wants a fully qualified ref for the remote\nside, so refs/heads/master will work for the other protocols, and I guess s3 too.\n\n- robin\n"},{"id":"107347","messageId":"20090307211008.GP16213@spearce.org","threadId":"18198","inReplyTo":"200903071850.38045.robin.rosenberg.lists@dewire.com","subject":"Re: [JGit] Push to new Amazon S3 does not work? (\"funny refname\")","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-03-07T21:10:08Z","receivedAt":"2009-03-07T21:10:08Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Robin Rosenberg wrote:\n> lördag 07 mars 2009 17:05:02 skrev Daniel Cheng :\n> > Pushing to new Amazon S3 repository does not work.\n> > It say \"funny refname\" without pushing anything:\n> > \n> > <<<<<<<<<\n> > $ jgit push s3 master\n> > To amazon-s3://0NQ4APQ8R7S6HQ65TWR2@egitsdiz/1.git\n> > ! [remote rejected] master -> master (funny refname)\n> > $ s3cmd la\n> > DIR s3://egitsdiz/1.git/\n> > $\n> > >>>>>>>>>\n> \n> This is not specific to s3. It seems jgit wants a fully qualified ref for the remote\n> side, so refs/heads/master will work for the other protocols, and I guess s3 too.\n\nCorrect.\n\nThe \"jgit push\" command line client lacks the DWIMery of \"git push\".\n\nSpecifically, from a pure API usage perspective, \"jgit push\"\nis responsible for expanding the user input of \"master\" into the\n\"refs/heads/master:refs/heads/master\" refspec that the lower level\nPushProcess class wants.\n\nHere it failed to do that, and the lower-level transport (rightly)\nrejected the invalid ref name.\n\n Side note:\n\n That API definition that says the client should do the DWIMery\n of ref expansion also makes it nearly impossible to implement\n \"push matching\" or \"randomsha1:master\" refspec, as the client\n doesn't have the network connection open and doesn't have the\n advertised ref information early enough.\n\nThe reason we punted on this and didn't do this particular\nexpansion DWIMery in \"jgit push\" is we lack a good way to resolve\n\"master\" into \"refs/heads/master\", or \"v1.0\" into \"refs/tags/v1.0\".\nRepository does not expose the ref lookup algorithm, only resolve(),\nwhich converts \"master\" into a SHA-1 ObjectId.\n\nIf someone exposed this portion of the resolve logic in the\nRepository class, I think it would be a fairly simple change\nin Push to support this DIWMery.\n\nBut until then, you need to say:\n\n jgit push s3 refs/heads/master:refs/heads/master\n\nor maybe this DWIMery might work:\n\n jgit push s3 refs/heads/master\n\nIts been a while since I passed args. I usually have\nremote.$name.push in place for things that I push to.\n\n-- \nShawn.\n"},{"id":"107352","messageId":"1236464299-11491-1-git-send-email-robin.rosenberg@dewire.com","threadId":"18198","inReplyTo":"20090307211008.GP16213@spearce.org","subject":"[EGIT PATCH] Evaluate short refnames into full names during push","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2009-03-07T22:18:19Z","receivedAt":"2009-03-07T22:18:19Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"With this we can use short names like master instead of refs/heads/master\nwhen pushing. This is slightly more convenient. Pushing a delete still\nrequires the long format.\n\nSigned-off-by: Robin Rosenberg \n---\n .../src/org/spearce/jgit/lib/Repository.java | 11 ++++++++++\n .../src/org/spearce/jgit/transport/Transport.java | 21 +++++++++++++++++--\n 2 files changed, 29 insertions(+), 3 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\nindex 30bd4a3..3ab51b1 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java\n@@ -928,6 +928,17 @@ public String getBranch() throws IOException {\n \t}\n \t\n \t/**\n+\t * Get a ref by name.\n+\t * \n+\t * @param name\n+\t * @return the Ref with the given name, or null if it does not exist\n+\t * @throws IOException\n+\t */\n+\tpublic Ref getRef(final String name) throws IOException {\n+\t\treturn refs.readRef(name);\n+\t}\n+\n+\t/**\n \t * @return all known refs (heads, tags, remotes).\n \t */\n \tpublic Map getAllRefs() {\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java b/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java\nindex 3aec5ca..64745a8 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java\n@@ -51,6 +51,7 @@\n \n import org.spearce.jgit.errors.NotSupportedException;\n import org.spearce.jgit.errors.TransportException;\n+import org.spearce.jgit.lib.Constants;\n import org.spearce.jgit.lib.NullProgressMonitor;\n import org.spearce.jgit.lib.ProgressMonitor;\n import org.spearce.jgit.lib.Ref;\n@@ -243,10 +244,24 @@ else if (TransportLocal.canHandle(remote))\n \t\tfinal Collection procRefs = expandPushWildcardsFor(db, specs);\n \n \t\tfor (final RefSpec spec : procRefs) {\n-\t\t\tfinal String srcRef = spec.getSource();\n+\t\t\tString srcRef = spec.getSource();\n+\t\t\tfinal Ref src = db.getRef(srcRef);\n+\t\t\tif (src != null)\n+\t\t\t\tsrcRef = src.getName();\n+\t\t\tString remoteName = spec.getDestination();\n \t\t\t// null destination (no-colon in ref-spec) is a special case\n-\t\t\tfinal String remoteName = (spec.getDestination() == null ? spec\n-\t\t\t\t\t.getSource() : spec.getDestination());\n+\t\t\tif (remoteName == null) {\n+\t\t\t\tremoteName = srcRef; \n+\t\t\t} else {\n+\t\t\t\tif (!remoteName.startsWith(Constants.R_REFS)) {\n+\t\t\t\t\t// null source is another special case (delete)\n+\t\t\t\t\tif (srcRef != null) {\n+\t\t\t\t\t\t// assume the same type of ref at the destination\n+\t\t\t\t\t\tString srcPrefix = srcRef.substring(0, srcRef.indexOf('/', Constants.R_REFS.length()));\n+\t\t\t\t\t\tremoteName = srcPrefix + \"/\" + remoteName;\n+\t\t\t\t\t}\n+\t\t\t\t}\n+\t\t\t}\n \t\t\tfinal boolean forceUpdate = spec.isForceUpdate();\n \t\t\tfinal String localName = findTrackingRefName(remoteName, fetchSpecs);\n \n-- \n1.6.1.285.g35d8b\n"},{"id":"107354","messageId":"20090307224831.GS16213@spearce.org","threadId":"18198","inReplyTo":"1236464299-11491-1-git-send-email-robin.rosenberg@dewire.com","subject":"Re: [EGIT PATCH] Evaluate short refnames into full names during push","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-03-07T22:48:31Z","receivedAt":"2009-03-07T22:48:31Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Robin Rosenberg wrote:\n> @@ -243,10 +244,24 @@ else if (TransportLocal.canHandle(remote))\n> \t\tfinal Collection procRefs = expandPushWildcardsFor(db, specs);\n> \n> \t\tfor (final RefSpec spec : procRefs) {\n> -\t\t\tfinal String srcRef = spec.getSource();\n> +\t\t\tString srcRef = spec.getSource();\n> +\t\t\tfinal Ref src = db.getRef(srcRef);\n> +\t\t\tif (src != null)\n> +\t\t\t\tsrcRef = src.getName();\n> +\t\t\tString remoteName = spec.getDestination();\n> \t\t\t// null destination (no-colon in ref-spec) is a special case\n> -\t\t\tfinal String remoteName = (spec.getDestination() == null ? spec\n> -\t\t\t\t\t.getSource() : spec.getDestination());\n\nOh, right. I forgot about the fact that Marek put the code here, as\nthen \"push = master\" in a config file works...\n\nOK. I'll apply.\n\n-- \nShawn.\n"},{"id":"107384","messageId":"1236525667-852-1-git-send-email-robin.rosenberg@dewire.com","threadId":"18198","inReplyTo":"20090307224831.GS16213@spearce.org","subject":"[EGIT PATCH] Prevent an exception if the user tries to push a non-existing ref.","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2009-03-08T15:21:07Z","receivedAt":"2009-03-08T15:21:07Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"Instead of a StringIndexOutOfBoundsException we now get an error telling\nus that the ref could not be resolved.\n\nSigned-off-by: Robin Rosenberg \n---\n .../src/org/spearce/jgit/transport/Transport.java | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java b/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java\nindex a0a2575..8a25213 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java\n@@ -255,7 +255,7 @@ else if (TransportLocal.canHandle(remote))\n \t\t\t} else {\n \t\t\t\tif (!remoteName.startsWith(Constants.R_REFS)) {\n \t\t\t\t\t// null source is another special case (delete)\n-\t\t\t\t\tif (srcRef != null) {\n+\t\t\t\t\tif (src != null) {\n \t\t\t\t\t\t// assume the same type of ref at the destination\n \t\t\t\t\t\tString srcPrefix = srcRef.substring(0, srcRef.indexOf('/', Constants.R_REFS.length()));\n \t\t\t\t\t\tremoteName = srcPrefix + \"/\" + remoteName;\n-- \n1.6.1.285.g35d8b\n"},{"id":"107468","messageId":"20090309155049.GE11989@spearce.org","threadId":"18198","inReplyTo":"1236525667-852-1-git-send-email-robin.rosenberg@dewire.com","subject":"Re: [EGIT PATCH] Prevent an exception if the user tries to push a non-existing ref.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-03-09T15:50:49Z","receivedAt":"2009-03-09T15:50:49Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Robin Rosenberg wrote:\n> Instead of a StringIndexOutOfBoundsException we now get an error telling\n> us that the ref could not be resolved.\n\n*sigh*\n\n> diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java b/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java\n> index a0a2575..8a25213 100644\n> --- a/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java\n> +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java\n> @@ -255,7 +255,7 @@ else if (TransportLocal.canHandle(remote))\n> \t\t\t} else {\n> \t\t\t\tif (!remoteName.startsWith(Constants.R_REFS)) {\n> \t\t\t\t\t// null source is another special case (delete)\n> -\t\t\t\t\tif (srcRef != null) {\n> +\t\t\t\t\tif (src != null) {\n> \t\t\t\t\t\t// assume the same type of ref at the destination\n> \t\t\t\t\t\tString srcPrefix = srcRef.substring(0, srcRef.indexOf('/', Constants.R_REFS.length()));\n> \t\t\t\t\t\tremoteName = srcPrefix + \"/\" + remoteName;\n\nAfter reading that code again, I'm tempted to apply this instead.\nIts a much larger patch, but I think the result is a lot easier\nto follow.\n\n--8<--\nFix DWIMery for push to handle non-existant source refs\n\nInstead of a StringIndexOutOfBoundsException we now get an error\ntelling us that the ref could not be resolved.\n\nFound-by: Robin Rosenberg \nSigned-off-by: Shawn O. Pearce \n---\n .../src/org/spearce/jgit/transport/Transport.java | 45 ++++++++++---------\n 1 files changed, 24 insertions(+), 21 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java b/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java\nindex a0a2575..1068f50 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java\n@@ -244,29 +244,32 @@ else if (TransportLocal.canHandle(remote))\n \t\tfinal Collection procRefs = expandPushWildcardsFor(db, specs);\n \n \t\tfor (final RefSpec spec : procRefs) {\n-\t\t\tString srcRef = spec.getSource();\n-\t\t\tfinal Ref src = db.getRef(srcRef);\n-\t\t\tif (src != null)\n-\t\t\t\tsrcRef = src.getName();\n-\t\t\tString remoteName = spec.getDestination();\n-\t\t\t// null destination (no-colon in ref-spec) is a special case\n-\t\t\tif (remoteName == null) {\n-\t\t\t\tremoteName = srcRef;\n-\t\t\t} else {\n-\t\t\t\tif (!remoteName.startsWith(Constants.R_REFS)) {\n-\t\t\t\t\t// null source is another special case (delete)\n-\t\t\t\t\tif (srcRef != null) {\n-\t\t\t\t\t\t// assume the same type of ref at the destination\n-\t\t\t\t\t\tString srcPrefix = srcRef.substring(0, srcRef.indexOf('/', Constants.R_REFS.length()));\n-\t\t\t\t\t\tremoteName = srcPrefix + \"/\" + remoteName;\n-\t\t\t\t\t}\n-\t\t\t\t}\n+\t\t\tString srcSpec = spec.getSource();\n+\t\t\tfinal Ref srcRef = db.getRef(srcSpec);\n+\t\t\tif (srcRef != null)\n+\t\t\t\tsrcSpec = srcRef.getName();\n+\n+\t\t\tString destSpec = spec.getDestination();\n+\t\t\tif (destSpec == null) {\n+\t\t\t\t// No destination (no-colon in ref-spec), DWIMery assumes src\n+\t\t\t\t//\n+\t\t\t\tdestSpec = srcSpec;\n \t\t\t}\n-\t\t\tfinal boolean forceUpdate = spec.isForceUpdate();\n-\t\t\tfinal String localName = findTrackingRefName(remoteName, fetchSpecs);\n \n-\t\t\tfinal RemoteRefUpdate rru = new RemoteRefUpdate(db, srcRef,\n-\t\t\t\t\tremoteName, forceUpdate, localName, null);\n+\t\t\tif (srcRef != null && !destSpec.startsWith(Constants.R_REFS)) {\n+\t\t\t\t// Assume the same kind of ref at the destination, e.g.\n+\t\t\t\t// \"refs/heads/foo:master\", DWIMery assumes master is also\n+\t\t\t\t// under \"refs/heads/\".\n+\t\t\t\t//\n+\t\t\t\tfinal String n = srcRef.getName();\n+\t\t\t\tfinal int kindEnd = n.indexOf('/', Constants.R_REFS.length());\n+\t\t\t\tdestSpec = n.substring(0, kindEnd + 1) + destSpec;\n+\t\t\t}\n+\n+\t\t\tfinal boolean forceUpdate = spec.isForceUpdate();\n+\t\t\tfinal String localName = findTrackingRefName(destSpec, fetchSpecs);\n+\t\t\tfinal RemoteRefUpdate rru = new RemoteRefUpdate(db, srcSpec,\n+\t\t\t\t\tdestSpec, forceUpdate, localName, null);\n \t\t\tresult.add(rru);\n \t\t}\n \t\treturn result;\n-- \n1.6.2.185.g8b635\n\n\n-- \nShawn.\n"},{"id":"107507","messageId":"200903092229.01289.robin.rosenberg.lists@dewire.com","threadId":"18198","inReplyTo":"20090309155049.GE11989@spearce.org","subject":"Re: [EGIT PATCH] Prevent an exception if the user tries to push a non-existing ref.","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg.lists@dewire.com","sentAt":"2009-03-09T21:29:00Z","receivedAt":"2009-03-09T21:29:00Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"måndag 09 mars 2009 16:50:49 skrev \"Shawn O. Pearce\" :\n> After reading that code again, I'm tempted to apply this instead.\n> Its a much larger patch, but I think the result is a lot easier\n> to follow.\n\nI wouldn't say \"a lot\", but a little perhaps.\n\n-- robin\n"}]}