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

NB Build: Basic support for javac.release property. #7188

Merged
merged 1 commit into from
Apr 8, 2024

Conversation

mbien
Copy link
Member

@mbien mbien commented Mar 25, 2024

  • custom javac and sigtest tasks needed updates
  • JarWithModuleAttributes too, which sets the OpenIDE-Module-Java-Dependencies manifest attribute based on the language level
  • ant nbm projects (apisupport.ant) needed to be made aware of the javac release property

apisupport.ant of NB 21 loads all projects which use javac.release as java 1.4 projects, if we get this into NB 22 and devs start using the new release, we can start swapping out source/target with release during the NB 23 dev cycle.

@mbien mbien added build ci:all-tests [ci] enable all tests labels Mar 25, 2024
@mbien mbien force-pushed the basic-javac-release-support branch from 69b2833 to e73d553 Compare March 25, 2024 18:01
@mbien mbien force-pushed the basic-javac-release-support branch 2 times, most recently from 646faef to ffead33 Compare March 25, 2024 19:03
@mbien
Copy link
Member Author

mbien commented Mar 25, 2024

all green except paperwork
edit: also fixed

@mbien mbien removed the ci:all-tests [ci] enable all tests label Mar 25, 2024
@mbien mbien force-pushed the basic-javac-release-support branch from ffead33 to 646ff4f Compare March 25, 2024 20:35
@mbien mbien added work-in-progress Ant [ci] enable "build tools" tests labels Mar 25, 2024
@mbien mbien force-pushed the basic-javac-release-support branch from 646ff4f to f14c088 Compare March 25, 2024 22:37
@mbien mbien added ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) and removed work-in-progress labels Mar 27, 2024
@mbien mbien force-pushed the basic-javac-release-support branch from f14c088 to 5a446b7 Compare March 27, 2024 13:26
@mbien mbien marked this pull request as ready for review March 27, 2024 18:41
@mbien mbien added this to the NB22 milestone Mar 27, 2024
@mbien mbien added the do not merge Don't merge this PR, it is not ready or just demonstration purposes. label Mar 27, 2024
 - javac release option support for Ant NBM projects
 - update custom javac and sigtest tasks
@mbien mbien force-pushed the basic-javac-release-support branch from 5a446b7 to 0a64c81 Compare April 5, 2024 18:35
@mbien mbien removed the do not merge Don't merge this PR, it is not ready or just demonstration purposes. label Apr 5, 2024
@mbien
Copy link
Member Author

mbien commented Apr 5, 2024

this PR now only contains the changes to the build system and is targeting NB 22. Part 2 for NB 23 would be in #7247

Copy link
Contributor

@lahodaj lahodaj left a comment

Choose a reason for hiding this comment

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

Looks sensible to me.

@mbien
Copy link
Member Author

mbien commented Apr 8, 2024

gave this a bit more testing:

  • the right bytecode level is produced
  • no errors in the editor while using release=17 and jdk 17 API (means that the property is picked up correctly)
  • value can be modified using project UI

this however did not work:

  • when using JDK 17 API but setting release to 11, the editor did not show the API usage as error even though the build fails

this likely indicates that the editor is using source still. I have to take another look if I can implement this too, however even if not I think we should merge this as first step.

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.

I think it's Ok to merge, and fix the API usage later on. It affects the module developers only. Speaking of me I can live with that.

@mbien
Copy link
Member Author

mbien commented Apr 8, 2024

I tracked it down to this which returns null:

String useRelease = useRelease(sourceLevel, validatedSourceLevel);

if (validatedSourceLevel.equals(sourceLevel)) {
return null;
}

because validatedSourceLevel is the same as the requested source level. If I let it return the requested source level instead of null, the javac task will set relaese and there will be error badges, however this causes exceptions somewhere else.

Lets not do that. Going to merge, thanks for the reviews.

@mbien mbien merged commit 87016c5 into apache:master Apr 8, 2024
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ant [ci] enable "build tools" tests build ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants