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

Initial implementation of Project Dependency API #4149

Merged
merged 10 commits into from May 27, 2022

Conversation

sdedic
Copy link
Member

@sdedic sdedic commented May 25, 2022

We have API to get inter-project dependencies, but we do not have an API to get a view on dependencies incl. external resources. There's a possibility to get a ClassPath (for Java) enumerating all necessary resources as a flat list, but if one would like to get structure information ... there's probably no way.

This PR is an attempt to define some abstractions, which could eventually become part of project.api. They probably won't fit to good old ANT projects (but if someone finds a suitable adjustment that would make ArtifactSpec more Ant-like, please share your thoughts !!), but for example NetBeans modules (although ant-based) could make it.

The initial goal is to provide a structured view, a dependency tree, such as maven dependency:tree or gradle dependencies. The project may start to report its product (artifact, or artifacts ?) and later a project-type-neutral way can be offered to map a resource (e.g. jar) to an artifact known to a project. All this is TBD.

The API is initially implemented as private; implementation dependency is reuquired to use/implement it. This will change after the constructs are validated on more project types.

@sdedic sdedic added enhancement API Change [ci] enable extra API related tests Maven [ci] enable "build tools" tests Platform [ci] enable platform tests (platform/*) labels May 25, 2022
@sdedic sdedic added this to the NB15 milestone May 25, 2022
@sdedic sdedic requested review from ppisl and dbalek May 25, 2022 12:18
@sdedic sdedic self-assigned this May 25, 2022
Copy link
Member

@ppisl ppisl left a comment

Choose a reason for hiding this comment

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

I went through this PR and it seems to me ok .

@NonNull String versionSpec, boolean optional, @NonNull V data) {
return new ArtifactSpec<V>(VersionKind.SNAPSHOT, groupId, artifactId, versionSpec, type, classifier, false, data);
}

Copy link
Member

Choose a reason for hiding this comment

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

And VersionKind.Latest? We don't need it now?

Copy link
Member Author

Choose a reason for hiding this comment

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

will remove. Can be added later.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed in 0221749

"vscode-debugadapter": "1.51.0",
"vscode-languageclient": "8.0.1"
"vscode-debugadapter": "1.42.1",
"vscode-languageclient": "7.0.0"
Copy link
Member

Choose a reason for hiding this comment

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

Is it intended to downgrade the versions? There are downgraded versions for more modules.

Copy link
Member Author

Choose a reason for hiding this comment

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

Unintentional - seems as leftovers from local experiments were committed, no change to vscode should be made here. Will clean up.

Copy link
Member Author

Choose a reason for hiding this comment

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

Changes removed (= same as master) in f3ee411

@junichi11
Copy link
Member

Maybe, Some files are missing the license header? (.md, .xml, .properties)

Copy link
Contributor

@dbalek dbalek left a comment

Choose a reason for hiding this comment

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

The proposed API looks sane to me.

@lkishalmi
Copy link
Contributor

It's an interesting start. Too "mavenly" though at the moment. I'd let give it a go, though when applying to Gradle there would be some interesting questions how would could we handle includedBuild substitutes, and different outgoing variants. Also the question is there, should we handle these at all?

@sdedic
Copy link
Member Author

sdedic commented May 27, 2022

It's an interesting start. Too "mavenly" though at the moment. I'd let give it a go, though when applying to Gradle there would be some interesting questions how would could we handle includedBuild substitutes, and different outgoing variants. Also the question is there, should we handle these at all?

I guess I may need your expertise in Gradle area here. Just OTOH (given the API), we might not even care, as the includedBuild (if I understood correctly) just replaces dependency with project dependency, but the project product artifact is still there. The dependency graph is also a snapshot of the final artifact resolution, so (IMHO) no change with included builds.

I thouhgt about adding extension services for the additional queries (e.g. one could determine build segments / included builds), which could be project-type-dependent or (if we find a suitable abstraction) project independent - that's why raw (unspecified) data is exposed from ArtifactSpec / Dependency. I thought about adding a Lookup.Provider to the DependencyResult to offer snapshot-bound additional services; but I was not able to see the details at the time of design.

BTW - maybe the Maven Graph could be adapted to this API to see graph dependency for Gradle too ;)

@sdedic
Copy link
Member Author

sdedic commented May 27, 2022

Just unreliable tests fail, merging.

@sdedic sdedic merged commit ea62da4 into apache:master May 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Change [ci] enable extra API related tests enhancement Maven [ci] enable "build tools" tests Platform [ci] enable platform tests (platform/*)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants