Comment 1 Thomas Wolf 2018-06-14 16:57:31 EDT

Note: the org.eclipse.jgit.lfs bundle _is_ included in the runtime workbench.

Comment 2 Eclipse Genie 2018-06-14 17:12:54 EDT

New Gerrit change created: https://git.eclipse.org/r/124565

Comment 3 Thomas Wolf 2018-06-14 17:14:04 EDT

(In reply to Eclipse Genie from comment #2)
> New Gerrit change created: https://git.eclipse.org/r/124565

Found it. The import got lost in the last merge stable-5.0 -> master.

Comment 4 Eclipse Genie 2018-06-14 17:34:33 EDT

Gerrit change https://git.eclipse.org/r/124565 was merged to [master].
Commit: http://git.eclipse.org/c/egit/egit.git/commit/?id=f4825dc2c92fc22c7b0c119b4619d50c8fa0f5ef

Comment 5 Matthias Sohn 2018-06-14 17:41:29 EDT

(In reply to Thomas Wolf from comment #3)
> (In reply to Eclipse Genie from comment #2)
> > New Gerrit change created: https://git.eclipse.org/r/124565
> 
> Found it. The import got lost in the last merge stable-5.0 -> master.

resolving conflicts on pom.xml and MANIFEST.MF files after merging new version to master is always painful and error-prone since there are so many of these files and only rarely there is a diff which needs to be preserved

thanks for fixing this

Comment 6 Markus Duft 2018-06-15 01:28:43 EDT

Ugh. I fixed the same thing on 5.0 as it was lost before... https://git.eclipse.org/r/#/c/124333/7/org.eclipse.egit.core/META-INF/MANIFEST.MF

How about if the code which uses reflection now to access BuiltinLFS would access it directly? This would change only a single thing: the compiler would tell us about the missing import. It would still be optional, but a more strict compile time dependency (which is IMHO OK, right?).

As a base for discussion: https://git.eclipse.org/r/#/c/124572/ - opinions?

Comment 7 Andrey Loskutov 2018-06-15 10:48:14 EDT

I'm observing this warning on every startup now: "Builtin LFS support not present/detected".

Honestly, I couldn't care less (we don't use LFS). Please can we put this warning under some EGit preference page where we would see if LFS is enabled or not, and may be a hint what to do to "enable" LFS etc?

I don't like to see *anything* in my error log which is not an error at all.

eclipse.buildId=4.9.0.I20180615-0655
java.version=1.8.0_131
java.vendor=Oracle Corporation
BootLoader constants: OS=linux, ARCH=x86_64, WS=gtk, NL=en_US
Framework arguments:  -product org.eclipse.sdk.ide
Command-line arguments:  -product org.eclipse.sdk.ide -data /data/4x_platform_workspace/../runtime-New_configuration -dev file:/data/4x_platform_workspace/.metadata/.plugins/org.eclipse.pde.core/New_configuration/dev.properties -os linux -ws gtk -arch x86_64 -consoleLog

org.eclipse.egit.core
Warning
Fri Jun 15 16:41:11 CEST 2018
Builtin LFS support not present/detected

java.lang.ClassNotFoundException: org.eclipse.jgit.lfs.BuiltinLFS cannot be found by org.eclipse.egit.core_5.1.0.201806142145
	at org.eclipse.osgi.internal.loader.BundleLoader.findClassInternal(BundleLoader.java:508)
	at org.eclipse.osgi.internal.loader.BundleLoader.findClass(BundleLoader.java:419)
	at org.eclipse.osgi.internal.loader.BundleLoader.findClass(BundleLoader.java:411)
	at org.eclipse.osgi.internal.loader.ModuleClassLoader.loadClass(ModuleClassLoader.java:150)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
	at java.lang.Class.forName0(Native Method)
	at java.lang.Class.forName(Class.java:264)
	at org.eclipse.egit.core.Activator.registerBuiltinLFS(Activator.java:279)
	at org.eclipse.egit.core.Activator.start(Activator.java:212)
	at org.eclipse.osgi.internal.framework.BundleContextImpl$3.run(BundleContextImpl.java:779)
	at org.eclipse.osgi.internal.framework.BundleContextImpl$3.run(BundleContextImpl.java:1)
	at java.security.AccessController.doPrivileged(Native Method)
	at org.eclipse.osgi.internal.framework.BundleContextImpl.startActivator(BundleContextImpl.java:772)
	at org.eclipse.osgi.internal.framework.BundleContextImpl.start(BundleContextImpl.java:729)
	at org.eclipse.osgi.internal.framework.EquinoxBundle.startWorker0(EquinoxBundle.java:1002)
	at org.eclipse.osgi.internal.framework.EquinoxBundle$EquinoxModule.startWorker(EquinoxBundle.java:354)
	at org.eclipse.osgi.container.Module.doStart(Module.java:581)
	at org.eclipse.osgi.container.Module.start(Module.java:449)
	at org.eclipse.osgi.framework.util.SecureAction.start(SecureAction.java:468)
	at org.eclipse.osgi.internal.hooks.EclipseLazyStarter.postFindLocalClass(EclipseLazyStarter.java:114)
	at org.eclipse.osgi.internal.loader.classpath.ClasspathManager.findLocalClass(ClasspathManager.java:505)
	at org.eclipse.osgi.internal.loader.ModuleClassLoader.findLocalClass(ModuleClassLoader.java:328)
	at org.eclipse.osgi.internal.loader.BundleLoader.findLocalClass(BundleLoader.java:392)
	at org.eclipse.osgi.internal.loader.sources.SingleSourcePackage.loadClass(SingleSourcePackage.java:36)
	at org.eclipse.osgi.internal.loader.BundleLoader.findClassInternal(BundleLoader.java:454)
	at org.eclipse.osgi.internal.loader.BundleLoader.findClass(BundleLoader.java:419)
	at org.eclipse.osgi.internal.loader.BundleLoader.findClass(BundleLoader.java:411)
	at org.eclipse.osgi.internal.loader.ModuleClassLoader.loadClass(ModuleClassLoader.java:150)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
	at org.eclipse.egit.ui.Activator$RepositoryChangeScanner.<init>(Activator.java:921)
	at org.eclipse.egit.ui.Activator.setupRepoChangeScanner(Activator.java:1034)
	at org.eclipse.egit.ui.Activator.start(Activator.java:336)
	at org.eclipse.osgi.internal.framework.BundleContextImpl$3.run(BundleContextImpl.java:779)
	at org.eclipse.osgi.internal.framework.BundleContextImpl$3.run(BundleContextImpl.java:1)
	at java.security.AccessController.doPrivileged(Native Method)
	at org.eclipse.osgi.internal.framework.BundleContextImpl.startActivator(BundleContextImpl.java:772)
	at org.eclipse.osgi.internal.framework.BundleContextImpl.start(BundleContextImpl.java:729)
	at org.eclipse.osgi.internal.framework.EquinoxBundle.startWorker0(EquinoxBundle.java:1002)
	at org.eclipse.osgi.internal.framework.EquinoxBundle$EquinoxModule.startWorker(EquinoxBundle.java:354)
	at org.eclipse.osgi.container.Module.doStart(Module.java:581)
	at org.eclipse.osgi.container.Module.start(Module.java:449)
	at org.eclipse.osgi.framework.util.SecureAction.start(SecureAction.java:468)
	at org.eclipse.osgi.internal.hooks.EclipseLazyStarter.postFindLocalClass(EclipseLazyStarter.java:114)
	at org.eclipse.osgi.internal.loader.classpath.ClasspathManager.findLocalClass(ClasspathManager.java:505)
	at org.eclipse.osgi.internal.loader.ModuleClassLoader.findLocalClass(ModuleClassLoader.java:328)
	at org.eclipse.osgi.internal.loader.BundleLoader.findLocalClass(BundleLoader.java:392)
	at org.eclipse.osgi.internal.loader.BundleLoader.findClassInternal(BundleLoader.java:470)
	at org.eclipse.osgi.internal.loader.BundleLoader.findClass(BundleLoader.java:419)
	at org.eclipse.osgi.internal.loader.BundleLoader.findClass(BundleLoader.java:411)
	at org.eclipse.osgi.internal.loader.ModuleClassLoader.loadClass(ModuleClassLoader.java:150)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
	at org.eclipse.osgi.internal.framework.EquinoxBundle.loadClass(EquinoxBundle.java:609)
	at org.eclipse.core.internal.registry.osgi.RegistryStrategyOSGI.createExecutableExtension(RegistryStrategyOSGI.java:177)
	at org.eclipse.core.internal.registry.ExtensionRegistry.createExecutableExtension(ExtensionRegistry.java:931)
	at org.eclipse.core.internal.registry.ConfigurationElement.createExecutableExtension(ConfigurationElement.java:243)
	at org.eclipse.core.internal.registry.ConfigurationElementHandle.createExecutableExtension(ConfigurationElementHandle.java:60)
	at org.eclipse.ui.internal.services.WorkbenchServiceRegistry.getSourceProviders(WorkbenchServiceRegistry.java:174)
	at org.eclipse.ui.internal.services.SourceProviderService.readRegistry(SourceProviderService.java:104)
	at org.eclipse.ui.internal.Workbench$34.runWithException(Workbench.java:2342)
	at org.eclipse.ui.internal.StartupThreading$StartupRunnable.run(StartupThreading.java:32)
	at org.eclipse.swt.widgets.Synchronizer.syncExec(Synchronizer.java:233)
	at org.eclipse.ui.internal.UISynchronizer.syncExec(UISynchronizer.java:144)
	at org.eclipse.swt.widgets.Display.syncExec(Display.java:5831)
	at org.eclipse.ui.internal.StartupThreading.runWithoutExceptions(StartupThreading.java:95)
	at org.eclipse.ui.internal.Workbench.initializeDefaultServices(Workbench.java:2337)
	at org.eclipse.ui.internal.Workbench.init(Workbench.java:1618)
	at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2806)
	at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:612)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:336)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:560)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:148)
	at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:152)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:134)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:388)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:243)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:656)
	at org.eclipse.equinox.launcher.Main.basicRun(Main.java:592)
	at org.eclipse.equinox.launcher.Main.run(Main.java:1498)
	at org.eclipse.equinox.launcher.Main.main(Main.java:1471)

Comment 8 Thomas Wolf 2018-06-15 11:11:07 EDT

(In reply to Andrey Loskutov from comment #7)
> I'm observing this warning on every startup now: "Builtin LFS support not
> present/detected".

Of course, if someone doesn't install the jgit.lfs bundle, he'll get that warning every time. I agree with Andrey. This should either be behind a preference, or be given only if the user's git config says he wants to use built-in JGit LFS.

Comment 9 Andrey Loskutov 2018-06-15 11:29:37 EDT

Sorry for hijacking this bug, but there is another issue with the new LFS support. I don't know how long loading of BuiltinLFS and registering takes, but if > 5 seconds, egit activation via org.eclipse.egit.core.Activator.registerBuiltinLFS() can fail with timeout because OSGI does not allow too long work on start(), see javadoc "Note 2: This method is intended to perform simple initialization of the plug-in environment. The platform may terminate initializers that do not complete in a timely fashion."

Comment 10 Andrey Loskutov 2018-06-15 11:33:14 EDT

(In reply to Thomas Wolf from comment #8)
> (In reply to Andrey Loskutov from comment #7)
> > I'm observing this warning on every startup now: "Builtin LFS support not
> > present/detected".
> 
> Of course, if someone doesn't install the jgit.lfs bundle, he'll get that
> warning every time. I agree with Andrey. This should either be behind a
> preference, or be given only if the user's git config says he wants to use
> built-in JGit LFS.

I've created bug 535954.

Comment 11 Markus Duft 2018-06-18 01:32:53 EDT

(In reply to Andrey Loskutov from comment #9)
> Sorry for hijacking this bug, but there is another issue with the new LFS
> support. I don't know how long loading of BuiltinLFS and registering takes,
> but if > 5 seconds, egit activation via
> org.eclipse.egit.core.Activator.registerBuiltinLFS() can fail with timeout
> because OSGI does not allow too long work on start(), see javadoc "Note 2:
> This method is intended to perform simple initialization of the plug-in
> environment. The platform may terminate initializers that do not complete in
> a timely fashion."

Time is not an issue here. Basically the initialization is adding 2 entries to a map, and settings a single pointer. Thats it. But apart from this, I agree that the current mechanism is merely a first version to get it to run :) Now we can make it nice :)