Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 370300 - Provide extension point for EGit CloneSourceProvider
Summary: Provide extension point for EGit CloneSourceProvider
Status: RESOLVED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: Mylyn (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: 0.9   Edit
Assignee: Sascha Scholz CLA
QA Contact:
URL:
Whiteboard:
Keywords: noteworthy
Depends on: 370646
Blocks:
  Show dependency tree
 
Reported: 2012-02-01 03:01 EST by Sascha Scholz CLA
Modified: 2012-03-13 05:55 EDT (History)
1 user (show)

See Also:


Attachments
screenshot (29.32 KB, image/png)
2012-02-17 15:55 EST, Steffen Pingel CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Sascha Scholz CLA 2012-02-01 03:01:51 EST
Depends on EGit 1.3 (will be released mid of February).

Proposed change http://review.mylyn.org/#change,115 (still draft)
Comment 1 Sascha Scholz CLA 2012-02-05 11:37:53 EST
I submitted a patchset for review: http://review.mylyn.org/#change,115
Comment 2 Sascha Scholz CLA 2012-02-16 16:12:47 EST
Merged 1ce9caed3b11e2a6d4832c83ce0fe423183f951f
Comment 3 Steffen Pingel CLA 2012-02-17 10:36:04 EST
I ran into this exception when I tried it on Windows:

java.lang.NullPointerException
	at org.eclipse.mylyn.internal.gerrit.ui.egit.GerritRepositorySearchPageContentProvider.hasChildren(GerritRepositorySearchPageContentProvider.java:49)
	at org.eclipse.jface.viewers.AbstractTreeViewer.isExpandable(AbstractTreeViewer.java:2127)
	at org.eclipse.jface.viewers.TreeViewer.isExpandable(TreeViewer.java:588)
	at org.eclipse.jface.viewers.AbstractTreeViewer.isExpandable(AbstractTreeViewer.java:2153)
	at org.eclipse.jface.viewers.AbstractTreeViewer.updatePlus(AbstractTreeViewer.java:2835)
	at org.eclipse.jface.viewers.TreeViewer.updatePlus(TreeViewer.java:852)
	at org.eclipse.jface.viewers.AbstractTreeViewer.createTreeItem(AbstractTreeViewer.java:834)
	at org.eclipse.jface.viewers.AbstractTreeViewer$1.run(AbstractTreeViewer.java:808)
	at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:70)
	at org.eclipse.jface.viewers.AbstractTreeViewer.createChildren(AbstractTreeViewer.java:782)
	at org.eclipse.jface.viewers.TreeViewer.createChildren(TreeViewer.java:644)
	at org.eclipse.jface.viewers.AbstractTreeViewer.createChildren(AbstractTreeViewer.java:753)
	at org.eclipse.jface.viewers.AbstractTreeViewer.internalInitializeTree(AbstractTreeViewer.java:1533)
	at org.eclipse.jface.viewers.TreeViewer.internalInitializeTree(TreeViewer.java:833)
	at org.eclipse.jface.viewers.AbstractTreeViewer$5.run(AbstractTreeViewer.java:1517)
	at org.eclipse.jface.viewers.StructuredViewer.preservingSelection(StructuredViewer.java:1443)
	at org.eclipse.jface.viewers.TreeViewer.preservingSelection(TreeViewer.java:403)
	at org.eclipse.jface.viewers.StructuredViewer.preservingSelection(StructuredViewer.java:1404)
	at org.eclipse.jface.viewers.AbstractTreeViewer.inputChanged(AbstractTreeViewer.java:1510)
	at org.eclipse.ui.dialogs.FilteredTree$NotifyingTreeViewer.inputChanged(FilteredTree.java:1187)
	at org.eclipse.jface.viewers.ContentViewer.setInput(ContentViewer.java:280)
	at org.eclipse.jface.viewers.StructuredViewer.setInput(StructuredViewer.java:1690)
	at org.eclipse.mylyn.internal.gerrit.ui.egit.GerritRepositorySearchPage.createControl(GerritRepositorySearchPage.java:93)
	at org.eclipse.jface.wizard.WizardDialog.updateForPage(WizardDialog.java:1247)
	at org.eclipse.jface.wizard.WizardDialog.access$4(WizardDialog.java:1239)
	at org.eclipse.jface.wizard.WizardDialog$8.run(WizardDialog.java:1228)
	at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:70)
	at org.eclipse.jface.wizard.WizardDialog.showPage(WizardDialog.java:1226)
	at org.eclipse.jface.wizard.WizardDialog.nextPressed(WizardDialog.java:915)
	at org.eclipse.jface.wizard.WizardDialog.buttonPressed(WizardDialog.java:428)
	at org.eclipse.jface.dialogs.Dialog$2.widgetSelected(Dialog.java:624)
	at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:240)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1053)
	at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4163)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3752)
	at org.eclipse.jface.window.Window.runEventLoop(Window.java:825)
	at org.eclipse.jface.window.Window.open(Window.java:801)
	at org.eclipse.ui.internal.handlers.WizardHandler$Import.executeHandler(WizardHandler.java:150)
	at org.eclipse.ui.internal.handlers.WizardHandler.execute(WizardHandler.java:277)
	at org.eclipse.ui.internal.handlers.HandlerProxy.execute(HandlerProxy.java:293)
	at org.eclipse.core.commands.Command.executeWithChecks(Command.java:476)
	at org.eclipse.core.commands.ParameterizedCommand.executeWithChecks(ParameterizedCommand.java:508)
	at org.eclipse.ui.internal.handlers.HandlerService.executeCommand(HandlerService.java:169)
	at org.eclipse.ui.internal.handlers.SlaveHandlerService.executeCommand(SlaveHandlerService.java:241)
	at org.eclipse.ui.internal.actions.CommandAction.runWithEvent(CommandAction.java:157)
	at org.eclipse.jface.action.ActionContributionItem.handleWidgetSelection(ActionContributionItem.java:584)
	at org.eclipse.jface.action.ActionContributionItem.access$2(ActionContributionItem.java:501)
	at org.eclipse.jface.action.ActionContributionItem$5.handleEvent(ActionContributionItem.java:411)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1053)
	at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4163)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3752)
	at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2696)
	at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2660)
	at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2494)
	at org.eclipse.ui.internal.Workbench$7.run(Workbench.java:674)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:667)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149)
	at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:123)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:344)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:179)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
	at java.lang.reflect.Method.invoke(Unknown Source)
	at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:622)
	at org.eclipse.equinox.launcher.Main.basicRun(Main.java:577)
	at org.eclipse.equinox.launcher.Main.run(Main.java:1410)
Comment 4 Sascha Scholz CLA 2012-02-17 11:33:56 EST
This is probably due to old configuration data that doesn't have project information. We should be more tolerant.

For unknown reason I'm currently not able to push, but the fix should be to change getProjects in GerritConfiguration to

	public List<Project> getProjects() {
		if (projects != null) {
			return projects;
		} else {
			return Collections.emptyList();
		}
	}

Sascha
Comment 5 Sascha Scholz CLA 2012-02-17 14:25:12 EST
Pushed b04019c4125622f76a549bf7b963b65eebb247e1 that should fix the NPE. Could you please verify it, Steffen?
Comment 6 Steffen Pingel CLA 2012-02-17 15:53:36 EST
I published another weekly build. It works now! 

I noticed that if I select the Eclipse.org Gerrit instance two identical https:// URIs are listed for each repository. Should we filter the duplicates? 

I was also wondering if it makes sense to default the URI selection to https:// or git://. Those protocols often works for anonymous access whereas ssh:// produces quirky errors when authentication fails.
Comment 7 Steffen Pingel CLA 2012-02-17 15:55:32 EST
Created attachment 211215 [details]
screenshot
Comment 8 Sascha Scholz CLA 2012-02-17 16:28:09 EST
> I published another weekly build. It works now!

Great, thanks!

> I noticed that if I select the Eclipse.org Gerrit instance two identical
> https:// URIs are listed for each repository. Should we filter the duplicates? 

This could happen if you haven't been authenticated when you did the last refresh of the repository configuration. A solution would be to show only URLs for DownloadScheme.ANON_HTTP and DownloadScheme.ANON_GIT if not authenticated (as the Gerrit UI does).

> I was also wondering if it makes sense to default the URI selection to https://
> or git://. Those protocols often works for anonymous access whereas ssh://
> produces quirky errors when authentication fails.

For anonymous access this would be the case if the above proposal is implemented. For authenticated access I chose SSH to be on the top of the list because that's the most common protocol at SAP and my intention was to to push the Gerrit connector forward here. I don't use HTTPS for the SAP Gerrit either because I haven't managed to set up SSL certificates correctly... :-(
Comment 9 Steffen Pingel CLA 2012-02-17 18:18:01 EST
Refreshing got indeed rid of the duplicate URLs. The username is now properly added.  

Makes sense to mirror the Gerrit web UI and I'm okay with leaving ssh as the default when authenticated. The errors that I encountered were just very strange but it's a separate problem that should be addressed in EGit.
Comment 10 Sascha Scholz CLA 2012-02-22 02:36:52 EST
Pushed a fix for duplicate clone URIs: 721316a2d56a08a720bec451246def78a46b0d58
Comment 11 Steffen Pingel CLA 2012-02-28 07:32:59 EST
A few minor suggestions came up in the UI review on the last conference call:

* The label of the "Add" button should be changed to "Add..." since it opens a dialog.
* We should consider nesting repository nodes that have a "/" e.g. show an "egit" node with "egit", "egit-pde" etc. as children. On large servers such as android-review.googlesource.com the list or repositories gets very long otherwise.
* When refreshing, the entire operation fails if any repository fails to refresh (e.g. due to a network error). That can make it impossible to refresh the list of repositories.
* Gerrit servers that are marked as "Disconnected" should probably be filtered from the list.
Comment 12 Sascha Scholz CLA 2012-02-28 09:21:18 EST
Thanks for the review!

* The label of the "Add" button should be changed to "Add..." since it opens a
dialog.

Ok, I will have a look today.

* We should consider nesting repository nodes that have a "/" e.g. show an
"egit" node with "egit", "egit-pde" etc. as children. On large servers such as
android-review.googlesource.com the list or repositories gets very long
otherwise.

A very good idea, but takes a bit more time. Probably, I won't have time for that before end of next week.

* When refreshing, the entire operation fails if any repository fails to
refresh (e.g. due to a network error). That can make it impossible to refresh
the list of repositories.

The button only refreshes the currently selected repository. Seems that we have to make it clearer in the UI. Any good ideas?

* Gerrit servers that are marked as "Disconnected" should probably be filtered
from the list.

Ok, I will have a look today.
Comment 13 Steffen Pingel CLA 2012-02-28 09:30:07 EST
(In reply to comment #12)
> * When refreshing, the entire operation fails if any repository fails to
> refresh (e.g. due to a network error). That can make it impossible to refresh
> the list of repositories.
> 
> The button only refreshes the currently selected repository. Seems that we have
> to make it clearer in the UI. Any good ideas?

Great. Then this is a probably a non issue. I'll try again, maybe I always had the failing repository selected and didn't realize it.
Comment 14 Steffen Pingel CLA 2012-02-28 09:32:20 EST
(In reply to comment #13)
> (In reply to comment #12)
> > * When refreshing, the entire operation fails if any repository fails to
> > refresh (e.g. due to a network error). That can make it impossible to refresh
> > the list of repositories.
> >
> > The button only refreshes the currently selected repository. Seems that we
> have
> > to make it clearer in the UI. Any good ideas?

One simple way to make it more obvious would be to show the label of the repository in the progress message. I think it only says "Refresh Configuration" now, if it said "Refreshing Mylyn Code Reviews" this would already help.
Comment 15 Sascha Scholz CLA 2012-02-28 14:31:03 EST
Pushed 78204c1d3f35392269b0fd9dd0b90fa3f2540a92 and ed2e9617483a49139646424fc0291c8c7a82a435:

- Label: "Add" -> "Add..."
- Hide disconnected task repositories
- show repository label during refresh

Open:

* We should consider nesting repository nodes that have a "/" e.g. show an "egit" node with "egit", "egit-pde" etc. as children. On large servers such as android-review.googlesource.com the list or repositories gets very long otherwise.
Comment 16 Sascha Scholz CLA 2012-03-12 15:32:33 EDT
Don't know when I'll have time to look into making the project list hierarchical. To much other work... :-(
Comment 17 Steffen Pingel CLA 2012-03-13 05:55:09 EDT
No worries, we can just file another bug and mark it as helpwanted to encourage someone from the community to pick it up.