{"thread":{"id":"19880","subject":"[JGIT PATCH 1/2] Cleanup Transport.applyConfig to use setter methods more consistently","startedAt":"2009-06-21T01:21:55Z","lastAt":"2009-06-23T16:50:16Z","messageCount":4,"participants":["Shawn O. Pearce","Robin Rosenberg"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"116686","messageId":"1245547316-10299-1-git-send-email-spearce@spearce.org","threadId":"19880","inReplyTo":null,"subject":"[JGIT PATCH 1/2] Cleanup Transport.applyConfig to use setter methods more consistently","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-06-21T01:21:55Z","receivedAt":"2009-06-21T01:21:55Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"This just struck me as odd that we sometimes used a setter method,\nand sometimes used direct assignment to the field. Now we use the\nsetter method if it is available. The fetch and push refspecs do\nnot have setters, so must still be done by direct field assignment.\n\nSigned-off-by: Shawn O. Pearce \n---\n .../src/org/spearce/jgit/transport/Transport.java | 4 ++--\n 1 files changed, 2 insertions(+), 2 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 1068f50..c36ccdd 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@@ -568,9 +568,9 @@ public void setRemoveDeletedRefs(final boolean remove) {\n \t */\n \tpublic void applyConfig(final RemoteConfig cfg) {\n \t\tsetOptionUploadPack(cfg.getUploadPack());\n-\t\tfetch = cfg.getFetchRefSpecs();\n+\t\tsetOptionReceivePack(cfg.getReceivePack());\n \t\tsetTagOpt(cfg.getTagOpt());\n-\t\toptionReceivePack = cfg.getReceivePack();\n+\t\tfetch = cfg.getFetchRefSpecs();\n \t\tpush = cfg.getPushRefSpecs();\n \t}\n \n-- \n1.6.3.2.416.g04d0\n"},{"id":"116687","messageId":"1245547316-10299-2-git-send-email-spearce@spearce.org","threadId":"19880","inReplyTo":"1245547316-10299-1-git-send-email-spearce@spearce.org","subject":"[JGIT PATCH 2/2] Add support for remote.name.pushurl","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-06-21T01:21:56Z","receivedAt":"2009-06-21T01:21:56Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"In C Git commit 203462347fce Michael J Gruber added support for a\nnew URL key within a remote block, permitting a different URL to\nbe used for push than for fetch. In the commit message he cites\nan example where fetch runs over git://, but push uses ssh://,\nas the git:// protocol has lower connection setup overheads.\n\nThis change complicates the Transport API as now we must know\nin advance when the Transport.open() call is made what type of\noperation the caller wants to perform, so we know which config\nkey to honor when constructing the Transport objects.\n\nSigned-off-by: Shawn O. Pearce \n---\n .../src/org/spearce/jgit/pgm/Push.java | 3 +-\n .../org/spearce/jgit/transport/RemoteConfig.java | 47 ++++++\n .../src/org/spearce/jgit/transport/Transport.java | 150 ++++++++++++++++++--\n 3 files changed, 190 insertions(+), 10 deletions(-)\n\ndiff --git a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Push.java b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Push.java\nindex 19d31a1..9364e4a 100644\n--- a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Push.java\n+++ b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Push.java\n@@ -102,7 +102,8 @@ protected void run() throws Exception {\n \t\t\t\trefSpecs.add(spec.setForceUpdate(true));\n \t\t}\n \n-\t\tfinal List transports = Transport.openAll(db, remote);\n+\t\tfinal List transports;\n+\t\ttransports = Transport.openAll(db, remote, Transport.Operation.PUSH);\n \t\tfor (final Transport transport : transports) {\n \t\t\ttransport.setPushThin(thin);\n \t\t\tif (receivePack != null)\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteConfig.java b/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteConfig.java\nindex 519a8a5..f7ed2d7 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteConfig.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteConfig.java\n@@ -58,6 +58,8 @@\n \n \tprivate static final String KEY_URL = \"url\";\n \n+\tprivate static final String KEY_PUSHURL = \"pushurl\";\n+\n \tprivate static final String KEY_FETCH = \"fetch\";\n \n \tprivate static final String KEY_PUSH = \"push\";\n@@ -108,6 +110,8 @@\n \n \tprivate List uris;\n \n+\tprivate List pushURIs;\n+\n \tprivate List fetch;\n \n \tprivate List push;\n@@ -147,6 +151,11 @@ public RemoteConfig(final RepositoryConfig rc, final String remoteName)\n \t\tfor (final String s : vlst)\n \t\t\turis.add(new URIish(s));\n \n+\t\tvlst = rc.getStringList(SECTION, name, KEY_PUSHURL);\n+\t\tpushURIs = new ArrayList(vlst.length);\n+\t\tfor (final String s : vlst)\n+\t\t\tpushURIs.add(new URIish(s));\n+\n \t\tvlst = rc.getStringList(SECTION, name, KEY_FETCH);\n \t\tfetch = new ArrayList(vlst.length);\n \t\tfor (final String s : vlst)\n@@ -187,6 +196,11 @@ public void update(final RepositoryConfig rc) {\n \t\trc.setStringList(SECTION, getName(), KEY_URL, vlst);\n \n \t\tvlst.clear();\n+\t\tfor (final URIish u : getPushURIs())\n+\t\t\tvlst.add(u.toPrivateString());\n+\t\trc.setStringList(SECTION, getName(), KEY_PUSHURL, vlst);\n+\n+\t\tvlst.clear();\n \t\tfor (final RefSpec u : getFetchRefSpecs())\n \t\t\tvlst.add(u.toString());\n \t\trc.setStringList(SECTION, getName(), KEY_FETCH, vlst);\n@@ -265,6 +279,39 @@ public boolean removeURI(final URIish toRemove) {\n \t}\n \n \t/**\n+\t * Get all configured push-only URIs under this remote.\n+\t * \n+\t * @return the set of URIs known to this remote.\n+\t */\n+\tpublic List getPushURIs() {\n+\t\treturn Collections.unmodifiableList(pushURIs);\n+\t}\n+\n+\t/**\n+\t * Add a new push-only URI to the end of the list of URIs.\n+\t * \n+\t * @param toAdd\n+\t * the new URI to add to this remote.\n+\t * @return true if the URI was added; false if it already exists.\n+\t */\n+\tpublic boolean addPushURI(final URIish toAdd) {\n+\t\tif (pushURIs.contains(toAdd))\n+\t\t\treturn false;\n+\t\treturn pushURIs.add(toAdd);\n+\t}\n+\n+\t/**\n+\t * Remove a push-only URI from the list of URIs.\n+\t * \n+\t * @param toRemove\n+\t * the URI to remove from this remote.\n+\t * @return true if the URI was added; false if it already exists.\n+\t */\n+\tpublic boolean removePushURI(final URIish toRemove) {\n+\t\treturn pushURIs.remove(toRemove);\n+\t}\n+\n+\t/**\n \t * Remembered specifications for fetching from a repository.\n \t * \n \t * @return set of specs used by default when fetching.\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 c36ccdd..ac4807f 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@@ -72,8 +72,18 @@\n * Callers must ensure a transport is accessed by only one thread at a time.\n */\n public abstract class Transport {\n+\t/** Type of operation a Transport is being opened for. */\n+\tpublic enum Operation {\n+\t\t/** Transport is to fetch objects locally. */\n+\t\tFETCH,\n+\t\t/** Transport is to push objects remotely. */\n+\t\tPUSH;\n+\t}\n+\n \t/**\n \t * Open a new transport instance to connect two repositories.\n+\t *

\n+\t * This method assumes {@link Operation#FETCH}.\n \t * \n \t * @param local\n \t * existing local repository.\n@@ -90,15 +100,41 @@\n \t */\n \tpublic static Transport open(final Repository local, final String remote)\n \t\t\tthrows NotSupportedException, URISyntaxException {\n+\t\treturn open(local, remote, Operation.FETCH);\n+\t}\n+\n+\t/**\n+\t * Open a new transport instance to connect two repositories.\n+\t * \n+\t * @param local\n+\t * existing local repository.\n+\t * @param remote\n+\t * location of the remote repository - may be URI or remote\n+\t * configuration name.\n+\t * @param op\n+\t * planned use of the returned Transport; the URI may differ\n+\t * based on the type of connection desired.\n+\t * @return the new transport instance. Never null. In case of multiple URIs\n+\t * in remote configuration, only the first is chosen.\n+\t * @throws URISyntaxException\n+\t * the location is not a remote defined in the configuration\n+\t * file and is not a well-formed URL.\n+\t * @throws NotSupportedException\n+\t * the protocol specified is not supported.\n+\t */\n+\tpublic static Transport open(final Repository local, final String remote,\n+\t\t\tfinal Operation op) throws NotSupportedException,\n+\t\t\tURISyntaxException {\n \t\tfinal RemoteConfig cfg = new RemoteConfig(local.getConfig(), remote);\n-\t\tfinal List uris = cfg.getURIs();\n-\t\tif (uris.size() == 0)\n+\t\tif (doesNotExist(cfg))\n \t\t\treturn open(local, new URIish(remote));\n-\t\treturn open(local, cfg);\n+\t\treturn open(local, cfg, op);\n \t}\n \n \t/**\n \t * Open new transport instances to connect two repositories.\n+\t *

\n+\t * This method assumes {@link Operation#FETCH}.\n \t *\n \t * @param local\n \t * existing local repository.\n@@ -116,18 +152,44 @@ public static Transport open(final Repository local, final String remote)\n \tpublic static List openAll(final Repository local,\n \t\t\tfinal String remote) throws NotSupportedException,\n \t\t\tURISyntaxException {\n+\t\treturn openAll(local, remote, Operation.FETCH);\n+\t}\n+\n+\t/**\n+\t * Open new transport instances to connect two repositories.\n+\t *\n+\t * @param local\n+\t * existing local repository.\n+\t * @param remote\n+\t * location of the remote repository - may be URI or remote\n+\t * configuration name.\n+\t * @param op\n+\t * planned use of the returned Transport; the URI may differ\n+\t * based on the type of connection desired.\n+\t * @return the list of new transport instances for every URI in remote\n+\t * configuration.\n+\t * @throws URISyntaxException\n+\t * the location is not a remote defined in the configuration\n+\t * file and is not a well-formed URL.\n+\t * @throws NotSupportedException\n+\t * the protocol specified is not supported.\n+\t */\n+\tpublic static List openAll(final Repository local,\n+\t\t\tfinal String remote, final Operation op)\n+\t\t\tthrows NotSupportedException, URISyntaxException {\n \t\tfinal RemoteConfig cfg = new RemoteConfig(local.getConfig(), remote);\n-\t\tfinal List uris = cfg.getURIs();\n-\t\tif (uris.size() == 0) {\n+\t\tif (doesNotExist(cfg)) {\n \t\t\tfinal ArrayList transports = new ArrayList(1);\n \t\t\ttransports.add(open(local, new URIish(remote)));\n \t\t\treturn transports;\n \t\t}\n-\t\treturn openAll(local, cfg);\n+\t\treturn openAll(local, cfg, op);\n \t}\n \n \t/**\n \t * Open a new transport instance to connect two repositories.\n+\t *

\n+\t * This method assumes {@link Operation#FETCH}.\n \t * \n \t * @param local\n \t * existing local repository.\n@@ -144,17 +206,45 @@ public static Transport open(final Repository local, final String remote)\n \t */\n \tpublic static Transport open(final Repository local, final RemoteConfig cfg)\n \t\t\tthrows NotSupportedException {\n-\t\tif (cfg.getURIs().isEmpty())\n+\t\treturn open(local, cfg, Operation.FETCH);\n+\t}\n+\n+\t/**\n+\t * Open a new transport instance to connect two repositories.\n+\t * \n+\t * @param local\n+\t * existing local repository.\n+\t * @param cfg\n+\t * configuration describing how to connect to the remote\n+\t * repository.\n+\t * @param op\n+\t * planned use of the returned Transport; the URI may differ\n+\t * based on the type of connection desired.\n+\t * @return the new transport instance. Never null. In case of multiple URIs\n+\t * in remote configuration, only the first is chosen.\n+\t * @throws NotSupportedException\n+\t * the protocol specified is not supported.\n+\t * @throws IllegalArgumentException\n+\t * if provided remote configuration doesn't have any URI\n+\t * associated.\n+\t */\n+\tpublic static Transport open(final Repository local,\n+\t\t\tfinal RemoteConfig cfg, final Operation op)\n+\t\t\tthrows NotSupportedException {\n+\t\tfinal List uris = getURIs(cfg, op);\n+\t\tif (uris.isEmpty())\n \t\t\tthrow new IllegalArgumentException(\n \t\t\t\t\t\"Remote config \\\"\"\n \t\t\t\t\t+ cfg.getName() + \"\\\" has no URIs associated\");\n-\t\tfinal Transport tn = open(local, cfg.getURIs().get(0));\n+\t\tfinal Transport tn = open(local, uris.get(0));\n \t\ttn.applyConfig(cfg);\n \t\treturn tn;\n \t}\n \n \t/**\n \t * Open new transport instances to connect two repositories.\n+\t *

\n+\t * This method assumes {@link Operation#FETCH}.\n \t *\n \t * @param local\n \t * existing local repository.\n@@ -168,7 +258,29 @@ public static Transport open(final Repository local, final RemoteConfig cfg)\n \t */\n \tpublic static List openAll(final Repository local,\n \t\t\tfinal RemoteConfig cfg) throws NotSupportedException {\n-\t\tfinal List uris = cfg.getURIs();\n+\t\treturn openAll(local, cfg, Operation.FETCH);\n+\t}\n+\n+\t/**\n+\t * Open new transport instances to connect two repositories.\n+\t *\n+\t * @param local\n+\t * existing local repository.\n+\t * @param cfg\n+\t * configuration describing how to connect to the remote\n+\t * repository.\n+\t * @param op\n+\t * planned use of the returned Transport; the URI may differ\n+\t * based on the type of connection desired.\n+\t * @return the list of new transport instances for every URI in remote\n+\t * configuration.\n+\t * @throws NotSupportedException\n+\t * the protocol specified is not supported.\n+\t */\n+\tpublic static List openAll(final Repository local,\n+\t\t\tfinal RemoteConfig cfg, final Operation op)\n+\t\t\tthrows NotSupportedException {\n+\t\tfinal List uris = getURIs(cfg, op);\n \t\tfinal List transports = new ArrayList(uris.size());\n \t\tfor (final URIish uri : uris) {\n \t\t\tfinal Transport tn = open(local, uri);\n@@ -178,6 +290,26 @@ public static Transport open(final Repository local, final RemoteConfig cfg)\n \t\treturn transports;\n \t}\n \n+\tprivate static List getURIs(final RemoteConfig cfg,\n+\t\t\tfinal Operation op) {\n+\t\tswitch (op) {\n+\t\tcase FETCH:\n+\t\t\treturn cfg.getURIs();\n+\t\tcase PUSH: {\n+\t\t\tList uris = cfg.getPushURIs();\n+\t\t\tif (uris.isEmpty())\n+\t\t\t\turis = cfg.getURIs();\n+\t\t\treturn uris;\n+\t\t}\n+\t\tdefault:\n+\t\t\tthrow new IllegalArgumentException(op.toString());\n+\t\t}\n+\t}\n+\n+\tprivate static boolean doesNotExist(final RemoteConfig cfg) {\n+\t\treturn cfg.getURIs().isEmpty() && cfg.getPushURIs().isEmpty();\n+\t}\n+\n \t/**\n \t * Open a new transport instance to connect two repositories.\n \t * \n-- \n1.6.3.2.416.g04d0\n"},{"id":"116715","messageId":"200906211930.43809.robin.rosenberg@dewire.com","threadId":"19880","inReplyTo":"1245547316-10299-2-git-send-email-spearce@spearce.org","subject":"Re: [JGIT PATCH 2/2] Add support for remote.name.pushurl","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2009-06-21T17:30:43Z","receivedAt":"2009-06-21T17:30:43Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"söndag 21 juni 2009 03:21:56 skrev \"Shawn O. Pearce\" :\n> In C Git commit 203462347fce Michael J Gruber added support for a\n> new URL key within a remote block, permitting a different URL to\n> be used for push than for fetch. In the commit message he cites\n> an example where fetch runs over git://, but push uses ssh://,\n> as the git:// protocol has lower connection setup overheads.\n> \n> This change complicates the Transport API as now we must know\n> in advance when the Transport.open() call is made what type of\n> operation the caller wants to perform, so we know which config\n> key to honor when constructing the Transport objects.\n> \n> Signed-off-by: Shawn O. Pearce \n> ---\n> .../src/org/spearce/jgit/pgm/Push.java | 3 +-\n> .../org/spearce/jgit/transport/RemoteConfig.java | 47 ++++++\n> .../src/org/spearce/jgit/transport/Transport.java | 150 ++++++++++++++++++--\n> 3 files changed, 190 insertions(+), 10 deletions(-)\n> \n> diff --git a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Push.java b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Push.java\n> index 19d31a1..9364e4a 100644\n> --- a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Push.java\n> +++ b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Push.java\n> @@ -102,7 +102,8 @@ protected void run() throws Exception {\n> \t\t\t\trefSpecs.add(spec.setForceUpdate(true));\n> \t\t}\n> \n> -\t\tfinal List transports = Transport.openAll(db, remote);\n> +\t\tfinal List transports;\n> +\t\ttransports = Transport.openAll(db, remote, Transport.Operation.PUSH);\n\nNit-pick. We usually initialize in one statement. I'll squash that for you.\n\nSeems we need to consider the Eclipse UI, since that has only of only one UI. Can we hold on to\nthat or at least establish a related issue in the bug tracker for Egit.\n\n-- robin\n"},{"id":"116827","messageId":"20090623165016.GD11191@spearce.org","threadId":"19880","inReplyTo":"200906211930.43809.robin.rosenberg@dewire.com","subject":"Re: [JGIT PATCH 2/2] Add support for remote.name.pushurl","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-06-23T16:50:16Z","receivedAt":"2009-06-23T16:50:16Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Robin Rosenberg wrote:\n> s?ndag 21 juni 2009 03:21:56 skrev \"Shawn O. Pearce\" :\n> > diff --git a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Push.java b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Push.java\n> > index 19d31a1..9364e4a 100644\n> > --- a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Push.java\n> > +++ b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/Push.java\n> > @@ -102,7 +102,8 @@ protected void run() throws Exception {\n> > \t\t\t\trefSpecs.add(spec.setForceUpdate(true));\n> > \t\t}\n> > \n> > -\t\tfinal List transports = Transport.openAll(db, remote);\n> > +\t\tfinal List transports;\n> > +\t\ttransports = Transport.openAll(db, remote, Transport.Operation.PUSH);\n> \n> Nit-pick. We usually initialize in one statement. I'll squash that for you.\n\nActually I did that because otherwise we'd run over 80 columns on\nthat line. If you let Eclipse format the code for you, it line\nwraps onto a 2nd line anyway. In such cases I personally just find\nthis format more readable. But whatever.\n \n-- \nShawn.\n"}]}