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

Keep the temp init scripts on disk #2249

Merged
merged 1 commit into from
Sep 28, 2022

Conversation

jdneo
Copy link
Contributor

@jdneo jdneo commented Sep 28, 2022

  • The temp files will not be deleted on JVM exist, since it will cause buildship fail when get Eclipse project instance after the server restarts.
  • Cache the temp files for each session, to make sure there will not be too many temp files generated when reloading projects.

related with redhat-developer/vscode-java#2692

Signed-off-by: Sheng Chen sheche@microsoft.com

- The temp files will not be deleted on JVM exist, since it will cause
  buildship fail when get Eclipse project instance after the server restarts.
- Cache the temp files for each session, to make sure there will not be
  too many temp files generated when reloading projects.

Signed-off-by: Sheng Chen <sheche@microsoft.com>
@jdneo
Copy link
Contributor Author

jdneo commented Sep 28, 2022

test this please

@jdneo
Copy link
Contributor Author

jdneo commented Sep 28, 2022

test this please

@jdneo
Copy link
Contributor Author

jdneo commented Sep 28, 2022

@rgrunber Not sure why the test case is failing. Could we include this change in this release? It makes sure the gradle project reloading will not fail due to file not exists.

@testforstephen
Copy link
Contributor

@testforstephen
Copy link
Contributor

test this please

@jdneo jdneo added this to the End September milestone Sep 28, 2022
@jdneo jdneo added the bug label Sep 28, 2022
@testforstephen
Copy link
Contributor

testforstephen commented Sep 28, 2022

https://github.com/eclipse/eclipse.jdt.ls/blob/d65385a0d09df1c3a904a6644efe1b0c1a7860d0/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/managers/GradleProjectImporter.java#L631-L640

A different approach I was thinking of is that we could create a fixed temporary file for an identical scriptPath. For example, you could calculate the md5 value of the scriptPath and create a file in the tmp directory using md5 as the filename. This way you avoid creating many temporary files for the init scripts, and you don't need to delete them as well.

@rgrunber
Copy link
Contributor

I think it makes sense to accept this PR as-is to at least improve the situation prior to releasing. From there we can further improve this.

doing a checksum on the content of the init script(s) would definitely avoid creating duplicates.

@rgrunber rgrunber merged commit 372989c into eclipse-jdtls:master Sep 28, 2022
@jdneo jdneo deleted the cs/issue-2692 branch September 29, 2022 00:48
@jdneo
Copy link
Contributor Author

jdneo commented Sep 29, 2022

Sounds good. I can do a checksum for the file content, then it will only generate new files when the content of the script is changed.

Meanwhile, checksum can be used to check the integrity of the script for security concern

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants