Bug 543363 - Running the Formatter Application fail
Summary: Running the Formatter Application fail
Status: RESOLVED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 4.11   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 4.15 M3   Edit
Assignee: Jonah Graham CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 534081 552973 (view as bug list)
Depends on: 558759
Blocks: 552495
  Show dependency tree
 
Reported: 2019-01-11 04:38 EST by Joe chen CLA
Modified: 2020-01-30 09:41 EST (History)
10 users (show)

See Also:


Attachments
error log file (31.12 KB, text/x-log)
2019-01-11 04:38 EST, Joe chen CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Joe chen CLA 2019-01-11 04:38:08 EST
Created attachment 277131 [details]
error log file

by descript in hlep document at below url

https://help.eclipse.org/2018-12/index.jsp

Java development user guide > Tasks > Using the Formatter Application

  eclipse -vm <path to virtual machine> -application org.eclipse.jdt.core.JavaCodeFormatter [ OPTIONS ] <files>

when I execute

  LANG=C DISPLAY= ./eclipse  -application org.eclipse.jdt.core.JavaCodeFormatter -help

I will get error message at console

Eclipse: Cannot open display: 
Eclipse: Cannot open display: 
org.eclipse.m2e.logback.configuration: The org.eclipse.m2e.logback.configuration bundle was activated before the state location was initialized.  Will retry after the state location is initialized.
Eclipse: Cannot open display: 
Eclipse:
An error has occurred. See the log file
/home/scsi/usr/eclipse/eclipse-2018.12/configuration/1547197914186.log.

Thanks for your help
Comment 1 Stephan Herrmann CLA 2019-01-13 13:35:44 EST
Please try adding s.t. like "-data /tmp/no_workspace" to your command line.

This shouldn't be necessary, but will tell us whether you see the same as I do.

@Mateusz, do you have an idea how this worked in the past? Does the formatter require any access to a workspace? Should we ask core.resources if more is being done now in org.eclipse.core.resources.ResourcesPlugin.start() ?
Comment 2 Mateusz Matela CLA 2019-01-15 17:24:41 EST
This problem exists since Photon.

The line where the root cause exception is thrown in org.eclipse.core.internal.runtime.DataArea:59 and has the following comment:
> // See bug 514333: don't allow clients to initialize instance location
> if the instance area is not explicitly defined yet

Bug 514333 was done in Photon and its description seems to explain your observation that setting -data helps. From the surrounding code and the discussions in the bug I gather CodeFormatterApplication should probably set osgi.dataAreaRequiresExplicitInit to false somewhere, but I don't understand the details.
Comment 3 Andrey Loskutov CLA 2019-01-15 17:31:57 EST
(In reply to Mateusz Matela from comment #2)
> This problem exists since Photon.
> 
> The line where the root cause exception is thrown in
> org.eclipse.core.internal.runtime.DataArea:59 and has the following comment:
> > // See bug 514333: don't allow clients to initialize instance location
> > if the instance area is not explicitly defined yet
> 
> Bug 514333 was done in Photon and its description seems to explain your
> observation that setting -data helps. From the surrounding code and the
> discussions in the bug I gather CodeFormatterApplication should probably set
> osgi.dataAreaRequiresExplicitInit to false somewhere, but I don't understand
> the details.

This is in general a bad idea, because the application will create and re-use a default workspace in default location, making the build dependent on current user environment, and also disallowing parallel builds. In most cases no unattended workspace creation should happen, so the jdt formatter code should either not rely on workspace or explicitly require workspace.
Comment 4 Mateusz Matela CLA 2019-01-15 17:42:55 EST
(In reply to Andrey Loskutov from comment #3)
> In most
> cases no unattended workspace creation should happen, so the jdt formatter
> code should either not rely on workspace or explicitly require workspace.

Thanks for the quick reaction :)

The formatter application doesn't do anything with the workspace. It's that org.eclipse.jdt.core bundle requires org.eclipse.core.resources which fails in ResourcesPlugin.start(). It seems to be trying to create a wokspace if it doesn't exist, but the exception is thrown during checking.

Should we ask org.eclipse.core.resources to change their implementation?
Comment 5 Andrey Loskutov CLA 2019-01-15 17:52:18 EST
(In reply to Mateusz Matela from comment #4)
> Should we ask org.eclipse.core.resources to change their implementation?

No, it is impossible, resources IS the workspace :) 
What one could do is to change jdt core/formatter to not load resources on startup, or, for the formatter application to *require* the workspace to be given.
Comment 6 Mateusz Matela CLA 2019-01-15 18:01:22 EST
(In reply to Andrey Loskutov from comment #5)
> What one could do is to change jdt core/formatter to not load resources on
> startup, or, for the formatter application to *require* the workspace to be
> given.

Requiring a workspace would go against the purpose of the app, which is to make it as simple as possible to run the formatter from command line.

What does it mean for jdt.core to not load resources at startup? The stacktraces in log don't show any jdt.core classes, so it's not done explicitly. I guess resources is started simply because it's in required bunldes. Can a constraint be setup so that a required bundle is not started?
Comment 7 Stephan Herrmann CLA 2019-01-15 18:25:23 EST
ResourcesPlugin.start() is only triggered when a class in the plug-in is touched. But I guess we cannot avoid touching classes from core.resources during the formatter application, or can we?

Can we reconstruct how the problem was avoided pre Photon? Was the default workspace implicitly used, implicitly created?

Can the formatter app define and use a dummy workspace in some temp dir?
Comment 8 Andrey Loskutov CLA 2019-01-16 00:50:22 EST
(In reply to Stephan Herrmann from comment #7)
> ResourcesPlugin.start() is only triggered when a class in the plug-in is
> touched. But I guess we cannot avoid touching classes from core.resources
> during the formatter application, or can we?
> 
> Can we reconstruct how the problem was avoided pre Photon? Was the default
> workspace implicitly used, implicitly created?
> 
> Can the formatter app define and use a dummy workspace in some temp dir?

Before you had a ~/workspace created *and used* every time you had formatter call in a script. For us it caused all kinds of unexpected build issues. Some users with a regular workspace at same place had build errors because they workspace had different settings etc, unexpected by the build. Also they had issues that build was failing if they had multiple builds scheduled, because running in parallel is not an option due workspace locks. So we end up explicitly specifying -data location for formatter application.

Because of this, formatter app should not define *some* location for workspace, if we can't get rid of Resources bundle activation, it should be still allowed to specify the workspace.

Better we should investigate how to teach formatter app to avoid resources activation. I remember this was due some preferences API used, but I have to check the stack trace.
Comment 9 Stephan Herrmann CLA 2019-01-16 10:21:56 EST
(In reply to Andrey Loskutov from comment #8)
> Because of this, formatter app should not define *some* location for
> workspace, if we can't get rid of Resources bundle activation, it should be
> still allowed to specify the workspace.

IIUC, the formatter app doesn't *use* the workspace, so if a workspace is *accidentally* accessed, wouldn't it be best to ensure it is empty and not shared with anything else?
 
> Better we should investigate how to teach formatter app to avoid resources
> activation. I remember this was due some preferences API used, but I have to
> check the stack trace.

This would certainly be preferable, if such avoidance could possibly be (future-) safe.

Following Mateusz's train of thought, it would be nice if the formatter could signal: "I don't mean to access core.resources, if that still happens, please raise an exception". But I guess no API exists for this :/
Comment 10 Mateusz Matela CLA 2019-01-23 16:11:45 EST
(In reply to Andrey Loskutov from comment #8)
> Better we should investigate how to teach formatter app to avoid resources
> activation. I remember this was due some preferences API used, but I have to
> check the stack trace.

It seems it isn't a fault of any java code, because even when I remove everything from CodeFormatterApplication (except for "return IApplication.EXIT_OK;") the problem persists. Like the class is not even accessed before the error occurs.
Any other ideas?
Comment 11 Andrey Loskutov CLA 2019-01-23 17:11:54 EST
As said, JDT most likely requires and activates resources plugin. You can put breakpoint at the resources bundle activator constructor and at start() and go down the stack checking who activates the bundle. I bet this is from JDT core activation, some preference lookup requiring workspace scope. 
If you have some easy steps to reproduce, please leave them here, may be I will have some time to debug it.
Comment 12 Mateusz Matela CLA 2019-01-23 17:33:01 EST
(In reply to Andrey Loskutov from comment #11)
> If you have some easy steps to reproduce, please leave them here, may be I
> will have some time to debug it.

It's quite simple to debug through remote. I add these vmargs in eclipse.ini:
-Xdebug
-Xnoagent
-Xrunjdwp:transport=dt_socket,server=y,suspend=y,address=4444

Then run
eclipse.exe -application org.eclipse.jdt.core.JavaCodeFormatter

Now it's waiting for a debugger to connect, so I can add breakpoints and start remote debug in Eclipse on port 4444.
Comment 13 Andrey Loskutov CLA 2019-01-23 17:35:54 EST
Why remotely? If I only need that application and no arguments, should be easily possible to use default launch config with that application set. No arguments like files to format needed at all?
Comment 14 Mateusz Matela CLA 2019-01-23 18:12:21 EST
If the app complaints about missing arguments, it means it already went past the stage with the reported problem :)
When I try creating proper run configurations as "Eclipse application", it produces some other mysterious errors, I don't know why... Maybe it's just some mess in my workspace.
Comment 15 Mateusz Matela CLA 2019-03-12 17:12:03 EDT
*** Bug 534081 has been marked as a duplicate of this bug. ***
Comment 16 Jonah Graham CLA 2019-11-12 14:53:51 EST
*** Bug 552973 has been marked as a duplicate of this bug. ***
Comment 17 Jonah Graham CLA 2019-11-12 19:43:38 EST
This conversation is split across many bugs. A key comment in one of those now closed bugs is:

(In reply to Andrey Loskutov from Bug 534081 comment #1)
> The main problem is that the formatter app requires workspace to be set for
> the current Eclipse instance. This is because it is bound to JDT core which
> requires resources plugin to be activated, which in turn implies a workspace
> to be set.
> 
> I think this is more a request to the platform resources to allow -data=none
> to be used *together* with resources plugin. As far as I remember, if using
> -data=@none, resources plugin throws some exception about no workspace on
> startup.
> 
> I wonder if resources plugin can check that @none is set and throw
> exceptions only if called to do something with workspace, not directly on
> activation.

@none working is a nice idea, perhaps an @temp which would automatically use a new temp dir for the workspace and then dispose it at the end of the run.

----

From my reading of this and the conversations surrounding, it is unlikely that the formatter app will run anytime soon without an explicit workspace or some other changes that are not likely to be resourced soon. Therefore we should do the following:

1- Fix the documentation to match reality
2- Improve the error message so that there is some indication to the user what to do.

I'll provide a fix for 1 momentarily, but reality is I have no idea how to improve 2 - as mentioned in Comment 10 the exception happens before the application's entry point.
Comment 18 Eclipse Genie CLA 2019-11-12 19:55:53 EST
New Gerrit change created: https://git.eclipse.org/r/152534
Comment 19 Eclipse Genie CLA 2019-11-12 22:09:01 EST
New Gerrit change created: https://git.eclipse.org/r/152535
Comment 20 Jonah Graham CLA 2019-11-12 22:14:15 EST
(In reply to Eclipse Genie from comment #19)
> New Gerrit change created: https://git.eclipse.org/r/152535

So I did come up with a possible way to have a proper warning about -data before the exception is thrown. I did it by having a new bundle that only activates org.eclipse.jdt.core once the sanity check on -data has been done.

If this looks like a suitable solution for the JDT folk I can try to add the releng work that is needed around the code. For now I only added the basic project files to work in Eclipse.

(In reply to Andrey Loskutov from comment #13)
> Why remotely? If I only need that application and no arguments, should be
> easily possible to use default launch config with that application set. No
> arguments like files to format needed at all?

I had a problem with remote vs launching within Eclipse. In the end I set workspace in Eclipse Launch Config to @noDefault to achieve what (I hope) is the same behaviour as launching eclipse from the command line. Simply leaving the workspace field blank defaulted the effective workspace to ${user.dir}/workspace
Comment 21 Andrey Loskutov CLA 2019-12-06 10:09:37 EST
(In reply to Jonah Graham from comment #20)
> (In reply to Eclipse Genie from comment #19)
> > New Gerrit change created: https://git.eclipse.org/r/152535
> 
> So I did come up with a possible way to have a proper warning about -data
> before the exception is thrown. I did it by having a new bundle that only
> activates org.eclipse.jdt.core once the sanity check on -data has been done.
> 
> If this looks like a suitable solution for the JDT folk I can try to add the
> releng work that is needed around the code. For now I only added the basic
> project files to work in Eclipse.

I think this is worth to try out. Since I'm not a maven expert, I would appreciate if you would add all the woodoo needed to integrate this patch into the build.
Comment 22 Eclipse Genie CLA 2019-12-06 11:46:38 EST
New Gerrit change created: https://git.eclipse.org/r/154027
Comment 23 Jonah Graham CLA 2019-12-06 11:54:19 EST
(In reply to Andrey Loskutov from comment #21)
> I think this is worth to try out. Since I'm not a maven expert, I would
> appreciate if you would add all the woodoo needed to integrate this patch
> into the build.

Done - waiting on builds which may all fail until next week[1] though to see if I did it right.

[1] https://www.eclipse.org/lists/platform-releng-dev/msg35314.html
Comment 25 Jonah Graham CLA 2019-12-16 12:19:04 EST
(In reply to Eclipse Genie from comment #24)
> Gerrit change https://git.eclipse.org/r/152534 was merged to [master].
> Commit:
> http://git.eclipse.org/c/platform/eclipse.platform.common.git/commit/
> ?id=cf320f2eda304cfd7049f43da21b30f683914a62

Thank you Karsten for merging https://git.eclipse.org/r/#/c/152534/ - now I hope someone has time to review my approach to solving the problem that I added in https://git.eclipse.org/r/#/c/152535/ I would appreciate any feedback on that idea as I need a solution for CDT too.
Comment 26 Jonah Graham CLA 2020-01-02 17:55:08 EST
Can someone help out with the build error on the gerrit (https://ci.eclipse.org/jdt/job/eclipse.jdt.core-Gerrit/2577/console):

 --- tycho-eclipserun-plugin:1.6.0-SNAPSHOT:eclipse-run (api-analysis) @ org.eclipse.jdt.core.formatterapp ---
17:47:12 [INFO] Toolchain in tycho-eclipserun-plugin: JDK[/opt/tools/java/oracle/jdk-8/latest/jre]
17:47:12 [INFO] Expected eclipse log file: /home/jenkins/agent/workspace/eclipse.jdt.core-Gerrit/org.eclipse.jdt.core.formatterapp/target/eclipserun-work/data/.metadata/.log
17:47:12 [INFO] Command line:
17:47:12 	[/opt/tools/java/oracle/jdk-8/latest/jre/bin/java, -Xmx2048M, -jar, /home/jenkins/agent/workspace/eclipse.jdt.core-Gerrit/.repository/p2/osgi/bundle/org.eclipse.equinox.launcher/1.5.600.v20191014-2022/org.eclipse.equinox.launcher-1.5.600.v20191014-2022.jar, -install, /home/jenkins/agent/workspace/eclipse.jdt.core-Gerrit/org.eclipse.jdt.core.formatterapp/target/eclipserun-work, -configuration, /home/jenkins/agent/workspace/eclipse.jdt.core-Gerrit/org.eclipse.jdt.core.formatterapp/target/eclipserun-work/configuration, -data, /home/jenkins/agent/workspace/eclipse.jdt.core-Gerrit/org.eclipse.jdt.core.formatterapp/../target/org.eclipse.jdt.core.formatterapp-apiAnalyzer-workspace, -application, org.eclipse.pde.api.tools.apiAnalyzer, -project, /home/jenkins/agent/workspace/eclipse.jdt.core-Gerrit/org.eclipse.jdt.core.formatterapp, -baseline, /home/jenkins/agent/workspace/eclipse.jdt.core-Gerrit/org.eclipse.jdt.core.formatterapp/target/org.eclipse.jdt.core.formatterapp-apiBaseline.target, -dependencyList, /home/jenkins/agent/workspace/eclipse.jdt.core-Gerrit/org.eclipse.jdt.core.formatterapp/target/dependencies-list.txt, -failOnError]
17:47:21 org.eclipse.core.runtime.CoreException: Problems occurred while resolving the target contents
17:47:21 	at org.eclipse.pde.api.tools.internal.ApiAnalysisApplication.setBaseline(ApiAnalysisApplication.java:258)
17:47:21 	at org.eclipse.pde.api.tools.internal.ApiAnalysisApplication.start(ApiAnalysisApplication.java:120)
17:47:21 	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:203)
17:47:21 	at org.eclipse.equinox.internal.app.AnyThreadAppLauncher.run(AnyThreadAppLauncher.java:30)
17:47:21 	at java.lang.Thread.run(Thread.java:748)
17:47:21 Contains: Problems loading repositories
17:47:21 Contains: Problems loading repositories
17:47:21 Contains: Unable to locate installable unit org.eclipse.jdt.core.formatterapp 0.0.0

I assume this has to do with some releng changes due to new bundle.

Thank you.
Comment 27 Andrey Loskutov CLA 2020-01-02 19:15:57 EST
Johan, I believe there was a similar issue recently, please check this thread below.

https://www.eclipse.org/lists/platform-releng-dev/msg35415.html
Comment 28 Jonah Graham CLA 2020-01-02 20:00:29 EST
Looks like I need to skip api analysis as that other new bundle did. https://git.eclipse.org/r/#/c/154733/4/bundles/org.eclipse.e4.ui.ide/pom.xml,unified I will update soon, thanks for the pointers.
Comment 29 Jonah Graham CLA 2020-01-03 14:24:42 EST
Can this be targetted at 4.15 M1 - I don't feel comfortable making that change myself, while I am able to update reviews, a committer needs to have a look at this at some point. I have finally got the gerrit (https://git.eclipse.org/r/#/c/152535/) through the build.
Comment 30 Jonah Graham CLA 2020-01-14 16:43:13 EST
(In reply to Jonah Graham from comment #29)
> Can this be targetted at 4.15 M1

How about M2?
Comment 32 Andrey Loskutov CLA 2020-01-17 07:59:13 EST
(In reply to Eclipse Genie from comment #31)
> Gerrit change https://git.eclipse.org/r/152535 was merged to [master].
> Commit:
> http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/
> ?id=28edb69b5bee0985c95efe9b5dcfbc219159c921

Let see if this breaks the SDK build over the weekend.

Note: if the build works, before resolving this, we need to flip the API analysis "skipAPIAnalysis" back to false:
https://git.eclipse.org/r/#/c/152535/15/org.eclipse.jdt.core.formatterapp/pom.xml@25

I will provide gerrit for this, nut sure however if we need to wait with the merge for the M2 build to have the new bundle in the "baseline", or even for the new release.
Comment 33 Eclipse Genie CLA 2020-01-17 08:04:38 EST
New Gerrit change created: https://git.eclipse.org/r/156087
Comment 34 Andrey Loskutov CLA 2020-01-17 08:08:49 EST
(In reply to Eclipse Genie from comment #33)
> New Gerrit change created: https://git.eclipse.org/r/156087

The API analysis flag patch. I've asked Vikas after which build we can merge that.
Comment 35 Andrey Loskutov CLA 2020-01-18 03:46:46 EST
(In reply to Eclipse Genie from comment #31)
> Gerrit change https://git.eclipse.org/r/152535 was merged to [master].
> Commit:
> http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/
> ?id=28edb69b5bee0985c95efe9b5dcfbc219159c921

This broke the SDK in the sense that the org.eclipse.jdt.core.formatterapp was not shipped with SDK and therefore starting formatter app fails with  "Application "org.eclipse.jdt.core.JavaCodeFormatter" could not be found in the registry.".

We need some extra configuration that says SDK that it should include org.eclipse.jdt.core.formatterapp bundle.

I guess this is jdt.core feature.xml, will push the patch in a moment.
Comment 36 Eclipse Genie CLA 2020-01-18 03:50:02 EST
New Gerrit change created: https://git.eclipse.org/r/156129
Comment 38 Andrey Loskutov CLA 2020-01-18 04:27:13 EST
(In reply to Eclipse Genie from comment #37)
> Gerrit change https://git.eclipse.org/r/156129 was merged to [master].
> Commit:
> http://git.eclipse.org/c/jdt/eclipse.jdt.git/commit/
> ?id=2ead9019f95c7db79df31191ed8b95fd641f97df

The gerrit https://git.eclipse.org/r/#/c/156129/ that should add the bundle to the feature fails because it can't find the bundle because it was not published to the update site because it was not added to the feature on previous build. A nice recursion :-)

According to https://www.eclipse.org/lists/platform-releng-dev/msg35421.html this is expected, so the next SDK build should both succeed and contain the new bundle. I will check that tomorrow or cry for the help.
Comment 39 Andrey Loskutov CLA 2020-01-19 02:31:25 EST
Good news: SDK build didn't fail, bad news: we've got relent test error:

https://download.eclipse.org/eclipse/downloads/drops4/I20200118-1800/testresults/html/org.eclipse.releng.tests_ep415I-unit-cen64-gtk3-java8_linux.gtk.x86_64_8.0.html

Plugin directory missing required files: /home/cbi/genie.releng/workspace/ep415I-unit-cen64-gtk3-java8/workarea/I20200118-1800/eclipse-testing/test-eclipse/eclipse/plugins/org.eclipse.jdt.core.formatterapp.source_1.0.0.v20200117-0823.jar;

java.lang.AssertionError: Plugin directory missing required files: /home/cbi/genie.releng/workspace/ep415I-unit-cen64-gtk3-java8/workarea/I20200118-1800/eclipse-testing/test-eclipse/eclipse/plugins/org.eclipse.jdt.core.formatterapp.source_1.0.0.v20200117-0823.jar;
at org.junit.Assert.fail(Assert.java:88)
at org.junit.Assert.assertTrue(Assert.java:41)
at org.eclipse.releng.tests.BuildTests.testPluginFiles(BuildTests.java:424)

I assume we need some pom magic here, going to investigate.
Comment 40 Alexander Kurtakov CLA 2020-01-19 02:39:51 EST
(In reply to Andrey Loskutov from comment #39)
> Good news: SDK build didn't fail, bad news: we've got relent test error:
> 
> https://download.eclipse.org/eclipse/downloads/drops4/I20200118-1800/
> testresults/html/org.eclipse.releng.tests_ep415I-unit-cen64-gtk3-java8_linux.
> gtk.x86_64_8.0.html
> 
> Plugin directory missing required files:
> /home/cbi/genie.releng/workspace/ep415I-unit-cen64-gtk3-java8/workarea/
> I20200118-1800/eclipse-testing/test-eclipse/eclipse/plugins/org.eclipse.jdt.
> core.formatterapp.source_1.0.0.v20200117-0823.jar;
> 
> java.lang.AssertionError: Plugin directory missing required files:
> /home/cbi/genie.releng/workspace/ep415I-unit-cen64-gtk3-java8/workarea/
> I20200118-1800/eclipse-testing/test-eclipse/eclipse/plugins/org.eclipse.jdt.
> core.formatterapp.source_1.0.0.v20200117-0823.jar;
> at org.junit.Assert.fail(Assert.java:88)
> at org.junit.Assert.assertTrue(Assert.java:41)
> at org.eclipse.releng.tests.BuildTests.testPluginFiles(BuildTests.java:424)
> 
> I assume we need some pom magic here, going to investigate.

No pom magic here :) . What is missing is src.includes = about.html in build.properties :)
Comment 41 Eclipse Genie CLA 2020-01-19 02:49:18 EST
New Gerrit change created: https://git.eclipse.org/r/156139
Comment 42 Andrey Loskutov CLA 2020-01-19 02:50:16 EST
(In reply to Alexander Kurtakov from comment #40) 
> No pom magic here :) . What is missing is src.includes = about.html in
> build.properties :)

Thanks Alex! Would never notice that!
Comment 44 Jonah Graham CLA 2020-01-19 12:54:24 EST
Thank you Andrey for pushing this through the system and others for review and identifying things.

I will test it out in the SDK this week and then start applying the same change to CDT's version in Bug 552495.
Comment 45 Dani Megert CLA 2020-01-20 07:24:54 EST
Sorry to chime in late, but I'd like to see this reverted.

First, it will break clients of the formatter app who have their defined set of bundles in their target (or installed). The new bundle will not be there and the formatter app will fail.

Second, we should not start to create separate bundles for each application. Instead, a general solution needs to be found that waits for the resources bundle(s) to be loaded.
Comment 46 Andrey Loskutov CLA 2020-01-20 08:28:48 EST
(In reply to Dani Megert from comment #45)
> Sorry to chime in late, but I'd like to see this reverted.
> 
> First, it will break clients of the formatter app who have their defined set
> of bundles in their target (or installed). The new bundle will not be there
> and the formatter app will fail.

Good point. If we try to modify the current app to require -data, would this be acceptable?

> Second, we should not start to create separate bundles for each application.
> Instead, a general solution needs to be found that waits for the resources
> bundle(s) to be loaded.

The problem is not that resources bundle is not yet loaded, bit that it can be loaded with *implicitly* defined default workspace, or that it is throwing an error in the case of such implicit workspace init is disallowed.

Formatter is JDT, JDT is resources based. It will be impossible to decouple formatter from JDT or JDT from resources.
Comment 47 Andrey Loskutov CLA 2020-01-20 10:29:50 EST
(In reply to Andrey Loskutov from comment #46)
> (In reply to Dani Megert from comment #45)
> > Sorry to chime in late, but I'd like to see this reverted.

Before we start revert, may be we can agree on compromise: we restore the old app and provide a *new* one with the -data check added? I honestly don't see other *non-breaking* solution that we could provide with a *reasonable* effort, and reverting to the status quo is too lame if we have something useful (the original problem will remain).
Comment 48 Dani Megert CLA 2020-01-20 11:44:09 EST
(In reply to Andrey Loskutov from comment #47)
> (In reply to Andrey Loskutov from comment #46)
> > (In reply to Dani Megert from comment #45)
> > > Sorry to chime in late, but I'd like to see this reverted.
> 
> Before we start revert, may be we can agree on compromise: we restore the
> old app and provide a *new* one with the -data check added? I honestly don't
> see other *non-breaking* solution that we could provide with a *reasonable*
> effort, and reverting to the status quo is too lame if we have something
> useful (the original problem will remain).

> the original problem will remain
Can you repeat for me what it is, together with steps to reproduce?

There are a lot of comments in this bug and some seem contradictory. One claims a workspace is not needed for the formatter app. If so, could we just hard-code it to use no workspace?
Comment 49 Jonah Graham CLA 2020-01-20 14:23:47 EST
(In reply to Dani Megert from comment #48)
> > the original problem will remain
> Can you repeat for me what it is, together with steps to reproduce?

The problem is that user gets errors when -data is not specified, even for -help. These errors are new(ish) because access to workspace when not explicitly set is now an error (Bug 514333 changed that). The documentation has been fixed to say -data is now required[1,2]

To reproduce, run the formatter with -help with and without -data:

$ eclipse -application org.eclipse.jdt.core.JavaCodeFormatter -help


$ eclipse -data /tmp/data -application org.eclipse.jdt.core.JavaCodeFormatter -help
Usage: eclipse -application org.eclipse.jdt.core.JavaCodeFormatter [ OPTIONS ] -config <configFile> <files>

   <files>   Java source files and/or directories to format.
             Only files ending with .java will be formatted in the given directory.
   -config <configFile> Use the formatting style from the specified properties file.
                        Refer to the help documentation to find out how to generate this file.

 OPTIONS:

   -help                Display this message.
   -quiet               Only print error messages.
   -verbose             Be verbose about the formatting job.



[1] https://help.eclipse.org/2019-09/index.jsp?topic=%2Forg.eclipse.jdt.doc.user%2Ftasks%2Ftasks-231.htm
[2] https://git.eclipse.org/r/#/c/152534/4/bundles/org.eclipse.jdt.doc.user/tasks/tasks-231.htm - should be in 4.15 aka 2020-03 help.



> There are a lot of comments in this bug and some seem contradictory. One
> claims a workspace is not needed for the formatter app. 

I believe this is correct - there has been discussion about having a "dummy" or "virtual" workspace that is in memory only or something like that. No persistence, but items trying to access the workspace would just get defaults instead of errors. Perhaps a new option like -data @memory

> If so, could we just hard-code it to use no workspace?

Yes, as long as it was unique. i.e. can't hard-code -data ~/formatter-workspace.


Disentangling the jdt core plug-in so that it does not load Resources bundle would be too high an amount of work. It essentially would mean jdt.core (and all of its dependencies!) would have to stop using Activators that required the Resources Plug-in.
Comment 50 Jonah Graham CLA 2020-01-22 10:32:25 EST
(In reply to Dani Megert from comment #45)
> First, it will break clients of the formatter app who have their defined set
> of bundles in their target (or installed). The new bundle will not be there
> and the formatter app will fail.

I don't know how much of an issue this is, or if it really is worse than other changes in 4.15. There is another new required bundle in 4.15 already and when I upgraded our product defined by plugins to 4.15 target platform it needed[1] the new org.eclipse.e4.ui.ide bundle adding. So, for people who are using bundles only to define their environment, they are likely already facing problems, and for people using features, the JDT feature includes the new bundle[2]

As someone who maintains a product[3] that is based on plug-ins, my experience is that most every release of the Platform changes the set of bundles required in some small way. You can see the history[4] of the debug.product file to get an idea.

[1] https://git.eclipse.org/r/#/c/156283/3/debug/org.eclipse.cdt.debug.application.product/debug.product
[2] https://git.eclipse.org/r/#/c/156129/1/org.eclipse.jdt-feature/feature.xml
[3] https://wiki.eclipse.org/CDT/StandaloneDebugger
[4] https://git.eclipse.org/c/cdt/org.eclipse.cdt.git/blame/debug/org.eclipse.cdt.debug.application.product/debug.product#n174
Comment 51 Andrey Loskutov CLA 2020-01-22 10:43:07 EST
(In reply to Jonah Graham from comment #50)
> (In reply to Dani Megert from comment #45)
> > First, it will break clients of the formatter app who have their defined set
> > of bundles in their target (or installed). The new bundle will not be there
> > and the formatter app will fail.
> 
> I don't know how much of an issue this is, or if it really is worse than
> other changes in 4.15. There is another new required bundle in 4.15 already
> and when I upgraded our product defined by plugins to 4.15 target platform
> it needed[1] the new org.eclipse.e4.ui.ide bundle adding. So, for people who
> are using bundles only to define their environment, they are likely already
> facing problems, and for people using features, the JDT feature includes the
> new bundle[2]

I came to the same conclusion. I withdraw my proposal from comment 47.

All clients that just used default SDK installation (or any of the "standard" packages that included JDT feature) will still work with the current code.

All product-specific products that only included *some* dedicated jdt bundles (and not the jdt feature) and used formatter app via command line are "easy" to fix by adding new bundle to the product target definition.

The only thing we need is to add N&N note about the app moved to a new bundle
(see https://git.eclipse.org/r/www.eclipse.org/eclipse/news.git).

Dani: WDYT?
Comment 52 Jonah Graham CLA 2020-01-22 10:48:57 EST
(In reply to Andrey Loskutov from comment #51)
> The only thing we need is to add N&N note about the app moved to a new bundle
> (see https://git.eclipse.org/r/www.eclipse.org/eclipse/news.git).

Gerrit coming soon.
Comment 53 Eclipse Genie CLA 2020-01-22 11:01:21 EST
New Gerrit change created: https://git.eclipse.org/r/156351
Comment 54 Dani Megert CLA 2020-01-30 03:35:06 EST
(In reply to Andrey Loskutov from comment #51)
> (In reply to Jonah Graham from comment #50)
> > (In reply to Dani Megert from comment #45)
> > > First, it will break clients of the formatter app who have their defined set
> > > of bundles in their target (or installed). The new bundle will not be there
> > > and the formatter app will fail.
> > 
> > I don't know how much of an issue this is, or if it really is worse than
> > other changes in 4.15. There is another new required bundle in 4.15 already
> > and when I upgraded our product defined by plugins to 4.15 target platform
> > it needed[1] the new org.eclipse.e4.ui.ide bundle adding. So, for people who
> > are using bundles only to define their environment, they are likely already
> > facing problems, and for people using features, the JDT feature includes the
> > new bundle[2]
> 
> I came to the same conclusion. I withdraw my proposal from comment 47.
> 
> All clients that just used default SDK installation (or any of the
> "standard" packages that included JDT feature) will still work with the
> current code.
> 
> All product-specific products that only included *some* dedicated jdt
> bundles (and not the jdt feature) and used formatter app via command line
> are "easy" to fix by adding new bundle to the product target definition.
> 
> The only thing we need is to add N&N note about the app moved to a new bundle
> (see https://git.eclipse.org/r/www.eclipse.org/eclipse/news.git).
> 
> Dani: WDYT?
OK. Can you review the proposed N&N entry please.
Comment 55 Andrey Loskutov CLA 2020-01-30 03:50:54 EST
(In reply to Dani Megert from comment #54)
> OK. Can you review the proposed N&N entry please.

Done.
Comment 57 Andrey Loskutov CLA 2020-01-30 09:41:05 EST
Looks like we are done here. Thanks Jonah!