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

Download sources for the masses #1628

Merged
merged 1 commit into from
Jan 15, 2021

Conversation

rgrunber
Copy link
Contributor

@rgrunber rgrunber commented Dec 9, 2020

Signed-off-by: Fred Bricon fbricon@gmail.com
[rgrunber@redhat.com]: Fix testDynamicSourceLookups()

  • Move testcase from HoverHandlerTest to InvisibleProjectBuildSupportTest
  • Set up the external library after project import to mimic approach
    in other invisible projects
    Signed-off-by: Roland Grunberg rgrunber@redhat.com

@fbricon

Based on #1580 .

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.

Tried it in an unmanaged folder (invisible project), the downloader works.
sourcedownload

Need a PR in the client side to expose the new preference key. Others look good to me.

@testforstephen testforstephen added this to the Mid January 2021 milestone Jan 7, 2021
@fbricon
Copy link
Contributor

fbricon commented Jan 7, 2021

IIRC there's one case that doesn't work well: Take an invisible project with 2 jars in lib/ , if you hover over a class from jar 1, source is downloaded, classpath is updated with path to sources of jar 1, if you hover over a class from jar 2, source is downloaded, classpath is updated with path to sources of jar 2, but sourcepath of jar 1 is lost

@rgrunber
Copy link
Contributor Author

rgrunber commented Jan 8, 2021

IIRC there's one case that doesn't work well: Take an invisible project with 2 jars in lib/ , if you hover over a class from jar 1, source is downloaded, classpath is updated with path to sources of jar 1, if you hover over a class from jar 2, source is downloaded, classpath is updated with path to sources of jar 2, but sourcepath of jar 1 is lost

I could have a look and see what would be involved in fixing. I'd lean towards filing a separate bug for this as it would be good to get some initial support in for mid-January.

UPDATE : @fbricon I tried a simple invisible project with 2 jars from maven central in lib/ and a source file with references to types from those jars. Whether I hovered/go-to defition on one and then the other while watching the hidden .classpath file and the classpath 'lib' entries were updated with a sourcepath to the local repository location and it never went away.

- Fixes redhat-developer/vscode-java#1664
- Download maven-central artifact's sources on-demand

Signed-off-by: Fred Bricon <fbricon@gmail.com>
[rgrunber@redhat.com]: Fix testDynamicSourceLookups()
- Move testcase from HoverHandlerTest to InvisibleProjectBuildSupportTest
- Set up the external library after project import to mimic approach
  in other invisible projects
- Create IMavenArtifactIdentifier to be implemented by
  MavenPropertiesIdentifier and MavenCentralIdentifier
Signed-off-by: Roland Grunberg <rgrunber@redhat.com>
@testforstephen
Copy link
Contributor

UPDATE : @fbricon I tried a simple invisible project with 2 jars from maven central in lib/ and a source file with references to types from those jars. Whether I hovered/go-to defition on one and then the other while watching the hidden .classpath file and the classpath 'lib' entries were updated with a sourcepath to the local repository location and it never went away.

I also tried an invisible project with 2 jars, and the source path metadata didn't disappear while hovering over the class from another jar.

@rgrunber
Copy link
Contributor Author

As redhat-developer/vscode-java#1758 is ready for submission and this change is also now in good shape, I'll be pushing these in by the end of today.

@rgrunber rgrunber merged commit 6dd38fa into eclipse-jdtls:master Jan 15, 2021
@rgrunber rgrunber deleted the universal-source-downloader branch January 15, 2021 17:49
@rgrunber
Copy link
Contributor Author

rgrunber commented Jan 18, 2021

Ok, so @fbricon clarified a possible reproducer and seems specific to invisible projects.

  1. Set up the invisible project (include a jar in the lib/ folder)
  2. Update the project classpath by adding a second jar to the lib folder

The first jar's source classpath should disappear likely due to a bug with the invisible project's classpath updating logic.

I haven't tested this out, but if so, I can file a bug for this.

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.

support download of sources for classes in jar with maven coordinates
3 participants