Do not apply pushInsteadOf to existing pushUris
Per the git config documentation[1], pushInsteadOf is ignored when
a remote has explicit pushUris.
Implement this, and adapt tests.
Up to now JGit mistakenly applied pushInsteadOf also to existing
pushUris. If some repositories had relied on this mis-feature,
pushes may newly suddenly fail (the uncritical case; the config
just needs to be fixed) or even still succeed but push to unexpected
places, namely to the non-rewritten pushUrls (the critical case).
The release notes should point out this change.
[1] https://git-scm.com/docs/git-config
Bug: 393170
Change-Id: I38c83204d2ac74f88f3d22d0550bf5ff7ee86daf
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/RemoteConfigTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/RemoteConfigTest.java
index b64c134..a0cf0d2 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/RemoteConfigTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/RemoteConfigTest.java
@@ -499,19 +499,30 @@
}
@Test
- public void singlePushInsteadOf() throws Exception {
+ public void pushInsteadOfNotAppliedToPushUri() throws Exception {
config.setString("remote", "origin", "pushurl", "short:project.git");
config.setString("url", "https://server/repos/", "pushInsteadOf",
"short:");
RemoteConfig rc = new RemoteConfig(config, "origin");
assertFalse(rc.getPushURIs().isEmpty());
- assertEquals("https://server/repos/project.git", rc.getPushURIs()
- .get(0).toASCIIString());
+ assertEquals("short:project.git",
+ rc.getPushURIs().get(0).toASCIIString());
+ }
+
+ @Test
+ public void pushInsteadOfAppliedToUri() throws Exception {
+ config.setString("remote", "origin", "url", "short:project.git");
+ config.setString("url", "https://server/repos/", "pushInsteadOf",
+ "short:");
+ RemoteConfig rc = new RemoteConfig(config, "origin");
+ assertFalse(rc.getPushURIs().isEmpty());
+ assertEquals("https://server/repos/project.git",
+ rc.getPushURIs().get(0).toASCIIString());
}
@Test
public void multiplePushInsteadOf() throws Exception {
- config.setString("remote", "origin", "pushurl", "prefixproject.git");
+ config.setString("remote", "origin", "url", "prefixproject.git");
config.setStringList("url", "https://server/repos/", "pushInsteadOf",
Arrays.asList("pre", "prefix", "pref", "perf"));
RemoteConfig rc = new RemoteConfig(config, "origin");
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/RemoteConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/RemoteConfig.java
index f192be1..a0d81c0 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/RemoteConfig.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/RemoteConfig.java
@@ -173,20 +173,22 @@
for (final String s : vlst) {
uris.add(new URIish(replaceUri(s, insteadOf)));
}
- Map<String, String> pushInsteadOf = getReplacements(rc,
- KEY_PUSHINSTEADOF);
String[] plst = rc.getStringList(SECTION, name, KEY_PUSHURL);
pushURIs = new ArrayList<>(plst.length);
for (final String s : plst) {
- pushURIs.add(new URIish(replaceUri(s, pushInsteadOf)));
+ pushURIs.add(new URIish(s));
}
- if (pushURIs.isEmpty() && !pushInsteadOf.isEmpty()) {
+ if (pushURIs.isEmpty()) {
// Would default to the uris. If we have pushinsteadof, we must
// supply rewritten push uris.
- for (String s : vlst) {
- String replaced = replaceUri(s, pushInsteadOf);
- if (!s.equals(replaced)) {
- pushURIs.add(new URIish(replaced));
+ Map<String, String> pushInsteadOf = getReplacements(rc,
+ KEY_PUSHINSTEADOF);
+ if (!pushInsteadOf.isEmpty()) {
+ for (String s : vlst) {
+ String replaced = replaceUri(s, pushInsteadOf);
+ if (!s.equals(replaced)) {
+ pushURIs.add(new URIish(replaced));
+ }
}
}
}
