Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop generating metadata files at project's root #1900

Merged
merged 6 commits into from
Nov 24, 2021

Conversation

jdneo
Copy link
Contributor

@jdneo jdneo commented Oct 9, 2021

resolve redhat-developer/vscode-java#618, requires redhat-developer/vscode-java#2153

There is a setting called java.import.generatesMetadataFilesAtProjectRoot to control whether the project metadata files will be generated at project root. By default it's false, meaning those files will be hidden.

Some known issues I found:
1. Buildship hard code the .settings/ folder location: eclipse/buildship#1111
2. When calling .getLocation() from the metadata file(i.e. project.getFile(".project").getLocation()), the file location always points to the project root. This is because the upstream implementation does not depend on FileStore to resolve the location, see FileSystemResourceManager#locationFor(). But getLocationURI() works fine.

Workaround: use FileUtil.toPath(xxx.getLocationURI()) in our implementation.

Signed-off-by: Sheng Chen [email protected]

@jdneo

This comment has been minimized.

@jdneo

This comment has been minimized.

@jdneo jdneo marked this pull request as ready for review November 9, 2021 05:07
@jdneo jdneo changed the title [WIP] Stop generating metadata files at project's root Stop generating metadata files at project's root Nov 9, 2021
Signed-off-by: Sheng Chen <[email protected]>
Signed-off-by: sheche <[email protected]>
Copy link
Contributor

@rgrunber rgrunber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change is working well for me. Played around with java.settings.url outside workspace, and within, and couldn't see any breakage.

@guw
Copy link
Contributor

guw commented Nov 20, 2021

There is a setting called java.import.generatesMetadataFilesAtProjectRoot to control whether the project metadata files will be generated at project root. By default it's false, meaning those files will be hidden.

That's currently a system property. It should be more configurable. We have a large source code base which is modified by many different editors/IDEs. This change would break VS Code users unless the setting is detected somehow at startup/during import.

@jdneo
Copy link
Contributor Author

jdneo commented Nov 21, 2021

@guw Because the file system contribution will take effect at very begin before the server and client start communication via LSP. So it is a system property instead of a preference.

I don't think the change will break the users because we considered the compatibility in implementation:

  1. If the system property is not set, those metadata files will be generated at the project root.
  2. If the metadata files exist both in project root and the workspace storage, the files at the project root win.

In the code base you mentioned:

  1. If those metadata files are added to .gitignore, that's not a problem because the metadata files won't exist in the code base, no matter what tools are used by users and how the tools generates metadata files, that only happens locally and won't affect anything in the code base.
  2. If those metadata files are committed into the code base. Then the VS Code Java will use them, which means the behavior is still the same as before.

If in your implementation, you want to deal with those metadata files. You can use the standard API to do it. For example if you want to update the project nature, use: project.getDescription().setNatureIds(xxx). Just don't hard code the path and directly update it via file IO.

If you do want to get the file instance of the metadata files, for example, for the .project, use: FileUtil.toPath(project.getFile(IProjectDescription.DESCRIPTION_FILE_NAME).getLocationURI()).toFile()

Copy link
Contributor

@testforstephen testforstephen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Have a try, it works with Maven/Gradle/Invisible project.

@rgrunber
Copy link
Contributor

Feel free to combine the commits and merge when ready.

@jdneo jdneo merged commit ecb29b4 into eclipse-jdtls:master Nov 24, 2021
@jdneo jdneo deleted the cs/metadata branch November 24, 2021 04:33
@testforstephen testforstephen added this to the Mid November milestone Nov 24, 2021
crabbkw added a commit to crabbkw/lsp-java that referenced this pull request Jul 31, 2024
lsp-java used to treat this as a configuration property of jdtls.
However it is not a standard lsp configuration property. Instead it
must be passed as a Java System Property on the command line invoking
jdtls. See
[JLFsUtils.java](https://github.com/eclipse-jdtls/eclipse.jdt.ls/blob/master/org.eclipse.jdt.ls.filesystem/src/org/eclipse/jdt/ls/core/internal/filesystem/JLSFsUtils.java#L192-L197)
for how this property is read. Also see
[eclipse-jdtls/eclipse.jdt.ls#1900](eclipse-jdtls/eclipse.jdt.ls#1900 (comment))
for an explanation of why this is a system property and not a regular
LSP configuration property.

Use a regular `defcustom` instead of using `lsp-defcustom` for
declaring lsp-java-import-generates-metadata-files-at-project-root.
Use this defcustom variable lsp-java--ls-command to pass an
appropriate value for
`java.import.generatesMetadataFilesAtProjectRoot`.

With this change importing a Gradle-based java project into LSP will
nolonger create `.classpath`, `.settings`, and `.project` entries at
the root of the project.
crabbkw added a commit to crabbkw/lsp-java that referenced this pull request Jul 31, 2024
lsp-java used to treat this as a configuration property of jdtls.
However it is not a standard lsp configuration property. Instead it
must be passed as a Java System Property on the command line invoking
jdtls. See
[JLFsUtils.java](https://github.com/eclipse-jdtls/eclipse.jdt.ls/blob/master/org.eclipse.jdt.ls.filesystem/src/org/eclipse/jdt/ls/core/internal/filesystem/JLSFsUtils.java#L192-L197)
for how this property is read. Also see
[eclipse-jdtls/eclipse.jdt.ls#1900](eclipse-jdtls/eclipse.jdt.ls#1900 (comment))
for an explanation of why this is a system property and not a regular
LSP configuration property.

Use a regular `defcustom` instead of using `lsp-defcustom` for
declaring lsp-java-import-generates-metadata-files-at-project-root.
Use this defcustom variable in `lsp-java--ls-command` to pass an
appropriate value for
`java.import.generatesMetadataFilesAtProjectRoot`.

With this change importing a Gradle-based java project into LSP will
nolonger create `.classpath`, `.settings`, and `.project` entries at
the root of the project.
crabbkw added a commit to crabbkw/lsp-java that referenced this pull request Jul 31, 2024
lsp-java used to treat this as a configuration property of jdtls.
However it is not a standard lsp configuration property. Instead it
must be passed as a Java System Property on the command line invoking
jdtls. See
[JLFsUtils.java](https://github.com/eclipse-jdtls/eclipse.jdt.ls/blob/master/org.eclipse.jdt.ls.filesystem/src/org/eclipse/jdt/ls/core/internal/filesystem/JLSFsUtils.java#L192-L197)
for how this property is read. Also see
[eclipse-jdtls/eclipse.jdt.ls#1900](eclipse-jdtls/eclipse.jdt.ls#1900 (comment))
for an explanation of why this is a system property and not a regular
LSP configuration property.

Use a regular `defcustom` instead of using `lsp-defcustom` for
declaring lsp-java-import-generates-metadata-files-at-project-root.
Use this defcustom variable in `lsp-java--ls-command` to pass an
appropriate value for
`java.import.generatesMetadataFilesAtProjectRoot`.

With this change importing a Gradle-based java project into LSP will
nolonger create `.classpath`, `.settings`, and `.project` entries at
the root of the project.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Any way to get rid of .project .classpath .settings/ files?
4 participants