RFR: 8295884: Implement IDE support for Eclipse [v39]

[Erik Joelsson]

On Tue, 7 Mar 2023 16:44:26 GMT, Julian Waters <jwaters at openjdk.org> wrote:

>> Eclipse is a popular and very well-known IDE in the world of Java development, utilized widely in many contexts, by beginners and experienced teams alike. Although a relatively lightweight IDE, it features surprisingly powerful indexing and code analysis capabilities, as well as useful tools, among which are make integration. While the tools it provides are not always as sophisticated as other IDEs (IntelliJ IDEA will likely come to mind as one such competitor), the simplicity of using it, as well as the reliability of this rugged IDE makes up greatly for the slightly less advanced tooling. Eclipse requires very little starting infrastructure in the workspace for all these features and indexing support as well, which makes it a good candidate for developing on the JDK.
>> 
>> This enhancement adds 4 extra targets to the make system for generating a basic Eclipse Workspace that provides almost full indexing support for the JDK, with varying levels as desired, from a minimalistic option only including the Java Virtual Machine's source code, to generating a workspace with both Java and C/C++ natures included, which allows for using Eclipse's unique ability to quickly swap between Java and C/C++ mode to work on both native and Java sources at the same time. Cross Compiling support is available, and in its entirety the change touches very little of the existing make system, barring its own Makefile within the ide subdirectory.
>> 
>> Indexing capabilities utilizing the enhancement:
>> <img width="960" alt="java" src="https://user-images.githubusercontent.com/32636402/197784819-67ec7de4-7e27-4f33-b738-59b75a9e4403.PNG">
>> <img width="787" alt="escape" src="https://user-images.githubusercontent.com/32636402/197784843-df8621a8-7b0a-42da-86f4-3afb92de0f71.PNG">
>
> Julian Waters has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Address Review and add extra comments

Functionally this looks ok to me, just some comment on cosmetics.

doc/ide.md line 94:

> 92: indexing the Java sources in the JDK (see below), is to enable dark mode
> 93: before doing so. Trust us, it looks much better than Eclipse's default look
> 94: and feel. ;)

Does this change how Eclipse functions with our sources in any way, or is it just your opinion that it looks better? If the latter, then I don't think it really belongs in this document.

make/ide/eclipse/CreateWorkspace.gmk line 40:

> 38: include hotspot/lib/JvmFlags.gmk
> 39: 
> 40: # Warning: This file does not have the best formatting!

Is this still true? If so, we should fix that so the comment can be removed.

After having read through the file. I find quite a few places where broken up lines have 2 space indentation instead of 4. (See https://openjdk.org/groups/build/doc/code-conventions.html) Fixing this should be pretty quick and simple and will greatly improve readability at least for me and Magnus. I will mark some of the typical cases for you.

make/ide/eclipse/CreateWorkspace.gmk line 60:

> 58: # Taken from JdkNativeCompilation.gmk
> 59: FindJavaHeaderDir = \
> 60:   $(if $(strip $1),$(wildcard $(SUPPORT_OUTPUTDIR)/headers/$(strip $1)))

Indent 4.

make/ide/eclipse/CreateWorkspace.gmk line 64:

> 62: JAVA_DIRS := $(strip $(foreach module, $(call FindAllModules), \
> 63:   $(patsubst $(TOPDIR)/%,%,$(filter-out $(OUTPUTDIR)%, \
> 64:   $(call FindModuleSrcDirs, $(module))))))

Indent 4.

make/ide/eclipse/CreateWorkspace.gmk line 134:

> 132:     # This is an annoying bug that has not been fixed for some time now
> 133:     $1_CLASSPATH += $$(foreach src,$(JAVA_DIRS), \
> 134:       <classpathentry excluding="module-info.java|module-info.java.extra" kind="src" path="$$(src)"/>$$(NEWLINE))

Indent 4.

make/ide/eclipse/CreateWorkspace.gmk line 141:

> 139:       REPLACEMENTS := \
> 140:           @@CLASSPATH@@ => $$($1_CLASSPATH), \
> 141:     ))

Indent 4.

make/ide/eclipse/CreateWorkspace.gmk line 154:

> 152: 
> 153:     $1_BUILD_MANAGERS += \
> 154:       <buildCommand> \

Indent 4.

make/ide/eclipse/CreateWorkspace.gmk line 168:

> 166: 
> 167:     $1_NATURES += \
> 168:       <nature>org.eclipse.cdt.core.cnature</nature> \

Indent 4.

make/ide/eclipse/CreateWorkspace.gmk line 233:

> 231:           @@SRC@@ => $$($1_NATIVE_SRCS) ; \
> 232:           @@MAKE_TARGETS@@ => $$($1_MATCHING_MAKE_TARGETS), \
> 233:     ))

Suggestion:

    $$(eval $$(call SetupTextFileProcessing, $1_CREATE_NATIVE_FILE, \
        SOURCE_FILES := $(TOPDIR)/make/ide/eclipse/native.template, \
        OUTPUT_FILE := $$($1_NATIVE_FILE), \
        REPLACEMENTS := \
            @@DIR@@ => $$(call FixPath, $(TOPDIR)) ; \
            @@ENV@@ => $$($1_ENV) ; \
            @@WORKSPACE@@ => $$($1_WORKSPACE_MAJOR) ; \
            @@MINOR@@ =>  $$($1_WORKSPACE_MINOR) ; \
            @@MAKE@@ => $$($1_MAKE) ; \
            @@SRC@@ => $$($1_NATIVE_SRCS) ; \
            @@MAKE_TARGETS@@ => $$($1_MATCHING_MAKE_TARGETS), \
    ))

make/ide/eclipse/CreateWorkspace.gmk line 315:

> 313:           @@CSETTINGS@@ => $$($1_CSETTINGS) ; \
> 314:           @@CXXSETTINGS@@ => $$($1_CXXSETTINGS), \
> 315:     ))

Suggestion:

    $$(eval $$(call SetupTextFileProcessing, $1_CREATE_SETTINGS_FILE, \
        SOURCE_FILES := $(TOPDIR)/make/ide/eclipse/settings.template, \
        OUTPUT_FILE := $$($1_SETTINGS_FILE), \
        REPLACEMENTS := \
            @@WORKSPACE@@ => $$($1_WORKSPACE_MAJOR) ; \
            @@CSETTINGS@@ => $$($1_CSETTINGS) ; \
            @@CXXSETTINGS@@ => $$($1_CXXSETTINGS), \
    ))

make/ide/eclipse/CreateWorkspace.gmk line 364:

> 362:         @@NATURES@@ => $$($1_NATURES) ; \
> 363:         @@LINKED_RESOURCES@@ => $$($1_LINKED_RESOURCES), \
> 364:   ))

Suggestion:

  $$(eval $$(call SetupTextFileProcessing, $1_CREATE_WORKSPACE_FILE, \
      SOURCE_FILES := $(TOPDIR)/make/ide/eclipse/workspace.template, \
      OUTPUT_FILE := $$($1_WORKSPACE_FILE), \
      REPLACEMENTS := \
          @@BUILD_MANAGERS@@ => $$($1_BUILD_MANAGERS) ; \
          @@NATURES@@ => $$($1_NATURES) ; \
          @@LINKED_RESOURCES@@ => $$($1_LINKED_RESOURCES), \
  ))

make/ide/eclipse/CreateWorkspace.gmk line 390:

> 388:   SHARED := $(SHARED), \
> 389: ))
> 390: endif

The lack of indentation makes this hard to read. This is how I would like the first block to look:

ifeq ($(WORKSPACE), java)
  $(eval $(call SetupEclipseWorkspace, SETUP_WORKSPACE, \
      NATURE := JAVA, \
      SHARED := $(SHARED), \
  ))
else ...

-------------

PR: https://git.openjdk.org/jdk/pull/10853