From f5264cfabfb2f921304536f4471bff469d9ba546 Mon Sep 17 00:00:00 2001 From: Matthias Sohn Date: Sat, 19 Aug 2017 23:45:00 +0200 Subject: [PATCH] Remove repository from ListRemoteOperation LsRemoteCommand can run ls-remote without a repository hence remove the unnecessary repository from ListRemoteOperation. SourceBranchPage created an unnecessary temporary repository in /tmp in order to run ls-remote before cloning a repository. This may fail if the user does not have write permissions in this folder. Bug: 521033 Change-Id: Id39107266551a3b7a9251cf9d26233f9bbfdc516 Signed-off-by: Matthias Sohn --- .../egit/core/test/op/ListRemoteOperationTest.java | 11 ++++------- .../org/eclipse/egit/core/op/ListRemoteOperation.java | 14 +++----------- .../eclipse/egit/gitflow/op/FeatureListOperation.java | 2 +- .../eclipse/egit/ui/common/LocalRepositoryTestCase.java | 3 +-- .../eclipse/egit/ui/internal/clone/SourceBranchPage.java | 11 +---------- .../ui/internal/components/RefContentAssistProvider.java | 3 +-- .../eclipse/egit/ui/internal/components/RefSpecPage.java | 2 +- .../egit/ui/internal/fetch/FetchGerritChangePage.java | 16 ++++++---------- .../eclipse/egit/ui/internal/fetch/FetchSourcePage.java | 4 ++-- 9 files changed, 20 insertions(+), 46 deletions(-) diff --git a/org.eclipse.egit.core.test/src/org/eclipse/egit/core/test/op/ListRemoteOperationTest.java b/org.eclipse.egit.core.test/src/org/eclipse/egit/core/test/op/ListRemoteOperationTest.java index ad3d5721e..58a538dcc 100644 --- a/org.eclipse.egit.core.test/src/org/eclipse/egit/core/test/op/ListRemoteOperationTest.java +++ b/org.eclipse.egit.core.test/src/org/eclipse/egit/core/test/op/ListRemoteOperationTest.java @@ -103,15 +103,14 @@ public class ListRemoteOperationTest extends DualRepositoryTestCase { URIish uri = new URIish("file:///" + repository2.getRepository().getDirectory().getPath()); - ListRemoteOperation lrop = new ListRemoteOperation(repository1 - .getRepository(), uri, 0); + ListRemoteOperation lrop = new ListRemoteOperation(uri, 0); lrop.run(null); assertEquals(4, lrop.getRemoteRefs().size()); assertNotNull(lrop.getRemoteRef("refs/heads/test")); uri = new URIish("file:///" + repository1.getRepository().getDirectory().getPath()); - lrop = new ListRemoteOperation(repository2.getRepository(), uri, 0); + lrop = new ListRemoteOperation(uri, 0); lrop.run(new NullProgressMonitor()); assertEquals(2, lrop.getRemoteRefs().size()); assertNotNull(lrop.getRemoteRef("refs/heads/master")); @@ -127,8 +126,7 @@ public class ListRemoteOperationTest extends DualRepositoryTestCase { URIish uri = new URIish("file:///" + repository2.getRepository().getDirectory().getPath()); - ListRemoteOperation lrop = new ListRemoteOperation(repository1 - .getRepository(), uri, 0); + ListRemoteOperation lrop = new ListRemoteOperation(uri, 0); try { lrop.getRemoteRefs(); fail("Expected Exception not thrown"); @@ -146,8 +144,7 @@ public class ListRemoteOperationTest extends DualRepositoryTestCase { public void testIllegalURI() throws Exception { URIish uri = new URIish("file:///" + "no/path"); - ListRemoteOperation lrop = new ListRemoteOperation(repository1 - .getRepository(), uri, 0); + ListRemoteOperation lrop = new ListRemoteOperation(uri, 0); try { lrop.run(new NullProgressMonitor()); fail("Expected Exception not thrown"); diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/op/ListRemoteOperation.java b/org.eclipse.egit.core/src/org/eclipse/egit/core/op/ListRemoteOperation.java index a53c30dba..d7745f320 100644 --- a/org.eclipse.egit.core/src/org/eclipse/egit/core/op/ListRemoteOperation.java +++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/op/ListRemoteOperation.java @@ -19,7 +19,6 @@ import org.eclipse.jgit.api.LsRemoteCommand; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.api.errors.JGitInternalException; import org.eclipse.jgit.lib.Ref; -import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.transport.CredentialsProvider; import org.eclipse.jgit.transport.URIish; @@ -34,21 +33,14 @@ public class ListRemoteOperation { /** * Create listing operation for specified local repository (needed by * transport) and remote repository URI. - * - * @param localDb - * local repository (needed for transport) where fetch would - * occur. * @param uri * URI of remote repository to list. * @param timeout * timeout is seconds; 0 means no timeout */ - public ListRemoteOperation(final Repository localDb, final URIish uri, - int timeout) { - try (Git git = new Git(localDb)) { - rc = git.lsRemote(); - rc.setRemote(uri.toString()).setTimeout(timeout); - } + public ListRemoteOperation(final URIish uri, int timeout) { + rc = Git.lsRemoteRepository(); + rc.setRemote(uri.toString()).setTimeout(timeout); } /** diff --git a/org.eclipse.egit.gitflow/src/org/eclipse/egit/gitflow/op/FeatureListOperation.java b/org.eclipse.egit.gitflow/src/org/eclipse/egit/gitflow/op/FeatureListOperation.java index d858a3bab..714839073 100644 --- a/org.eclipse.egit.gitflow/src/org/eclipse/egit/gitflow/op/FeatureListOperation.java +++ b/org.eclipse.egit.gitflow/src/org/eclipse/egit/gitflow/op/FeatureListOperation.java @@ -64,7 +64,7 @@ public final class FeatureListOperation extends GitFlowOperation { URIish uri = new URIish(uriString); ListRemoteOperation listRemoteOperation = new ListRemoteOperation( - repository.getRepository(), uri, timeout); + uri, timeout); listRemoteOperation.run(progress.newChild(1)); Collection remoteRefs = listRemoteOperation.getRemoteRefs(); for (Ref ref : remoteRefs) { diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/LocalRepositoryTestCase.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/LocalRepositoryTestCase.java index 04713a870..035100983 100644 --- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/LocalRepositoryTestCase.java +++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/common/LocalRepositoryTestCase.java @@ -790,9 +790,8 @@ public abstract class LocalRepositoryTestCase extends EGitTestCase { } protected static Collection getRemoteRefs(URIish uri) throws Exception { - final Repository db = FileRepositoryBuilder.create(new File("/tmp")); //$NON-NLS-1$ int timeout = 20; - ListRemoteOperation listRemoteOp = new ListRemoteOperation(db, uri, + ListRemoteOperation listRemoteOp = new ListRemoteOperation(uri, timeout); listRemoteOp.run(null); return listRemoteOp.getRemoteRefs(); diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/clone/SourceBranchPage.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/clone/SourceBranchPage.java index 9d3de0883..71f9da3d4 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/clone/SourceBranchPage.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/clone/SourceBranchPage.java @@ -13,8 +13,6 @@ *******************************************************************************/ package org.eclipse.egit.ui.internal.clone; -import java.io.File; -import java.io.IOException; import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.Arrays; @@ -50,8 +48,6 @@ import org.eclipse.jgit.api.errors.TransportException; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Ref; -import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.storage.file.FileRepositoryBuilder; import org.eclipse.jgit.transport.URIish; import org.eclipse.osgi.util.NLS; import org.eclipse.swt.SWT; @@ -324,11 +320,9 @@ class SourceBranchPage extends WizardPage { final ListRemoteOperation listRemoteOp; final URIish uri = newRepoSelection.getURI(); try { - final Repository db = FileRepositoryBuilder - .create(new File("/tmp")); //$NON-NLS-1$ int timeout = Activator.getDefault().getPreferenceStore().getInt( UIPreferences.REMOTE_CONNECTION_TIMEOUT); - listRemoteOp = new ListRemoteOperation(db, uri, timeout); + listRemoteOp = new ListRemoteOperation(uri, timeout); if (credentials != null) listRemoteOp .setCredentialsProvider(new EGitCredentialsProvider( @@ -347,9 +341,6 @@ class SourceBranchPage extends WizardPage { if (showDetailedFailureDialog()) SourceBranchFailureDialog.show(getShell(), uri); return; - } catch (IOException e) { - transportError(UIText.SourceBranchPage_cannotCreateTemp); - return; } catch (InterruptedException e) { transportError(UIText.SourceBranchPage_remoteListingCancelled); return; diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/components/RefContentAssistProvider.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/components/RefContentAssistProvider.java index 06763ffc4..489f5c1c7 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/components/RefContentAssistProvider.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/components/RefContentAssistProvider.java @@ -85,8 +85,7 @@ public class RefContentAssistProvider { try { boolean local = pushMode == source; if (!local) { - final ListRemoteOperation lop = new ListRemoteOperation(repo, - uri, + final ListRemoteOperation lop = new ListRemoteOperation(uri, Activator.getDefault().getPreferenceStore().getInt( UIPreferences.REMOTE_CONNECTION_TIMEOUT)); IRunnableWithProgress runnable = new IRunnableWithProgress() { diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/components/RefSpecPage.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/components/RefSpecPage.java index a747e64e6..abd94417d 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/components/RefSpecPage.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/components/RefSpecPage.java @@ -246,7 +246,7 @@ public class RefSpecPage extends WizardPage { uri = newRepoSelection.getURI(pushPage); int timeout = Activator.getDefault().getPreferenceStore().getInt( UIPreferences.REMOTE_CONNECTION_TIMEOUT); - listRemotesOp = new ListRemoteOperation(local, uri, timeout); + listRemotesOp = new ListRemoteOperation(uri, timeout); if (credentials != null) listRemotesOp .setCredentialsProvider(new EGitCredentialsProvider( diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/fetch/FetchGerritChangePage.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/fetch/FetchGerritChangePage.java index 0df477ced..f2d928ec7 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/fetch/FetchGerritChangePage.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/fetch/FetchGerritChangePage.java @@ -258,7 +258,7 @@ public class FetchGerritChangePage extends WizardPage { if (list != null) { list.cancel(ChangeList.CancelMode.INTERRUPT); } - list = new ChangeList(repository, uriText); + list = new ChangeList(uriText); changeRefs.put(uriText, list); preFetch(list); } @@ -490,7 +490,7 @@ public class FetchGerritChangePage extends WizardPage { } for (String aUri : uris) { uriCombo.add(aUri); - changeRefs.put(aUri, new ChangeList(repository, aUri)); + changeRefs.put(aUri, new ChangeList(aUri)); } if (defaultUri != null) { uriCombo.setText(defaultUri); @@ -500,7 +500,7 @@ public class FetchGerritChangePage extends WizardPage { String currentUri = uriCombo.getText(); ChangeList list = changeRefs.get(currentUri); if (list == null) { - list = new ChangeList(repository, currentUri); + list = new ChangeList(currentUri); changeRefs.put(currentUri, list); } preFetch(list); @@ -761,7 +761,7 @@ public class FetchGerritChangePage extends WizardPage { throws InvocationTargetException, InterruptedException { String uriText = uriCombo.getText(); if (!changeRefs.containsKey(uriText)) { - changeRefs.put(uriText, new ChangeList(repository, uriText)); + changeRefs.put(uriText, new ChangeList(uriText)); } ChangeList list = changeRefs.get(uriText); if (!list.isFinished()) { @@ -1379,8 +1379,6 @@ public class FetchGerritChangePage extends WizardPage { PRISTINE, SCHEDULED, CANCELING, INTERRUPT, CANCELED, DONE } - private final Repository repository; - private final String uriText; private State state = State.PRISTINE; @@ -1389,8 +1387,7 @@ public class FetchGerritChangePage extends WizardPage { private InterruptibleJob job; - public ChangeList(Repository repository, String uriText) { - this.repository = repository; + public ChangeList(String uriText) { this.uriText = uriText; } @@ -1515,8 +1512,7 @@ public class FetchGerritChangePage extends WizardPage { } ListRemoteOperation listOp; try { - listOp = new ListRemoteOperation(repository, - new URIish(uriText), + listOp = new ListRemoteOperation(new URIish(uriText), Activator.getDefault().getPreferenceStore().getInt( UIPreferences.REMOTE_CONNECTION_TIMEOUT)); } catch (URISyntaxException e) { diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/fetch/FetchSourcePage.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/fetch/FetchSourcePage.java index f1ff6c2ce..744082e8e 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/fetch/FetchSourcePage.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/fetch/FetchSourcePage.java @@ -132,8 +132,8 @@ public class FetchSourcePage extends WizardPage { URIish uriToCheck; List proposals = new ArrayList<>(); uriToCheck = config.getURIs().get(0); - final ListRemoteOperation lop = new ListRemoteOperation(repository, - uriToCheck, Activator.getDefault().getPreferenceStore() + final ListRemoteOperation lop = new ListRemoteOperation(uriToCheck, + Activator.getDefault().getPreferenceStore() .getInt(UIPreferences.REMOTE_CONNECTION_TIMEOUT)); try { new ProgressMonitorDialog(getShell()).run(true, true, -- 2.11.4.GIT