add command to build workspace #326

Running org.eclipse.jdt.ls.core.internal.handlers.BuildWorkspaceHandlerTest
Tests run: 3, Failures: 0, Errors: 3, Skipped: 0, Time elapsed: 0.21 sec <<< FAILURE! - in org.eclipse.jdt.ls.core.internal.handlers.BuildWorkspaceHandlerTest
testCanceledCase(org.eclipse.jdt.ls.core.internal.handlers.BuildWorkspaceHandlerTest)  Time elapsed: 0.086 sec  <<< ERROR!
org.eclipse.core.internal.resources.ResourceException: Cannot create linked resource '/jdt.ls-java-project/src/home/snjeza/projects/eclipse.jdt.ls/org.eclipse.jdt.ls.tests/target/workingProjects/singlefile/s1.java'.  The parent resource is not accessible.
	at org.eclipse.core.internal.resources.Resource.assertLinkRequirements(Resource.java:175)
	at org.eclipse.core.internal.resources.Resource.createLink(Resource.java:640)
	at org.eclipse.jdt.ls.core.internal.managers.AbstractProjectsManagerBasedTest.linkFilesToDefaultProject(AbstractProjectsManagerBasedTest.java:116)
	at org.eclipse.jdt.ls.core.internal.handlers.BuildWorkspaceHandlerTest.setUp(BuildWorkspaceHandlerTest.java:42)

testFailedCase(org.eclipse.jdt.ls.core.internal.handlers.BuildWorkspaceHandlerTest)  Time elapsed: 0.048 sec  <<< ERROR!
org.eclipse.core.internal.resources.ResourceException: Cannot create linked resource '/jdt.ls-java-project/src/home/snjeza/projects/eclipse.jdt.ls/org.eclipse.jdt.ls.tests/target/workingProjects/singlefile/s1.java'.  The parent resource is not accessible.
	at org.eclipse.core.internal.resources.Resource.assertLinkRequirements(Resource.java:175)
	at org.eclipse.core.internal.resources.Resource.createLink(Resource.java:640)
	at org.eclipse.jdt.ls.core.internal.managers.AbstractProjectsManagerBasedTest.linkFilesToDefaultProject(AbstractProjectsManagerBasedTest.java:116)
	at org.eclipse.jdt.ls.core.internal.handlers.BuildWorkspaceHandlerTest.setUp(BuildWorkspaceHandlerTest.java:42)

testSucceedCase(org.eclipse.jdt.ls.core.internal.handlers.BuildWorkspaceHandlerTest)  Time elapsed: 0.065 sec  <<< ERROR!
org.eclipse.core.internal.resources.ResourceException: Cannot create linked resource '/jdt.ls-java-project/src/home/snjeza/projects/eclipse.jdt.ls/org.eclipse.jdt.ls.tests/target/workingProjects/singlefile/s1.java'.  The parent resource is not accessible.
	at org.eclipse.core.internal.resources.Resource.assertLinkRequirements(Resource.java:175)
	at org.eclipse.core.internal.resources.Resource.createLink(Resource.java:640)
	at org.eclipse.jdt.ls.core.internal.managers.AbstractProjectsManagerBasedTest.linkFilesToDefaultProject(AbstractProjectsManagerBasedTest.java:116)
	at org.eclipse.jdt.ls.core.internal.handlers.BuildWorkspaceHandlerTest.setUp(BuildWorkspaceHandlerTest.java:42)

ansyral · 2017-08-24T06:04:10Z

@snjeza unfortunately i cannot repro it locally. BTW, do you know why jenkins build isn't enabled for this pr?

gorkem · 2017-08-24T14:29:27Z

ok to test Jenkins

gorkem · 2017-08-30T08:08:14Z

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/JDTLanguageServer.java

+	 * @see org.eclipse.jdt.ls.core.internal.JavaProtocolExtensions#buildWorkspace(java.lang.String)
+	 */
+	@Override
+	public CompletableFuture<BuildWorkspaceResult> buildWorkspace(String type) {


What are the expected values for type? At the moment it looks like it is not used, we need to clarify its purpose.

I think it could be removed

gorkem · 2017-08-30T08:16:26Z

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/BuildWorkspaceResult.java

+	@NonNull
+	private BuildWorkspaceStatus status;
+
+	private String details;


I think it will be better if details is an array of org.eclipse.lsp4j.Diagnostic instead of an concatenated list of compiler error strings.

gorkem · 2017-08-30T08:19:03Z

...eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/BuildWorkspaceHandler.java

+			}
+		} catch (CoreException e) {
+			logException("Failed to build workspace.", e);
+			return new BuildWorkspaceResult(BuildWorkspaceStatus.FAILED, String.format("Exception: %s.", e.getMessage()));


Should we distinguish between a build failure and a build that returns errors?

nice~ I'll add another status to tell the two

fbricon · 2017-09-13T20:23:44Z

@ansyral any update?

ansyral · 2017-09-19T07:03:02Z

@gorkem added a new iteration and rebased on the master branch

fbricon · 2017-09-19T22:29:04Z

...eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/BuildWorkspaceHandler.java

+	private static List<IMarker> getBuildErrors(IProgressMonitor monitor) throws CoreException {
+		IProject[] projects = ResourcesPlugin.getWorkspace().getRoot().getProjects();
+		List<IMarker> errors = new ArrayList<>();
+		for (IProject project : projects) {


check is monitor is cancelled

fbricon · 2017-09-19T22:31:43Z

...eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/BuildWorkspaceHandler.java

+		for (Map.Entry<IResource, List<IMarker>> entry : map.entrySet()) {
+			IResource resource = entry.getKey();
+			IFile file = resource.getAdapter(IFile.class);
+			if (file != null && JavaCore.isJavaLikeFileName(file.getName())) {


this will miss build files. Check with ProjectsManager#isBuildFile

fbricon · 2017-09-19T22:36:41Z

org.eclipse.jdt.ls.tests/projects/singlefile/s1.java

@@ -0,0 +1,7 @@
+public class s1 {


Please use Java conventions: Pascal cased files and class names.

ansyral · 2017-09-20T03:24:59Z

One concern is that right now language server only publish syntax-like problems for user edit/save. while this command would send all errors to problem panel. Seems inconsistent, @fbricon @gorkem any idea why LS only reports syntax-like problems?

fbricon · 2017-09-21T20:32:37Z

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/BuildWorkspaceStatus.java

+ */
+public enum BuildWorkspaceStatus {
+
+	FAILED, SUCCEED, WITHERROR, CANCELLED,


fbricon · 2017-09-21T20:40:24Z

We should probably report some progress to the client, not sure how though. But I know vscode supports it

fbricon · 2017-09-21T20:42:48Z

@ansyral the reason why we only report syntax errors on standalone java files is explained here.

Indeed, workspace builds should report the same errors for standalone files (i.e attached to the default project)

ansyral · 2017-09-22T03:48:41Z

@fbricon added the progress in the pr redhat-developer/vscode-java#298

ansyral · 2017-10-25T09:16:17Z

@fbricon do you have more comments about the pr? could you please help merge the pr if no more comments? Thanks.

fbricon

the PR cannot be rebased cleanly

fbricon · 2017-10-25T21:50:01Z

...eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/BuildWorkspaceHandler.java

+
+	public BuildWorkspaceStatus buildWorkspace(IProgressMonitor monitor) {
+		try {
+			ResourcesPlugin.getWorkspace().build(IncrementalProjectBuilder.INCREMENTAL_BUILD, monitor);


shouldn't it be a full build? or even better, have the build kind be received as a command parameter?

added a parameter

fbricon · 2017-10-25T22:04:30Z

...ls.tests/src/org/eclipse/jdt/ls/core/internal/managers/AbstractProjectsManagerBasedTest.java

+		return to;
+	}
+
+	private static void createFolders(IContainer folder, IProgressMonitor monitor) throws CoreException {


this should be replaced by a call to JDTUtils.createFolders

Signed-off-by: xuzho <[email protected]>

fbricon · 2017-10-26T14:19:41Z

@tsmaeder is this something Che would use? If yes, can you check the API matches your expectations?

fbricon · 2017-10-27T03:41:03Z

I noticed that if you delete some packages from CLI (outside vscode), the missing classes are not picked up, so no compilation errors. Calling the build command won't actually show any compilation either. You need to restart vscode. I think the workspace needs to be refreshed before the build is triggered

ansyral · 2017-10-30T06:21:45Z

@fbricon seems like a known issue of vscode

fbricon · 2017-10-30T18:41:18Z

...eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/BuildWorkspaceHandler.java

+	}
+
+	public BuildWorkspaceStatus buildWorkspace(boolean forceReBuild, IProgressMonitor monitor) {
+		try {


Calling

ResourcesPlugin.getWorkspace().getRoot().refreshLocal(IResource.DEPTH_INFINITE, monitor);

here, fixes the issues with out-of-synch file system.

fbricon · 2017-10-31T22:47:32Z

merged as 11f2f46 (via #414)

ansyral closed this Aug 15, 2017

ansyral reopened this Aug 15, 2017

ansyral force-pushed the eclipse branch from 4a4e4d2 to 3410106 Compare August 15, 2017 04:10

fbricon requested review from gorkem and snjeza August 17, 2017 06:46

snjeza requested changes Aug 17, 2017

View reviewed changes

ansyral force-pushed the eclipse branch 5 times, most recently from 38913f6 to 20ed6b7 Compare August 25, 2017 12:06

snjeza approved these changes Aug 28, 2017

View reviewed changes

gorkem requested changes Aug 30, 2017

View reviewed changes

ansyral force-pushed the eclipse branch 2 times, most recently from ffdd365 to fc7249c Compare September 19, 2017 02:26

ansyral mentioned this pull request Sep 19, 2017

build workspace manually redhat-developer/vscode-java#298

Closed

fbricon reviewed Sep 19, 2017

View reviewed changes

ansyral force-pushed the eclipse branch from fc7249c to faa59ff Compare September 20, 2017 03:33

fbricon reviewed Sep 21, 2017

View reviewed changes

gorkem approved these changes Sep 24, 2017

View reviewed changes

fbricon requested changes Oct 25, 2017

View reviewed changes

fbricon reviewed Oct 25, 2017

View reviewed changes

ansyral added 9 commits October 26, 2017 10:12

add command to build workspace

67a3bd5

Signed-off-by: xuzho <[email protected]>

invoke pr validator

e08fc85

Signed-off-by: xuzho <[email protected]>

add tests and fix the way to get build errors

7f34e1d

Signed-off-by: xuzho <[email protected]>

try fix test failure

97101d1

Signed-off-by: xuzho <[email protected]>

add build result details as couldn't repro locally

01bcf32

Signed-off-by: xuzho <[email protected]>

fix test failure

df1c6cc

Signed-off-by: xuzho <[email protected]>

resolve comments

29ebe20

Signed-off-by: xuzho <[email protected]>

to keep consistent, not publish diagnostics for standalone files

f7f0da1

Signed-off-by: xuzho <[email protected]>

resolve comments

436371f

Signed-off-by: xuzho <[email protected]>

ansyral force-pushed the eclipse branch from 5295d35 to 436371f Compare October 26, 2017 06:14

ansyral mentioned this pull request Oct 30, 2017

Should not start debug when build is not done or is failed microsoft/vscode-java-debug#32

Closed

fbricon reviewed Oct 30, 2017

View reviewed changes

fbricon mentioned this pull request Oct 31, 2017

Add command to build workspace #414

Merged

fbricon closed this Oct 31, 2017

add command to build workspace #326

add command to build workspace #326

Conversation

ansyral commented Aug 14, 2017

eclipse-ls-bot commented Aug 14, 2017

ansyral commented Aug 14, 2017

eclipse-ls-bot commented Aug 15, 2017

ansyral commented Aug 17, 2017

snjeza left a comment

Choose a reason for hiding this comment

gorkem commented Aug 17, 2017

ansyral commented Aug 18, 2017 • edited Loading

ansyral commented Aug 23, 2017

snjeza commented Aug 23, 2017

ansyral commented Aug 24, 2017

gorkem commented Aug 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fbricon commented Sep 13, 2017

ansyral commented Sep 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ansyral commented Sep 20, 2017 • edited Loading

Choose a reason for hiding this comment

fbricon commented Sep 21, 2017

fbricon commented Sep 21, 2017

ansyral commented Sep 22, 2017

ansyral commented Oct 25, 2017

fbricon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fbricon commented Oct 26, 2017

fbricon commented Oct 27, 2017

ansyral commented Oct 30, 2017

Choose a reason for hiding this comment

fbricon commented Oct 31, 2017

ansyral commented Aug 18, 2017 •

edited

Loading

ansyral commented Sep 20, 2017 •

edited

Loading