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

Enhanced Referenced Library Support Proposal #1257

Closed
Vigilans opened this issue Nov 10, 2019 · 3 comments · Fixed by #1267
Closed

Enhanced Referenced Library Support Proposal #1257

Vigilans opened this issue Nov 10, 2019 · 3 comments · Fixed by #1267

Comments

@Vigilans
Copy link
Contributor

Introduction

Hello, I am seeking to provide a mechanism for adding external local dependencies to a project in recent days, which solves feature requests like microsoft/vscode-java-pack#94 and microsoft/vscode-java-dependency#174.

This feature would bring some notable benefits:

  • More friendly to beginners. In my university years, most of my classmates get to study Java with neither Maven nor Gradle, and they were going a clumsy way to resolve dependency: download a fat jar to somewhere even not necessarily in the project, and reference them by IDE.

    Eclipse provides Referenced Library and User Library to deal with it. If editors with language client (e.g. VS Code) could also provide a neat way to introduce local libaries (and help to convert to recommended practices), they would look less dauting to the beginners.

  • Complete a missing piece of project dependency management. There are now good supports for resolving (local) depedencies for Maven and Gradle projects. By adding support for Eclipse-based project, we could group them into a set of comprehensive commands, each command dispatching dependency requests to the handlers according to project types.

In #927, @fbricon introduced automatic detection of jars in lib/ folder. This is a nice PR, in that it implements a infrastructure (job queue and request merging, etc.) for updating classpathes.

However, this feature is somewhat restrictive and hard to get noticed as its logic is implicit. Turning it to a setting would make it more easy to be noticed and managed by users.

So, based on these ideas and previous works, my goal for this feature is to: extend the pre-defined lib folder to use an array of glob pattern to define referenced jars.

@Vigilans
Copy link
Contributor Author

Vigilans commented Nov 10, 2019

Discussion

Implement in Server Side

At first, I implemented a prototype for this feature in jdt.ls. I tried in server side because I thought it would bring me some benefits like: (Pros)

  • Easy to reuse and implement as we are merely upgrading lib/** to any globs.
  • Every language clients could benifit from it once the feature is done.

However, after the prototype was finished, I found out that it brought me some pains and just kept me away from adopting this choice: (Cons)

  • Poor support of glob in Java.

    I've done a bunch of investigations (seriously!), but could not find any mature package that could rival node-glob. The paradigms for glob used for Java never cared non-wildcard prefix optimization, nor did they care a real path with no wildcard.

    It means that if an absolute path is provided, those paradigms would have to search the root folder (/) down to find the file, rather than just fs.stat it.

  • Inability to watch changes of files outside of workspace.

    This is a more critical problem. Take VS Code as example:

    1. jdt.ls relies on vscode-language-server-node to watch file changes.
    2. vscode client uses workspace.createFileSystemWatcher for watching files
    3. createFileSystemWatcher explicitly stated that it will only watch files inside workspace.

    So, jdt.ls won't be able to receive workspace/onDidChangeWatchedFiles event when the file is from outside the workspace, thus not being able to update the dependency.

    It is not an unusual usecase. For my beginner classmates, they will often reference jar files in somewhere outside of workspace by just add an absolute path to them, bothing in java and cpp.

Therefore, these issues just blocks my way to use the server side proposal as final proposal.

@Vigilans
Copy link
Contributor Author

Vigilans commented Nov 10, 2019

Discussion

Implement in Client Side

Finally I decided to move fs-related logic to client side, leaving jdt.ls to only serve as a thin API provider for updating dependency files.

Pros:

  • Good and mature library support: chokidar for watching and node-glob for searching.
  • Decoupled with jdt.ls, making the feature more simple to be implemented.

Cons:

  • Will it break other clients' current behavior?

    Currently, all other language clients will be able to watch ${workspace}/lib folder by default.

    According to the specified requirements, there is only refactoring work to be done: separate fs-related logic from dependency updating jobs. It will simply keep current server's lib/** watching behavior unchanged, and make the structure more clean and compatible.

  • Other clients may not benefit from this feature...

    At the beginning, I thought it is indeed a regret, but as the development went on, I started to notice that the refactoring work is in truth enabling the clients to easily utilize the feature by providing a general interface to implement.

    By refactoring the server, it also grants the clients more flexibilty on their decisions to choose what to include, exclude, as well as custom source mapping.

All above analysis are summarized into following final proposal.

@Vigilans
Copy link
Contributor Author

Vigilans commented Nov 10, 2019

Proposal

Here's the proposal I've worked out until now:

Language Server

  1. Restrict UpdateBinaries to only updating dependency file by moving fs-related logic out:

    • ProjectUtils.collectBinaries

    • ProjectUtils.detectSources

  2. Change UpdateBinaries from accepting a lib folder to a map of binary path to source path:

    • Before: updateBinaries(..., IPath libFolderPath, ...)

    • After: updateBinaries(..., Map<String, String> libraries, ...)

  3. Refactor UpdateClasspathJob by providing a general interface:

    new UpdateClassPathJob(
       /* IJavaProject */ project, // target project to update dependency
       /* List<String> */ include, // for jars to be added
       /* List<String> */ exclude, // for jars to be ignored
       /* Map<String, String> */ sources, // path mapping from bin jar to src jar
    );
  4. Provide adaptor interface for compatbility with lib/ folder detection:

    public void updateClasspath(IJavaProject project, IPath libFolderPath, IProgressMonitor monitor) throws CoreException {
        Set<Path> binaries = ProjectUtils.collectBinaries([libFolderPath]);
        Set<String> includeBinaries = binaries.map(Path::toString);
        Map<String, String> sources = binaries.collect(toMap(Path::toString, ProjectUtils::detectSources));
        updateClasspath(project, includeBinaries, null, sources);
    }

Language Client

  1. VS Code will take care of the file watching and searching work by providng a setting:

    "java.dependency.referencedLibraries": string[] | {
      "include": string[],
      "exclude": string[],
      "sources": { [binary: string]: string }
    }

    ...with the following extended behaviors before sending to jdt.ls:

    • If specified setting is an array, it will be expanded into a full object, with the original content as include field.
    • Jars in lib/ but not included will be appended to exclude field. It is to ensure jars not listened by VS Code don't get shown in referenced libraries — only one source of truth. (This is not an elegent workaround, waiting for better alternative...)
    • A match of foo.jar and foo-src.jar (not necessarily in the same folder) will be added to sources mapping.
  2. Enabling users to resolve local dependency by providing an entry command:

    • Dispatch the requests to handlers according to the project type (Maven, Gradle, Invisible...);
    • Update the dependency according to each project type's own handling logic;
    • Help upgrade the to recommended practice (Use maven central, copy to lib/ folder, etc.)
    • ...

In this way, an entrance to resolving dependencies is established. Then we could decorate it with some upper level UI to make this feature more user friendly, which is a complete client-side issue.

Looking for advice within the issue, and from the Monday sync with vscode-java team.

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

Successfully merging a pull request may close this issue.

2 participants