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

API: access to build properties of gradle script [2/3] #4729

Merged
merged 3 commits into from
Oct 5, 2022

Conversation

sdedic
Copy link
Member

@sdedic sdedic commented Oct 3, 2022

Important Note: this is 2nd from 3 dependent PRs, depends on #4726. For review purposes, the PR targets a feature branch in the shared repository, that will be deleted after the related PRs are merged - it allows to inspect/review changes before the base PR (#4726) is merged. The time is becoming a little issue for me :)

Traditionally a module that need information from the gradle build system other than collected by the gradle core and exposed would have to add its 'agent' into netbeans-gradle-tooling subproject, then implement ProjectInfoExtractor.

This PR allows NetBeans modules to inspect gradle build properties. Clients may even adapt to changes by listening on Gradle project's reload event. The Gradle core module loads a lot more properties from the project model in Gradle process and marshall the information to the IDE. The introduced API allows to inspect individual properties, enumerate lists or access map keys.

NetBeans modules do not need to implement a tooling agent that their ProjectInfoExtractor interfaces with, and in fact they do not need the Extractor at all - unless they need to expose a service in projectLookup. I would consider the exact format exchanged between NetBeans gradle tooling (gradle) plugin and the BuildPropertiesSupport implementation private: although the data can be read by ProjectInfoExtractors, the intention is to hide the exact wire format behind the API.

The last PR in this group adaps java.gradle and micronaut modules to declare their file products (artifacts) based on values inspected from the gradle build script.

@sdedic sdedic added enhancement API Change [ci] enable extra API related tests Gradle [ci] enable "build tools" tests labels Oct 3, 2022
@sdedic sdedic self-assigned this Oct 3, 2022
@sdedic
Copy link
Member Author

sdedic commented Oct 3, 2022

Note: there was a concern that introspecting the buildscript will activate potentially harmful/long-running user code in Tasks. The fact is that data collector for Navigator window contents, that displays tasks performs task instantiation already. So tasks area created (and user code is potentially) for som years already.

Copy link
Contributor

@lkishalmi lkishalmi left a comment

Choose a reason for hiding this comment

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

Well, I still have mixed feelings about this one.
There are plenty of good innovation in this. I just have an odd feeling mixing NbProjectInfo with the new stuff.

I also had to admit, that I couldn't offer anything better at the moment, at least not without putting substantial effort in it. I always say the ugliest part of my NetBeans Gradle implementation is how NetBeans is fetching the data from Gralde. Enhancing the NbProjectInfBuilder makes it worse. Adding some generalization on the type serialization makes it better.

I wish one day my old serialization will go and be replaced with something better. Maybe this PR will lead on that direction, maybe we have to rethink how we shall do this again, though at the moment I shall bow my head and let this one happen for NB16.

Thanks!

@sdedic
Copy link
Member Author

sdedic commented Oct 4, 2022

Wouldn't it help if the 'introspection' is separated - i.e. in a separate helper class ? It does not actually solve the 'ugliness', the ad-hoc Map-property "protocol", but might be more readable.

@sdedic
Copy link
Member Author

sdedic commented Oct 4, 2022

@lkishalmi could you, please, also review the prerequisite PR #4726 ?

@sdedic sdedic changed the base branch from sdedic/gradle-fixes1 to master October 4, 2022 21:11
@sdedic
Copy link
Member Author

sdedic commented Oct 5, 2022

Rebased on latest master (with the prerequisite PR merged in). Added basic tests for build properties access, fixed bugs :)

@sdedic sdedic added the Micronaut [ci] enable enterprise job label Oct 5, 2022
@sdedic sdedic merged commit 4e81d3a into apache:master Oct 5, 2022
@neilcsmith-net neilcsmith-net added this to the NB16 milestone Oct 18, 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 Gradle [ci] enable "build tools" tests Micronaut [ci] enable enterprise job
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants