You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by "jtulach (via GitHub)" <gi...@apache.org> on 2023/03/05 05:04:01 UTC
[GitHub] [netbeans] jtulach opened a new pull request, #5609: nb-javac: "Eat your own dog food" testing
jtulach opened a new pull request, #5609:
URL: https://github.com/apache/netbeans/pull/5609
With #5550 in, there is a time to start _"consuming our own dog food"_. NetBeans [relies on nb-javac](https://cwiki.apache.org/confluence/display/NETBEANS/Overview:+nb-javac) to provide *WYSIWYG Java editing experience* supporting the **latest Java language features** regardless the JDK the IDE runs on. Reliable, compatible and well tested [nb-javac](https://github.com/JaroslavTulach/nb-javac) JARs are essential for delivering such an experience and ensuring the same behavior as when running NetBeans IDE on the latest JDK without nb-javac.
There are many ways to ensure [nb-javac](https://github.com/JaroslavTulach/nb-javac) is compatible with JDK's javac including its unique [automatic generation process](https://cwiki.apache.org/confluence/display/NETBEANS/Overview:+nb-javac#Overview:nbjavac-AutomaticallyGeneratingnb-javac). However nothing can beat a real world experience: Let's *use nb-javac to build some real* world project! Let's use it to **build NetBeans**! NetBeans is a huge, long running, well established software project using various aspects of the Java language - by building it with [nb-javac](https://github.com/JaroslavTulach/nb-javac) we can proof the compiler is good enough (at least for building NetBeans).
To build NetBeans with this PR use:
```bash
$ ant download-all-extbins
$ ant build -Dnbjavac.class.path=java/libs.javacapi/external/*.jar
```
Without specifying the `nbjavac.class.path` property the build runs as normal. When the `nbjavac.class.path` property is specified, it uses the provided location to find [nb-javac](https://github.com/JaroslavTulach/nb-javac) JARs and load the compiler from there - a bit of `ClassLoader` magic is necessary to make sure the [nb-javac](https://github.com/JaroslavTulach/nb-javac) takes precedence over JDK's javac and is cached between subsequent compiler invocations in the Ant `nbbuild/build.xml` script, but everything seems to work across JDKs.
The [nb-javac](https://github.com/JaroslavTulach/nb-javac) JARs are intentionally specified via a property to simplify providing different version of `nb-javac`. Maintainers of [nb-javac](https://github.com/JaroslavTulach/nb-javac) project (e.g. me and DuĊĦan) are encouraged to include build of NetBeans into their own CI pipeline to verify new versions of [nb-javac](https://github.com/JaroslavTulach/nb-javac) remain compatible with NetBeans sources.
Similarly the NetBeans CI shall be extended with a job to verify `ant build -Dnbjavac.class.path=java/libs.javacapi/external/*.jar` continues to work. Here I rely on @mbien guidance as the CI configuration changed a lot since the last time I played with it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] matthiasblaesing commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "matthiasblaesing (via GitHub)" <gi...@apache.org>.
matthiasblaesing commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1484240644
@mbien was this what you had in mind: https://github.com/apache/netbeans/pull/5724/commits/8745ca5b958f2246ae68a552b0b0b0b619badda0?
A test run is currently executing here: https://github.com/apache/netbeans/pull/5724
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] jtulach commented on a diff in pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "jtulach (via GitHub)" <gi...@apache.org>.
jtulach commented on code in PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#discussion_r1133119166
##########
nbbuild/antsrc/org/netbeans/nbbuild/CustomJavac.java:
##########
@@ -252,4 +276,144 @@ private static boolean isIgnored(
return false;
}
+ private static final class NbJavacLoader extends URLClassLoader {
+ private static final String MAIN_COMPILER_CP = "nbjavac.class.path";
+ private static final String MAIN_COMPILER_CLASS = "com.sun.tools.javac.Main";
+ private final Map<String, Class<?>> priorityLoaded;
+
+ private NbJavacLoader(URL[] urls, ClassLoader parent) {
+ super(urls, parent);
+ this.priorityLoaded = new HashMap<>();
Review Comment:
I tried to cache the `Class<?>` in a static field and that isn't enough. The `CustomJavac` class is loaded again for each NetBeans project (and there are thousands of them). With:
```diff
netbeans$ git diff
diff --git nbbuild/antsrc/org/netbeans/nbbuild/CustomJavac.java nbbuild/antsrc/org/netbeans/nbbuild/CustomJavac.java
index 2e17b218d4..a1a57d01b1 100644
--- nbbuild/antsrc/org/netbeans/nbbuild/CustomJavac.java
+++ nbbuild/antsrc/org/netbeans/nbbuild/CustomJavac.java
@@ -52,6 +52,9 @@ import org.apache.tools.ant.util.JavaEnvUtils;
* and a separate task for {@link #cleanUpStaleClasses}.
*/
public class CustomJavac extends Javac {
+ static {
+ System.err.println("CustomJavac loaded by " + CustomJavac.class.getClassLoader());
+ }
public CustomJavac() {}
```
I see a lot of "CustomJavac loaded by AntClassLoader[/home/devel/NetBeansProjects/netbeans/nbbuild/build/nbantext.jar]" messages.
I'd like to share the NbJavac `Class<?>` among all such classloaders. Using (misusing) `System.getProperties` is a way to do that.
Without this trick the build was really slowed down. With the trick the time to build NetBeans with `nb-javac` is comparable to regular JDK's Javac.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] dbalek commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "dbalek (via GitHub)" <gi...@apache.org>.
dbalek commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1475834539
If I understand changes in this PR correctly, this PR only adds an **option** for people to build NB using `nb-javac` if they want to (it does not require them to do so) and it should not affect the standard NB build process (as written in the PR description: "Without specifying the `nbjavac.class.path` property the build runs as normal").
If this is so, I have nothing against it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1479236797
I also don't want to write books here and go into everything, but another point I should have had covered:
The PR advertises this as great achievement to be finally able to use javac 20, while we are doing this for month already in CI, long before nb-javac was ready. Once NB 18 branched we will likely build on 21 LTS too, PR #5653 is prepared and green. This gives us early warning without having to wait for nb-javac being ready - every too weeks a new ea build.
nb-javac is a dependency of a subset of all netbeans modules (the java editor specifically). Making it a build dependency for everything is not a feature in itself.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] jtulach commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "jtulach (via GitHub)" <gi...@apache.org>.
jtulach commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1491864438
Thank you for your comments. First and foremost: This PR **doesn't change anything on the way NetBeans is built** by default. An opt-in is needed to turn the functionality on. Description of the PR has been updated to highlight that.
Thanks @matthiasblaesing for your changes in https://github.com/apache/netbeans/pull/5609#issuecomment-1484133563 - I've accepted all of the ones for `CustomJavac.java` file. I haven't accepted anything in the GitHub Actions area - there are some mention of [Rust](https://github.com/matthiasblaesing/netbeans/blob/e1cdf89868d9845db7c98981bde111364ec662f8/.github/workflows/main.yml#L58) and I got confused/scared without knowing what to take and what not? Let's make it subject of further discussion.
I have two approvals. @neilcsmith-net wrote:
> If this change helps with testing upstream, it could go in.
@matthiasblaesing helped me polish this PR greatly - see 42ed77a. @mbien wrote:
> it is ok to add a few lines to the ant task if they make it easier for the nb-javac project to run CI tests
I believe we are all able to accept this **opt-in support for nb-javac** compiler executed from its JARs. I am removing the "do not merge" label and I'd like to integrate my change. Once it is in, the https://github.com/JaroslavTulach/nb-javac/pull/17 will go in as well. Further improvements (CI, discovery of nb-javac, removal of `-Xlint:classfile`, etc.) to this PR are of course possible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] neilcsmith-net commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1491878808
@jtulach see #5748
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] jtulach commented on a diff in pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "jtulach (via GitHub)" <gi...@apache.org>.
jtulach commented on code in PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#discussion_r1154413643
##########
nbbuild/antsrc/org/netbeans/nbbuild/CustomJavac.java:
##########
@@ -252,4 +276,144 @@ private static boolean isIgnored(
return false;
}
+ private static final class NbJavacLoader extends URLClassLoader {
+ private static final String MAIN_COMPILER_CP = "nbjavac.class.path";
+ private static final String MAIN_COMPILER_CLASS = "com.sun.tools.javac.Main";
+ private final Map<String, Class<?>> priorityLoaded;
+
+ private NbJavacLoader(URL[] urls, ClassLoader parent) {
+ super(urls, parent);
+ this.priorityLoaded = new HashMap<>();
+ }
+
+ private static synchronized Class<?> findMainCompilerClass(
+ Project prj
+ ) throws MalformedURLException, ClassNotFoundException, URISyntaxException {
+ String cp = prj.getProperty(MAIN_COMPILER_CP);
+ if (cp == null) {
+ return null;
+ }
+
+ Object c = System.getProperties().get(MAIN_COMPILER_CLASS);
+ if (!(c instanceof Class<?>)) {
+ FileSet fs = new FileSet();
+ final File cpPath = new File(cp);
+ if (cpPath.isAbsolute()) {
+ fs.setDir(cpPath.getParentFile());
+ fs.setIncludes(cpPath.getName());
+ } else {
+ String nball = prj.getProperty("nb_all");
+ if (nball != null) {
+ fs.setDir(new File(nball));
+ } else {
+ fs.setDir(prj.getBaseDir());
+ }
+ fs.setIncludes(cp);
+ }
+ List<URL> urls = new ArrayList<>();
+ final DirectoryScanner scan = fs.getDirectoryScanner(prj);
+ File base = scan.getBasedir();
+ for (String relative : scan.getIncludedFiles()) {
+ File file = new File(base, relative);
+ URL url = FileUtils.getFileUtils().getFileURL(file);
+ urls.add(url);
+ }
+ if (urls.isEmpty()) {
+ throw new BuildException("Cannot find nb-javac JAR libraries in " + base + " and " + cp);
+ }
+ URLClassLoader loader = new NbJavacLoader(urls.toArray(new URL[0]), CustomJavac.class.getClassLoader().getParent());
+ final Class<?> newCompilerClass = Class.forName(MAIN_COMPILER_CLASS, true, loader);
+ assertIsolatedClassLoader(newCompilerClass, loader);
+ System.getProperties().put(MAIN_COMPILER_CLASS, newCompilerClass);
+ c = newCompilerClass;
+ }
+ return (Class<?>) c;
+ }
+
+ private static void assertIsolatedClassLoader(Class<?> c, URLClassLoader loader) throws ClassNotFoundException, BuildException {
+ if (c.getClassLoader() != loader) {
+ throw new BuildException("Class " + c + " loaded by " + c.getClassLoader() + " and not " + loader);
+ }
+ Class<?> stdLoc = c.getClassLoader().loadClass(StandardLocation.class.getName());
+ if (stdLoc.getClassLoader() != loader) {
+ throw new BuildException("Class " + stdLoc + " loaded by " + stdLoc.getClassLoader() + " and not " + loader);
+ }
+ }
+
+ @Override
+ protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
+ if (isNbJavacClass(name)) {
+ Class<?> clazz = priorityLoaded.get(name);
+ if (clazz == null) {
+ clazz = findClass(name);
+ priorityLoaded.put(name, clazz);
+ }
+ return clazz;
+ }
+ return super.loadClass(name, resolve);
+ }
+
+ @Override
+ public Class<?> loadClass(String name) throws ClassNotFoundException {
+ return loadClass(name, false);
+ }
+
+ private static boolean isNbJavacClass(String name) {
+ return name.startsWith("javax.annotation.")
+ || name.startsWith("javax.tools.")
+ || name.startsWith("javax.lang.model.")
+ || name.startsWith("com.sun.source.")
+ || name.startsWith("com.sun.tools.");
+ }
+ }
+
+ private static final class NbJavacCompiler extends Javac13 {
+
+ private final Class<?> mainClazz;
+
+ NbJavacCompiler(Class<?> mainClazz) {
+ this.mainClazz = mainClazz;
+ }
+
+ @Override
+ public boolean execute() throws BuildException {
+ attributes.log("Using modern compiler", Project.MSG_VERBOSE);
+ Commandline cmd = setupModernJavacCommand();
+ final String[] args = cmd.getArguments();
+ boolean bootClasspath = false;
+ for (int i = 0; i < args.length; i++) {
+ if (args[i].startsWith("-Xbootclasspath/p:")) { // ide/html
+ bootClasspath = true;
+ }
+ if (args[i].startsWith("-J")) {
+ args[i] = "-Xlint:none"; // webcommon/javascript2.editor
+ }
+ }
+ for (int i = 0; i < args.length; i++) {
+ if (!bootClasspath) {
+ if ("-target".equals(args[i]) || "-source".equals(args[i])) {
+ args[i] = "--release";
+ if (args[i + 1].startsWith("1.")) {
+ args[i + 1] = "8";
+ }
+ }
+ }
+ if ("-Werror".equals(args[i])) {
+ args[i] = "-Xlint:none";
Review Comment:
Mitigated greatly by Matthias's 42ed77a
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] jtulach commented on a diff in pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "jtulach (via GitHub)" <gi...@apache.org>.
jtulach commented on code in PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#discussion_r1154409704
##########
nbbuild/antsrc/org/netbeans/nbbuild/CustomJavac.java:
##########
@@ -395,16 +410,20 @@ public boolean execute() throws BuildException {
}
}
}
- if ("-Werror".equals(args[i])) {
- args[i] = "-Xlint:none";
- }
}
+ // nbjavac in version 19 contains invalid ct.sym files, which cause
+ // warnings from build. Some of the modules are compiled with
+ // -Werror and thus this breaks the build
+ // Ater the update to version 20 this should be removed.
+ String[] args2 = new String[args.length + 1];
+ args2[0] = "-Xlint:-classfile";
+ System.arraycopy(args, 0, args2, 1, args.length);
Review Comment:
Thank you @matthiasblaesing - this is certainly better than my previous `-Xlint:none` code!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] jtulach commented on a diff in pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "jtulach (via GitHub)" <gi...@apache.org>.
jtulach commented on code in PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#discussion_r1141265451
##########
nbbuild/antsrc/org/netbeans/nbbuild/CustomJavac.java:
##########
@@ -252,4 +276,144 @@ private static boolean isIgnored(
return false;
}
+ private static final class NbJavacLoader extends URLClassLoader {
+ private static final String MAIN_COMPILER_CP = "nbjavac.class.path";
+ private static final String MAIN_COMPILER_CLASS = "com.sun.tools.javac.Main";
+ private final Map<String, Class<?>> priorityLoaded;
+
+ private NbJavacLoader(URL[] urls, ClassLoader parent) {
+ super(urls, parent);
+ this.priorityLoaded = new HashMap<>();
+ }
+
+ private static synchronized Class<?> findMainCompilerClass(
+ Project prj
+ ) throws MalformedURLException, ClassNotFoundException, URISyntaxException {
+ String cp = prj.getProperty(MAIN_COMPILER_CP);
+ if (cp == null) {
+ return null;
+ }
+
+ Object c = System.getProperties().get(MAIN_COMPILER_CLASS);
+ if (!(c instanceof Class<?>)) {
+ FileSet fs = new FileSet();
+ final File cpPath = new File(cp);
+ if (cpPath.isAbsolute()) {
+ fs.setDir(cpPath.getParentFile());
+ fs.setIncludes(cpPath.getName());
+ } else {
+ String nball = prj.getProperty("nb_all");
+ if (nball != null) {
+ fs.setDir(new File(nball));
+ } else {
+ fs.setDir(prj.getBaseDir());
+ }
+ fs.setIncludes(cp);
+ }
+ List<URL> urls = new ArrayList<>();
+ final DirectoryScanner scan = fs.getDirectoryScanner(prj);
+ File base = scan.getBasedir();
+ for (String relative : scan.getIncludedFiles()) {
+ File file = new File(base, relative);
+ URL url = FileUtils.getFileUtils().getFileURL(file);
+ urls.add(url);
+ }
+ if (urls.isEmpty()) {
+ throw new BuildException("Cannot find nb-javac JAR libraries in " + base + " and " + cp);
+ }
+ URLClassLoader loader = new NbJavacLoader(urls.toArray(new URL[0]), CustomJavac.class.getClassLoader().getParent());
+ final Class<?> newCompilerClass = Class.forName(MAIN_COMPILER_CLASS, true, loader);
+ assertIsolatedClassLoader(newCompilerClass, loader);
+ System.getProperties().put(MAIN_COMPILER_CLASS, newCompilerClass);
+ c = newCompilerClass;
+ }
+ return (Class<?>) c;
+ }
+
+ private static void assertIsolatedClassLoader(Class<?> c, URLClassLoader loader) throws ClassNotFoundException, BuildException {
+ if (c.getClassLoader() != loader) {
+ throw new BuildException("Class " + c + " loaded by " + c.getClassLoader() + " and not " + loader);
+ }
+ Class<?> stdLoc = c.getClassLoader().loadClass(StandardLocation.class.getName());
+ if (stdLoc.getClassLoader() != loader) {
+ throw new BuildException("Class " + stdLoc + " loaded by " + stdLoc.getClassLoader() + " and not " + loader);
+ }
+ }
+
+ @Override
+ protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
+ if (isNbJavacClass(name)) {
+ Class<?> clazz = priorityLoaded.get(name);
+ if (clazz == null) {
+ clazz = findClass(name);
+ priorityLoaded.put(name, clazz);
+ }
+ return clazz;
+ }
+ return super.loadClass(name, resolve);
+ }
+
+ @Override
+ public Class<?> loadClass(String name) throws ClassNotFoundException {
+ return loadClass(name, false);
+ }
+
+ private static boolean isNbJavacClass(String name) {
+ return name.startsWith("javax.annotation.")
+ || name.startsWith("javax.tools.")
+ || name.startsWith("javax.lang.model.")
+ || name.startsWith("com.sun.source.")
+ || name.startsWith("com.sun.tools.");
+ }
+ }
+
+ private static final class NbJavacCompiler extends Javac13 {
+
+ private final Class<?> mainClazz;
+
+ NbJavacCompiler(Class<?> mainClazz) {
+ this.mainClazz = mainClazz;
+ }
+
+ @Override
+ public boolean execute() throws BuildException {
+ attributes.log("Using modern compiler", Project.MSG_VERBOSE);
+ Commandline cmd = setupModernJavacCommand();
+ final String[] args = cmd.getArguments();
+ boolean bootClasspath = false;
+ for (int i = 0; i < args.length; i++) {
+ if (args[i].startsWith("-Xbootclasspath/p:")) { // ide/html
+ bootClasspath = true;
+ }
+ if (args[i].startsWith("-J")) {
+ args[i] = "-Xlint:none"; // webcommon/javascript2.editor
+ }
+ }
+ for (int i = 0; i < args.length; i++) {
+ if (!bootClasspath) {
+ if ("-target".equals(args[i]) || "-source".equals(args[i])) {
+ args[i] = "--release";
+ if (args[i + 1].startsWith("1.")) {
+ args[i + 1] = "8";
Review Comment:
No problem with that, but in order to keep this PR small and focused, I suggest to fix all the occurrences of `1.8`
```
netbeans$ grep 1.8 */*/nbproject/project.properties | wc -l
826
```
in some different PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] matthiasblaesing commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "matthiasblaesing (via GitHub)" <gi...@apache.org>.
matthiasblaesing commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1475295010
> One advantage of having a firm compiler during build is less dependence on specifics of the compile-time JDK. This might be important if we e.g. wanted to compile some module with `javac.source=19` or `javac.source=20` - currently, that would require upgrading the build JDK to 19/20, and everyone building NB would need to use that. Also, there may be conflict between supported JDKs of Gradle that we use, and the required JDK to build e.g. `javac.source=20` module.
I think this is an academic discussion. If we can't build with the current JDK, it is a bug that must be fixed. Fixating the compiler will only suppress the pain and when it is finally resolved, it will be painful to fix. We see this in the codebase already:
- NetBeans forked javac in the past instead of working with upstream to stabilize it and make it usable over multiple java versions. We are still suffering from that failure.
- NetBeans used an ancient copy of graaljs and maybe even patched this. The two branches got so far apart, that now they have to maintained independently. The initial help of the external codebased turned into a problem.
The same will happen, once we introduce a fixed compiler. People will not fix problems with upstream compiler and code will bitrot. We will be forced to use a non-standard javac ad infinitum.
It is also a bit ironic to speak about `javac.source=20` in the NetBeans codebase. It currently literally takes days to make a module, that can use current APIs (current as in "5 years ago"). The NetBeans codebase targets a compiler, that is 9(!) years old, so sorry, but that argument does not fly with me.
> Also, if we ever tried to make reproducible builds, having a firm, known, compiler might reduce the amount of problems to solve.
It is trivial to install a JDK from a known vendor, if the javac is really the biggest problem. Contrary to some comments about using recent JDKs read as if it takes days to install a current JDK, in reality it is a matter of minutes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] matthiasblaesing commented on a diff in pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "matthiasblaesing (via GitHub)" <gi...@apache.org>.
matthiasblaesing commented on code in PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#discussion_r1141393076
##########
nbbuild/antsrc/org/netbeans/nbbuild/CustomJavac.java:
##########
@@ -252,4 +276,144 @@ private static boolean isIgnored(
return false;
}
+ private static final class NbJavacLoader extends URLClassLoader {
+ private static final String MAIN_COMPILER_CP = "nbjavac.class.path";
+ private static final String MAIN_COMPILER_CLASS = "com.sun.tools.javac.Main";
+ private final Map<String, Class<?>> priorityLoaded;
+
+ private NbJavacLoader(URL[] urls, ClassLoader parent) {
+ super(urls, parent);
+ this.priorityLoaded = new HashMap<>();
Review Comment:
I mean this code:
https://github.com/apache/netbeans/blob/0112550f0a792fc6cfe0f26d74defc057a6d5cd0/nbbuild/antsrc/org/netbeans/nbbuild/CustomJavac.java#L343-L354
`priorityLoaded` is a `HashMap` and if it could be accessed from multiple threads it must be synchronized.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] jlahoda commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "jlahoda (via GitHub)" <gi...@apache.org>.
jlahoda commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1475270443
My personal opinion only.
One advantage of having a firm compiler during build is less dependence on specifics of the compile-time JDK. This might be important if we e.g. wanted to compile some module with `javac.source=19` or `javac.source=20` - currently, that would require upgrading the build JDK to 19/20, and everyone building NB would need to use that. Also, there may be conflict between supported JDKs of Gradle that we use, and the required JDK to build e.g. `javac.source=20` module.
Also, if we ever tried to make reproducible builds, having a firm, known, compiler might reduce the amount of problems to solve.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1484243908
> @mbien was this what you had in mind: [8745ca5](https://github.com/apache/netbeans/commit/8745ca5b958f2246ae68a552b0b0b0b619badda0)?
>
> A test run is currently executing here: #5724
yes almost :) I commented inline, I hope you can see it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [GitHub] [netbeans] mbien commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by Glenn Holmer <ce...@protonmail.com.INVALID>.
On 3/11/23 01:01, mbien wrote:
> We should rather concentrate on upgrading everything to JDK 11 which actually solves problems and has likely community consensus as I gather from the last thread about it. @neilcsmith-net drafted a proposal for this.
+1
--
Glenn Holmer (Linux registered user #16682)
"After the vintage season came the aftermath -- and Cenbe."
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1464846324
nb-javac allows the java editor to run on JDKs which are older than the JDK the editor modules were linked against. It is purely a bytecode level backport and is optional if NB is run on the _right_ JDK (same version as the one nb-javac was based on). Community installers for example could easily bundle NB without nb-javac in it, even NB itself could [download a runtime JDK](https://www.youtube.com/watch?v=8uEIqAd2JBk) and completely drop the nb-javac dependency some time in future.
I don't see what problem this PR is solving beside adding complexity and an indirection to the build process. Soon we will switch to building/testing on JDK 21 EA and part of the test is using the EA compiler too. The javac of the JDK works just fine and we make sure it builds on all LTS versions. Again: nb-javac is just a dependency for some modules, we don't have to bootstrap the whole project with it. NetBeans is more than just the java editor which currently relies on nb-javac.
nb-javac is currently also not in this repository (or even an apache project unless I miss something obvious), so I am not sure what you mean by "eat your own dog food".
I would not want to see this added to the project since I frankly don't see a reason for it. If the nb-javac project needs more tests, why can't nb-javac host the ant task (maven plugin?) and build a larger project there in CI?
We should rather concentrate on upgrading everything to JDK 11 which actually solves problems and has likely community consensus as I gather from the last thread about it. @neilcsmith-net drafted a proposal for this.
Further, a change like this (swapping out the compiler) would require more discussion on the dev list anyway - and a vote IMO.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] jlahoda commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "jlahoda (via GitHub)" <gi...@apache.org>.
jlahoda commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1475327204
> > One advantage of having a firm compiler during build is less dependence on specifics of the compile-time JDK. This might be important if we e.g. wanted to compile some module with `javac.source=19` or `javac.source=20` - currently, that would require upgrading the build JDK to 19/20, and everyone building NB would need to use that. Also, there may be conflict between supported JDKs of Gradle that we use, and the required JDK to build e.g. `javac.source=20` module.
>
> I think this is an academic discussion. If we can't build with the current JDK, it is a bug that must be fixed. Fixating the compiler will only suppress the pain and when it is finally resolved, it will be painful to fix. We see this in the codebase already:
>
> * NetBeans forked javac in the past instead of working with upstream to stabilize it and make it usable over multiple java versions. We are still suffering from that failure.
>
> * NetBeans used an ancient copy of graaljs and maybe even patched this. The two branches got so far apart, that now they have to maintained independently. The initial help of the external codebased turned into a problem.
>
>
> The same will happen, once we introduce a fixed compiler. People will not fix problems with upstream compiler and code will bitrot. We will be forced to use a non-standard javac ad infinitum.
I think this is backwards: the compiler would be the up-to-date compiler. So, people would have to deal with any problems, and it would be more difficult to hide from problems related to the compiler. (It might be easier to hide solving non-compiler related problems, though.)
>
> It is also a bit ironic to speak about `javac.source=20` in the NetBeans codebase. It currently literally takes days to make a module, that can use current APIs (current as in "5 years ago"). The NetBeans codebase targets bytecode, that is 9(!) years old, so sorry, but that argument does not fly with me.
I am completely confused here: on purely technical level, I think I can add a optional module with `javac.source=17` quite easily:
https://github.com/jlahoda/netbeans/tree/test-module-17
There are, of course, some issues with that, among others:
- maybe I would want that module to be usable on older JDKs. Then I probably cannot do it, but this is not a technical problem.
- I need to use JDK 17 to build NB from now, and everyone else has to.
- (possibly some handwaving around eager modules, which may or may not desirable for a given module)
The second point makes it unclear to me how to finalize a change like this - while this should not affect people running the resulting build, it will affect everyone building it, and will require upgrade of the build JDK. How does one do that?
Should we simply say that NB builds using the newest JDK only? (The newest JDK at the time of release, or something like this.) Because that's one of the very few alternatives I see for using newer sources?
(Here I'd like to point out that build JDK is not necessarily the runtime JDK. And, with the changes in past years, I believe it should be easily achievable to have build JDK newer than the runtime JDK, and the opposite direction was always the case.)
>
> > Also, if we ever tried to make reproducible builds, having a firm, known, compiler might reduce the amount of problems to solve.
>
> It is trivial to install a JDK from a known vendor, if the javac is really the biggest problem. Contrary to some comments about using recent JDKs read as if it takes days to install a current JDK, in reality it is a matter of minutes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] jtulach commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "jtulach (via GitHub)" <gi...@apache.org>.
jtulach commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1492850582
I've just squashed all the commits into single one a44b3ed to remove noise from the history.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] jtulach commented on a diff in pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "jtulach (via GitHub)" <gi...@apache.org>.
jtulach commented on code in PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#discussion_r1141263003
##########
nbbuild/antsrc/org/netbeans/nbbuild/CustomJavac.java:
##########
@@ -252,4 +276,144 @@ private static boolean isIgnored(
return false;
}
+ private static final class NbJavacLoader extends URLClassLoader {
+ private static final String MAIN_COMPILER_CP = "nbjavac.class.path";
+ private static final String MAIN_COMPILER_CLASS = "com.sun.tools.javac.Main";
+ private final Map<String, Class<?>> priorityLoaded;
+
+ private NbJavacLoader(URL[] urls, ClassLoader parent) {
+ super(urls, parent);
+ this.priorityLoaded = new HashMap<>();
Review Comment:
> accessing the HashMap in a potentially racy fashion.
Do you mean the `HashMap` behind `Properties`? `Properties` is an ancient class and it extends `Hashtable` which is `synchronized`.
Or do you mean some other race condition that would load the `nb-javac` classes more than once? If so, then it shouldn't be a problem. The `CustomJavac` task works when loading the classes multiple times - it's just slow, so we want to cache it and not load it thousand times. As soon as the `Class` lands in global properties, any compilation that happens 1s later will pick it up. That shall be good enough.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] neilcsmith-net commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1466045464
I share reservations of @mbien and @matthiasblaesing It's an interesting exercise, but could never be the default option, adds complexity to the build, and isn't really the best test of nb-javac IMO. Tests using modern language features and comparing output with the un-backported javac would seem more useful? Still, I'm -0 as long as it doesn't break things!
> nb-javac is currently also not in this repository (or even an apache project unless I miss something obvious), so I am not sure what you mean by "eat your own dog food".
Well, nb-javac is not an Apache NetBeans project, and _cannot_ be developed as an Apache NetBeans project*. Unless something significant changes in ASF rules, nb-javac is never going to be our "own dog food". Compiling NetBeans might be considered to be.
Whichever, the responsibility for development and the bulk of testing nb-javac needs to remain external. If this change helps with testing _upstream_, it could go in. We could even consider running that build test in CI here, but I'd probably limit that to nb-javac update PRs. We don't have unlimited capacity.
* one option to investigate, and discussed previously, might be running the backport javac build inside the NetBeans build, assuming no GPL+CPE sources required on our side. That would still need some checking with legal.
> We should rather concentrate on upgrading everything to JDK 11 which actually solves problems and has likely community consensus as I gather from the last thread about it. @neilcsmith-net drafted a proposal for this.
+1 to that, but I don't think there's any overlap in the two things. nb-javac cannot help with our requirements to update minimum JDK, and upgrading the base to JDK 11 then 17 won't remove our reliance on nb-javac for Java editing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] matthiasblaesing commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "matthiasblaesing (via GitHub)" <gi...@apache.org>.
matthiasblaesing commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1476875436
@jtulach the conflict is caused by overlapping changed from #5652. I put my words to work and cleaned the flags up. I suggest to remove the conflicting parts from 0112550f0a792fc6cfe0f26d74defc057a6d5cd0 and rebase this whole PR onto current master. That should clean this up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] jtulach commented on a diff in pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "jtulach (via GitHub)" <gi...@apache.org>.
jtulach commented on code in PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#discussion_r1141265312
##########
nbbuild/antsrc/org/netbeans/nbbuild/CustomJavac.java:
##########
@@ -252,4 +276,144 @@ private static boolean isIgnored(
return false;
}
+ private static final class NbJavacLoader extends URLClassLoader {
+ private static final String MAIN_COMPILER_CP = "nbjavac.class.path";
+ private static final String MAIN_COMPILER_CLASS = "com.sun.tools.javac.Main";
+ private final Map<String, Class<?>> priorityLoaded;
+
+ private NbJavacLoader(URL[] urls, ClassLoader parent) {
+ super(urls, parent);
+ this.priorityLoaded = new HashMap<>();
+ }
+
+ private static synchronized Class<?> findMainCompilerClass(
+ Project prj
+ ) throws MalformedURLException, ClassNotFoundException, URISyntaxException {
+ String cp = prj.getProperty(MAIN_COMPILER_CP);
+ if (cp == null) {
+ return null;
+ }
+
+ Object c = System.getProperties().get(MAIN_COMPILER_CLASS);
+ if (!(c instanceof Class<?>)) {
+ FileSet fs = new FileSet();
+ final File cpPath = new File(cp);
+ if (cpPath.isAbsolute()) {
+ fs.setDir(cpPath.getParentFile());
+ fs.setIncludes(cpPath.getName());
+ } else {
+ String nball = prj.getProperty("nb_all");
+ if (nball != null) {
+ fs.setDir(new File(nball));
+ } else {
+ fs.setDir(prj.getBaseDir());
+ }
+ fs.setIncludes(cp);
+ }
+ List<URL> urls = new ArrayList<>();
+ final DirectoryScanner scan = fs.getDirectoryScanner(prj);
+ File base = scan.getBasedir();
+ for (String relative : scan.getIncludedFiles()) {
+ File file = new File(base, relative);
+ URL url = FileUtils.getFileUtils().getFileURL(file);
+ urls.add(url);
+ }
+ if (urls.isEmpty()) {
+ throw new BuildException("Cannot find nb-javac JAR libraries in " + base + " and " + cp);
+ }
+ URLClassLoader loader = new NbJavacLoader(urls.toArray(new URL[0]), CustomJavac.class.getClassLoader().getParent());
+ final Class<?> newCompilerClass = Class.forName(MAIN_COMPILER_CLASS, true, loader);
+ assertIsolatedClassLoader(newCompilerClass, loader);
+ System.getProperties().put(MAIN_COMPILER_CLASS, newCompilerClass);
+ c = newCompilerClass;
+ }
+ return (Class<?>) c;
+ }
+
+ private static void assertIsolatedClassLoader(Class<?> c, URLClassLoader loader) throws ClassNotFoundException, BuildException {
+ if (c.getClassLoader() != loader) {
+ throw new BuildException("Class " + c + " loaded by " + c.getClassLoader() + " and not " + loader);
+ }
+ Class<?> stdLoc = c.getClassLoader().loadClass(StandardLocation.class.getName());
+ if (stdLoc.getClassLoader() != loader) {
+ throw new BuildException("Class " + stdLoc + " loaded by " + stdLoc.getClassLoader() + " and not " + loader);
+ }
+ }
+
+ @Override
+ protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
+ if (isNbJavacClass(name)) {
+ Class<?> clazz = priorityLoaded.get(name);
+ if (clazz == null) {
+ clazz = findClass(name);
+ priorityLoaded.put(name, clazz);
+ }
+ return clazz;
+ }
+ return super.loadClass(name, resolve);
+ }
+
+ @Override
+ public Class<?> loadClass(String name) throws ClassNotFoundException {
+ return loadClass(name, false);
+ }
+
+ private static boolean isNbJavacClass(String name) {
+ return name.startsWith("javax.annotation.")
+ || name.startsWith("javax.tools.")
+ || name.startsWith("javax.lang.model.")
+ || name.startsWith("com.sun.source.")
+ || name.startsWith("com.sun.tools.");
+ }
+ }
+
+ private static final class NbJavacCompiler extends Javac13 {
+
+ private final Class<?> mainClazz;
+
+ NbJavacCompiler(Class<?> mainClazz) {
+ this.mainClazz = mainClazz;
+ }
+
+ @Override
+ public boolean execute() throws BuildException {
+ attributes.log("Using modern compiler", Project.MSG_VERBOSE);
+ Commandline cmd = setupModernJavacCommand();
+ final String[] args = cmd.getArguments();
+ boolean bootClasspath = false;
+ for (int i = 0; i < args.length; i++) {
+ if (args[i].startsWith("-Xbootclasspath/p:")) { // ide/html
+ bootClasspath = true;
+ }
+ if (args[i].startsWith("-J")) {
+ args[i] = "-Xlint:none"; // webcommon/javascript2.editor
Review Comment:
Thank you. There were two definitions of `javac.compilerargs` in each of the project files. Removed together with the `CustomJavac` hack to eliminate `-J` options in 0112550f0a792
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] jtulach commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "jtulach (via GitHub)" <gi...@apache.org>.
jtulach commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1475294533
> One advantage of having a firm compiler during build is less dependence on specifics of the compile-time JDK.
Thank you Jan. While this PR demonstrates having such a **firm compiler specification** is possible, it is not the goal of this PR to make any such changes. My goal is to allow usage of `nb-javac` in the NetBeans build system and start testing such combo. Clearly there are issues in the `nb-javac` that shall be improved (for example the `-Werror` problem mentioned above). Improving them while having CI checks on NetBeans as well as `nb-javac` side is a solid engineering approach I am aiming at.
Hopefully the [three line change](https://github.com/apache/netbeans/pull/5609/files#diff-2e6575e0d5f99cfd495a77cb8c35ab906d34f9dd2ea1aa9c93156a180a237ab3R132) to the regular build process is found safe enough to allow us to get it in and start evolving the NetBeans+nb-javac combo properly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] neilcsmith-net commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1484154347
Thanks @matthiasblaesing Couple of comments, in priority order.
> Only run the nbjavac test job when the Java label is added to the PR.
Can we please add a `nb-javac` label and only run the test on that, not every Java labelled PR as per https://github.com/apache/netbeans/pull/5609#issuecomment-1476006761
> `loadClass` is not synchronized and does not synchronize access to the caching `HashMap`. Instead of introducing synchronization, use a thread safe `ConcurrentHashMap`.
Also use `computeIfAbsent()`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] jtulach commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "jtulach (via GitHub)" <gi...@apache.org>.
jtulach commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1491875960
Latest [GitHub Actions run detected](https://github.com/apache/netbeans/actions/runs/4574961454/jobs/8077218339?pr=5609#step:5:127883) a violation in JDK API usage. `java/java.j2seplatform` claims support for JDK8 by having `javac.source=8` (which implies `javac.target=8`) and yet it was using JDK9 API. Fixed in 8018019 - @mbien please review for code correctness.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] matthiasblaesing commented on a diff in pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "matthiasblaesing (via GitHub)" <gi...@apache.org>.
matthiasblaesing commented on code in PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#discussion_r1133089406
##########
nbbuild/antsrc/org/netbeans/nbbuild/CustomJavac.java:
##########
@@ -252,4 +276,144 @@ private static boolean isIgnored(
return false;
}
+ private static final class NbJavacLoader extends URLClassLoader {
+ private static final String MAIN_COMPILER_CP = "nbjavac.class.path";
+ private static final String MAIN_COMPILER_CLASS = "com.sun.tools.javac.Main";
+ private final Map<String, Class<?>> priorityLoaded;
+
+ private NbJavacLoader(URL[] urls, ClassLoader parent) {
+ super(urls, parent);
+ this.priorityLoaded = new HashMap<>();
+ }
+
+ private static synchronized Class<?> findMainCompilerClass(
+ Project prj
+ ) throws MalformedURLException, ClassNotFoundException, URISyntaxException {
+ String cp = prj.getProperty(MAIN_COMPILER_CP);
+ if (cp == null) {
+ return null;
+ }
+
+ Object c = System.getProperties().get(MAIN_COMPILER_CLASS);
+ if (!(c instanceof Class<?>)) {
+ FileSet fs = new FileSet();
+ final File cpPath = new File(cp);
+ if (cpPath.isAbsolute()) {
+ fs.setDir(cpPath.getParentFile());
+ fs.setIncludes(cpPath.getName());
+ } else {
+ String nball = prj.getProperty("nb_all");
+ if (nball != null) {
+ fs.setDir(new File(nball));
+ } else {
+ fs.setDir(prj.getBaseDir());
+ }
+ fs.setIncludes(cp);
+ }
+ List<URL> urls = new ArrayList<>();
+ final DirectoryScanner scan = fs.getDirectoryScanner(prj);
+ File base = scan.getBasedir();
+ for (String relative : scan.getIncludedFiles()) {
+ File file = new File(base, relative);
+ URL url = FileUtils.getFileUtils().getFileURL(file);
+ urls.add(url);
+ }
+ if (urls.isEmpty()) {
+ throw new BuildException("Cannot find nb-javac JAR libraries in " + base + " and " + cp);
+ }
+ URLClassLoader loader = new NbJavacLoader(urls.toArray(new URL[0]), CustomJavac.class.getClassLoader().getParent());
+ final Class<?> newCompilerClass = Class.forName(MAIN_COMPILER_CLASS, true, loader);
+ assertIsolatedClassLoader(newCompilerClass, loader);
+ System.getProperties().put(MAIN_COMPILER_CLASS, newCompilerClass);
+ c = newCompilerClass;
+ }
+ return (Class<?>) c;
+ }
+
+ private static void assertIsolatedClassLoader(Class<?> c, URLClassLoader loader) throws ClassNotFoundException, BuildException {
+ if (c.getClassLoader() != loader) {
+ throw new BuildException("Class " + c + " loaded by " + c.getClassLoader() + " and not " + loader);
+ }
+ Class<?> stdLoc = c.getClassLoader().loadClass(StandardLocation.class.getName());
+ if (stdLoc.getClassLoader() != loader) {
+ throw new BuildException("Class " + stdLoc + " loaded by " + stdLoc.getClassLoader() + " and not " + loader);
+ }
+ }
+
+ @Override
+ protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
+ if (isNbJavacClass(name)) {
+ Class<?> clazz = priorityLoaded.get(name);
+ if (clazz == null) {
+ clazz = findClass(name);
+ priorityLoaded.put(name, clazz);
+ }
+ return clazz;
+ }
+ return super.loadClass(name, resolve);
+ }
+
+ @Override
+ public Class<?> loadClass(String name) throws ClassNotFoundException {
+ return loadClass(name, false);
+ }
+
+ private static boolean isNbJavacClass(String name) {
+ return name.startsWith("javax.annotation.")
+ || name.startsWith("javax.tools.")
+ || name.startsWith("javax.lang.model.")
+ || name.startsWith("com.sun.source.")
+ || name.startsWith("com.sun.tools.");
+ }
+ }
+
+ private static final class NbJavacCompiler extends Javac13 {
+
+ private final Class<?> mainClazz;
+
+ NbJavacCompiler(Class<?> mainClazz) {
+ this.mainClazz = mainClazz;
+ }
+
+ @Override
+ public boolean execute() throws BuildException {
+ attributes.log("Using modern compiler", Project.MSG_VERBOSE);
+ Commandline cmd = setupModernJavacCommand();
+ final String[] args = cmd.getArguments();
+ boolean bootClasspath = false;
+ for (int i = 0; i < args.length; i++) {
+ if (args[i].startsWith("-Xbootclasspath/p:")) { // ide/html
+ bootClasspath = true;
+ }
+ if (args[i].startsWith("-J")) {
+ args[i] = "-Xlint:none"; // webcommon/javascript2.editor
Review Comment:
This looks strange. I see the "javac.compilerargs=-J-Xmx512m" and `build.compiler=extJavac` in `project.properties` from `webcommon/javascript2.editor`. Both additions look doubious and most probably should be dropped instead of adding this hack (they are identically present in `php/php.editor`).
##########
nbbuild/antsrc/org/netbeans/nbbuild/CustomJavac.java:
##########
@@ -252,4 +276,144 @@ private static boolean isIgnored(
return false;
}
+ private static final class NbJavacLoader extends URLClassLoader {
+ private static final String MAIN_COMPILER_CP = "nbjavac.class.path";
+ private static final String MAIN_COMPILER_CLASS = "com.sun.tools.javac.Main";
+ private final Map<String, Class<?>> priorityLoaded;
+
+ private NbJavacLoader(URL[] urls, ClassLoader parent) {
+ super(urls, parent);
+ this.priorityLoaded = new HashMap<>();
+ }
+
+ private static synchronized Class<?> findMainCompilerClass(
+ Project prj
+ ) throws MalformedURLException, ClassNotFoundException, URISyntaxException {
+ String cp = prj.getProperty(MAIN_COMPILER_CP);
+ if (cp == null) {
+ return null;
+ }
+
+ Object c = System.getProperties().get(MAIN_COMPILER_CLASS);
+ if (!(c instanceof Class<?>)) {
+ FileSet fs = new FileSet();
+ final File cpPath = new File(cp);
+ if (cpPath.isAbsolute()) {
+ fs.setDir(cpPath.getParentFile());
+ fs.setIncludes(cpPath.getName());
+ } else {
+ String nball = prj.getProperty("nb_all");
+ if (nball != null) {
+ fs.setDir(new File(nball));
+ } else {
+ fs.setDir(prj.getBaseDir());
+ }
+ fs.setIncludes(cp);
+ }
+ List<URL> urls = new ArrayList<>();
+ final DirectoryScanner scan = fs.getDirectoryScanner(prj);
+ File base = scan.getBasedir();
+ for (String relative : scan.getIncludedFiles()) {
+ File file = new File(base, relative);
+ URL url = FileUtils.getFileUtils().getFileURL(file);
+ urls.add(url);
+ }
+ if (urls.isEmpty()) {
+ throw new BuildException("Cannot find nb-javac JAR libraries in " + base + " and " + cp);
+ }
+ URLClassLoader loader = new NbJavacLoader(urls.toArray(new URL[0]), CustomJavac.class.getClassLoader().getParent());
+ final Class<?> newCompilerClass = Class.forName(MAIN_COMPILER_CLASS, true, loader);
+ assertIsolatedClassLoader(newCompilerClass, loader);
+ System.getProperties().put(MAIN_COMPILER_CLASS, newCompilerClass);
+ c = newCompilerClass;
+ }
+ return (Class<?>) c;
+ }
+
+ private static void assertIsolatedClassLoader(Class<?> c, URLClassLoader loader) throws ClassNotFoundException, BuildException {
+ if (c.getClassLoader() != loader) {
+ throw new BuildException("Class " + c + " loaded by " + c.getClassLoader() + " and not " + loader);
+ }
+ Class<?> stdLoc = c.getClassLoader().loadClass(StandardLocation.class.getName());
+ if (stdLoc.getClassLoader() != loader) {
+ throw new BuildException("Class " + stdLoc + " loaded by " + stdLoc.getClassLoader() + " and not " + loader);
+ }
+ }
+
+ @Override
+ protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
+ if (isNbJavacClass(name)) {
+ Class<?> clazz = priorityLoaded.get(name);
+ if (clazz == null) {
+ clazz = findClass(name);
+ priorityLoaded.put(name, clazz);
+ }
+ return clazz;
+ }
+ return super.loadClass(name, resolve);
+ }
+
+ @Override
+ public Class<?> loadClass(String name) throws ClassNotFoundException {
+ return loadClass(name, false);
+ }
+
+ private static boolean isNbJavacClass(String name) {
+ return name.startsWith("javax.annotation.")
+ || name.startsWith("javax.tools.")
+ || name.startsWith("javax.lang.model.")
+ || name.startsWith("com.sun.source.")
+ || name.startsWith("com.sun.tools.");
+ }
+ }
+
+ private static final class NbJavacCompiler extends Javac13 {
+
+ private final Class<?> mainClazz;
+
+ NbJavacCompiler(Class<?> mainClazz) {
+ this.mainClazz = mainClazz;
+ }
+
+ @Override
+ public boolean execute() throws BuildException {
+ attributes.log("Using modern compiler", Project.MSG_VERBOSE);
+ Commandline cmd = setupModernJavacCommand();
+ final String[] args = cmd.getArguments();
+ boolean bootClasspath = false;
+ for (int i = 0; i < args.length; i++) {
+ if (args[i].startsWith("-Xbootclasspath/p:")) { // ide/html
+ bootClasspath = true;
+ }
+ if (args[i].startsWith("-J")) {
+ args[i] = "-Xlint:none"; // webcommon/javascript2.editor
+ }
+ }
+ for (int i = 0; i < args.length; i++) {
+ if (!bootClasspath) {
+ if ("-target".equals(args[i]) || "-source".equals(args[i])) {
+ args[i] = "--release";
+ if (args[i + 1].startsWith("1.")) {
+ args[i + 1] = "8";
Review Comment:
We have this code at various locations. Would it make sense to go once through the codebase and switch all usesages of `1.7`/`1.8` to 7/8? The old numbering is dead.
##########
nbbuild/antsrc/org/netbeans/nbbuild/CustomJavac.java:
##########
@@ -252,4 +276,144 @@ private static boolean isIgnored(
return false;
}
+ private static final class NbJavacLoader extends URLClassLoader {
+ private static final String MAIN_COMPILER_CP = "nbjavac.class.path";
+ private static final String MAIN_COMPILER_CLASS = "com.sun.tools.javac.Main";
+ private final Map<String, Class<?>> priorityLoaded;
+
+ private NbJavacLoader(URL[] urls, ClassLoader parent) {
+ super(urls, parent);
+ this.priorityLoaded = new HashMap<>();
Review Comment:
Two questions: Does the JVM really not cache the class object? I would have expected the VM to hold the class definition in memory while the class is still in use. If it is indeed necessary, this should be a `ConcurrentHashMap` or be wrapped with `Collections#synchronizedMap`, as `loadClass` is not synchronized and there is no explicit synchronization provided there.
##########
nbbuild/antsrc/org/netbeans/nbbuild/CustomJavac.java:
##########
@@ -252,4 +276,144 @@ private static boolean isIgnored(
return false;
}
+ private static final class NbJavacLoader extends URLClassLoader {
+ private static final String MAIN_COMPILER_CP = "nbjavac.class.path";
+ private static final String MAIN_COMPILER_CLASS = "com.sun.tools.javac.Main";
+ private final Map<String, Class<?>> priorityLoaded;
+
+ private NbJavacLoader(URL[] urls, ClassLoader parent) {
+ super(urls, parent);
+ this.priorityLoaded = new HashMap<>();
+ }
+
+ private static synchronized Class<?> findMainCompilerClass(
+ Project prj
+ ) throws MalformedURLException, ClassNotFoundException, URISyntaxException {
+ String cp = prj.getProperty(MAIN_COMPILER_CP);
+ if (cp == null) {
+ return null;
+ }
+
+ Object c = System.getProperties().get(MAIN_COMPILER_CLASS);
+ if (!(c instanceof Class<?>)) {
+ FileSet fs = new FileSet();
+ final File cpPath = new File(cp);
+ if (cpPath.isAbsolute()) {
+ fs.setDir(cpPath.getParentFile());
+ fs.setIncludes(cpPath.getName());
+ } else {
+ String nball = prj.getProperty("nb_all");
+ if (nball != null) {
+ fs.setDir(new File(nball));
+ } else {
+ fs.setDir(prj.getBaseDir());
+ }
+ fs.setIncludes(cp);
+ }
+ List<URL> urls = new ArrayList<>();
+ final DirectoryScanner scan = fs.getDirectoryScanner(prj);
+ File base = scan.getBasedir();
+ for (String relative : scan.getIncludedFiles()) {
+ File file = new File(base, relative);
+ URL url = FileUtils.getFileUtils().getFileURL(file);
+ urls.add(url);
+ }
+ if (urls.isEmpty()) {
+ throw new BuildException("Cannot find nb-javac JAR libraries in " + base + " and " + cp);
+ }
+ URLClassLoader loader = new NbJavacLoader(urls.toArray(new URL[0]), CustomJavac.class.getClassLoader().getParent());
+ final Class<?> newCompilerClass = Class.forName(MAIN_COMPILER_CLASS, true, loader);
+ assertIsolatedClassLoader(newCompilerClass, loader);
+ System.getProperties().put(MAIN_COMPILER_CLASS, newCompilerClass);
+ c = newCompilerClass;
+ }
+ return (Class<?>) c;
+ }
+
+ private static void assertIsolatedClassLoader(Class<?> c, URLClassLoader loader) throws ClassNotFoundException, BuildException {
+ if (c.getClassLoader() != loader) {
+ throw new BuildException("Class " + c + " loaded by " + c.getClassLoader() + " and not " + loader);
+ }
+ Class<?> stdLoc = c.getClassLoader().loadClass(StandardLocation.class.getName());
+ if (stdLoc.getClassLoader() != loader) {
+ throw new BuildException("Class " + stdLoc + " loaded by " + stdLoc.getClassLoader() + " and not " + loader);
+ }
+ }
+
+ @Override
+ protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
+ if (isNbJavacClass(name)) {
+ Class<?> clazz = priorityLoaded.get(name);
+ if (clazz == null) {
+ clazz = findClass(name);
+ priorityLoaded.put(name, clazz);
+ }
+ return clazz;
+ }
+ return super.loadClass(name, resolve);
+ }
+
+ @Override
+ public Class<?> loadClass(String name) throws ClassNotFoundException {
+ return loadClass(name, false);
+ }
+
+ private static boolean isNbJavacClass(String name) {
+ return name.startsWith("javax.annotation.")
+ || name.startsWith("javax.tools.")
+ || name.startsWith("javax.lang.model.")
+ || name.startsWith("com.sun.source.")
+ || name.startsWith("com.sun.tools.");
+ }
+ }
+
+ private static final class NbJavacCompiler extends Javac13 {
+
+ private final Class<?> mainClazz;
+
+ NbJavacCompiler(Class<?> mainClazz) {
+ this.mainClazz = mainClazz;
+ }
+
+ @Override
+ public boolean execute() throws BuildException {
+ attributes.log("Using modern compiler", Project.MSG_VERBOSE);
+ Commandline cmd = setupModernJavacCommand();
+ final String[] args = cmd.getArguments();
+ boolean bootClasspath = false;
+ for (int i = 0; i < args.length; i++) {
+ if (args[i].startsWith("-Xbootclasspath/p:")) { // ide/html
+ bootClasspath = true;
+ }
+ if (args[i].startsWith("-J")) {
+ args[i] = "-Xlint:none"; // webcommon/javascript2.editor
+ }
+ }
+ for (int i = 0; i < args.length; i++) {
+ if (!bootClasspath) {
+ if ("-target".equals(args[i]) || "-source".equals(args[i])) {
+ args[i] = "--release";
+ if (args[i + 1].startsWith("1.")) {
+ args[i + 1] = "8";
+ }
+ }
+ }
+ if ("-Werror".equals(args[i])) {
+ args[i] = "-Xlint:none";
Review Comment:
If I understand this correctly this turn a "fail build on error" into an "suppress errors". This sounds strange to me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] jtulach commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "jtulach (via GitHub)" <gi...@apache.org>.
jtulach commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1465276210
> Further idea: My reading of the module system is, that it should be possible to selectively upgrade parts of the JDK runtime (see `--upgrade-module-path` option). That way "nb-javac" would just "be" `jdk.compiler`. No fiddling with the build environment necessary.
The drawback is: the`nb-javac` is not used that way in NetBeans. NetBeans is using similar "classloader trick" with [masking JDK classes](https://github.com/apache/netbeans/blob/c8de82fc93cc1dee8b385e77f1aa7352d6eecec8/java/libs.nbjavacapi/manifest.mf#L6). If we succeed with `--upgrade-module-path` option - it may not guarantee `nb-javac` works in NetBeans properly.
Btw. I wanted to "Eat our own dog food" with `nb-javac@19`, but I couldn't as https://github.com/JaroslavTulach/nb-javac/pull/11 - we wouldn't found that problem with `--upgrade-module-path` option.
While [real JDK test](https://github.com/JaroslavTulach/nb-javac/blob/1c6acf4c26172973075d4027ca39a36c063ad43b/build-jdk.sh#L99) is also useful check, for NetBeans I'd rather stick with old good "classloader trick".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1484233047
> @mbien could you check if that addresses some of you concerns? We can discuss whether nbjavac is really a good idea, but at this point in time it is an integral part of the core of Java editing in NetBeans and I think it might help catching bugs.
@matthiasblaesing github workflows can't really trigger on particular labels unfortunately. I believe the easiest way to build using nb-javac when the label is set would be to simply paste the proposed job into the main.yaml.
copy this:
https://github.com/matthiasblaesing/netbeans/blob/623a396c8ed341834d080eb7f4a1d2aa487e6b87/.github/workflows/nb-javac.yml#L6-L28
(rename `linux` to `nb-javac` for example, since that would be the job handle)
and paste it somewhere below `base-build` in main.yaml
having it as separate workflow is fine in theory, but it would startup and do nothing most of the time and create empty runs on that list:
https://github.com/apache/netbeans/actions
i would also recommend to remove the `|| github.event_name != 'pull_request'` and `ci:all-tests` conditions so that it doesn't have to run outside of PRs which set the label.
The cache should be used too so that we don't DDoS someone:
```
- name: Restore Cache
uses: actions/cache/restore@v3
with:
path: ~/.hgexternalcache
key: ${{ runner.os }}-${{ hashFiles('*/external/binaries-list', '*/*/external/binaries-list') }}
restore-keys: ${{ runner.os }}-
```
this should only restore the cache based on hash match or prefix as fallback but never store, other then that it works analog to the base-build job. (never used the restore action before, its a fairly new feature they added earlier this year)
For the record: I have nothing against nb-javac and do agree it is an important dependency of the java editor right now. However, the testing of nb-javac should be done in the nb-javac repository, not in a downstream repository in my opinion. What should we even do if that one job is red? Not merge a PR? Wait for nb-javac to fix something, upload a binary to central and test again?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] neilcsmith-net commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1492876414
@matthiasblaesing agreed to running with same logic as most other labels for now (`nb-javac` or `ci:all-tests` labelled PRs, plus non-PR runs). Let's see how that goes - false positive failures or resource issues and we pull the non-PR tests during the release.
btw - there wasn't an "approval" from me, just not standing in the way as long as it doesn't cause issues. There are certainly other tests of nb-javac that I think would be more valuable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] jtulach commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "jtulach (via GitHub)" <gi...@apache.org>.
jtulach commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1492877634
This PR has `nb-javac` label and I can see [NetBeans on nb-javac (pull_request)](https://github.com/apache/netbeans/actions/runs/4581979954/jobs/8091890311?pr=5609) job has been executed with 91951b2 changes. Good.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] jtulach commented on a diff in pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "jtulach (via GitHub)" <gi...@apache.org>.
jtulach commented on code in PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#discussion_r1141264240
##########
nbbuild/antsrc/org/netbeans/nbbuild/CustomJavac.java:
##########
@@ -252,4 +276,144 @@ private static boolean isIgnored(
return false;
}
+ private static final class NbJavacLoader extends URLClassLoader {
+ private static final String MAIN_COMPILER_CP = "nbjavac.class.path";
+ private static final String MAIN_COMPILER_CLASS = "com.sun.tools.javac.Main";
+ private final Map<String, Class<?>> priorityLoaded;
+
+ private NbJavacLoader(URL[] urls, ClassLoader parent) {
+ super(urls, parent);
+ this.priorityLoaded = new HashMap<>();
+ }
+
+ private static synchronized Class<?> findMainCompilerClass(
+ Project prj
+ ) throws MalformedURLException, ClassNotFoundException, URISyntaxException {
+ String cp = prj.getProperty(MAIN_COMPILER_CP);
+ if (cp == null) {
+ return null;
+ }
+
+ Object c = System.getProperties().get(MAIN_COMPILER_CLASS);
+ if (!(c instanceof Class<?>)) {
+ FileSet fs = new FileSet();
+ final File cpPath = new File(cp);
+ if (cpPath.isAbsolute()) {
+ fs.setDir(cpPath.getParentFile());
+ fs.setIncludes(cpPath.getName());
+ } else {
+ String nball = prj.getProperty("nb_all");
+ if (nball != null) {
+ fs.setDir(new File(nball));
+ } else {
+ fs.setDir(prj.getBaseDir());
+ }
+ fs.setIncludes(cp);
+ }
+ List<URL> urls = new ArrayList<>();
+ final DirectoryScanner scan = fs.getDirectoryScanner(prj);
+ File base = scan.getBasedir();
+ for (String relative : scan.getIncludedFiles()) {
+ File file = new File(base, relative);
+ URL url = FileUtils.getFileUtils().getFileURL(file);
+ urls.add(url);
+ }
+ if (urls.isEmpty()) {
+ throw new BuildException("Cannot find nb-javac JAR libraries in " + base + " and " + cp);
+ }
+ URLClassLoader loader = new NbJavacLoader(urls.toArray(new URL[0]), CustomJavac.class.getClassLoader().getParent());
+ final Class<?> newCompilerClass = Class.forName(MAIN_COMPILER_CLASS, true, loader);
+ assertIsolatedClassLoader(newCompilerClass, loader);
+ System.getProperties().put(MAIN_COMPILER_CLASS, newCompilerClass);
+ c = newCompilerClass;
+ }
+ return (Class<?>) c;
+ }
+
+ private static void assertIsolatedClassLoader(Class<?> c, URLClassLoader loader) throws ClassNotFoundException, BuildException {
+ if (c.getClassLoader() != loader) {
+ throw new BuildException("Class " + c + " loaded by " + c.getClassLoader() + " and not " + loader);
+ }
+ Class<?> stdLoc = c.getClassLoader().loadClass(StandardLocation.class.getName());
+ if (stdLoc.getClassLoader() != loader) {
+ throw new BuildException("Class " + stdLoc + " loaded by " + stdLoc.getClassLoader() + " and not " + loader);
+ }
+ }
+
+ @Override
+ protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
+ if (isNbJavacClass(name)) {
+ Class<?> clazz = priorityLoaded.get(name);
+ if (clazz == null) {
+ clazz = findClass(name);
+ priorityLoaded.put(name, clazz);
+ }
+ return clazz;
+ }
+ return super.loadClass(name, resolve);
+ }
+
+ @Override
+ public Class<?> loadClass(String name) throws ClassNotFoundException {
+ return loadClass(name, false);
+ }
+
+ private static boolean isNbJavacClass(String name) {
+ return name.startsWith("javax.annotation.")
+ || name.startsWith("javax.tools.")
+ || name.startsWith("javax.lang.model.")
+ || name.startsWith("com.sun.source.")
+ || name.startsWith("com.sun.tools.");
+ }
+ }
+
+ private static final class NbJavacCompiler extends Javac13 {
+
+ private final Class<?> mainClazz;
+
+ NbJavacCompiler(Class<?> mainClazz) {
+ this.mainClazz = mainClazz;
+ }
+
+ @Override
+ public boolean execute() throws BuildException {
+ attributes.log("Using modern compiler", Project.MSG_VERBOSE);
+ Commandline cmd = setupModernJavacCommand();
+ final String[] args = cmd.getArguments();
+ boolean bootClasspath = false;
+ for (int i = 0; i < args.length; i++) {
+ if (args[i].startsWith("-Xbootclasspath/p:")) { // ide/html
+ bootClasspath = true;
+ }
+ if (args[i].startsWith("-J")) {
+ args[i] = "-Xlint:none"; // webcommon/javascript2.editor
+ }
+ }
+ for (int i = 0; i < args.length; i++) {
+ if (!bootClasspath) {
+ if ("-target".equals(args[i]) || "-source".equals(args[i])) {
+ args[i] = "--release";
+ if (args[i + 1].startsWith("1.")) {
+ args[i + 1] = "8";
+ }
+ }
+ }
+ if ("-Werror".equals(args[i])) {
+ args[i] = "-Xlint:none";
Review Comment:
If I run with `-Werror`, then the build immediately fails on [these annotation warnings](https://github.com/apache/netbeans/actions/runs/4459083199/jobs/7831316138?pr=5609#step:5:54988) caused by the way I prepare `ct.sym` for [nb-javac](https://github.com/JaroslavTulach/nb-javac). The problems can be fixed in some new version of [nb-javac](https://github.com/JaroslavTulach/nb-javac), but for version 20.0.0 the removal of `-Werror` is necessary.
Btw. this shows why the _"dog food testing"_ of [nb-javac](https://github.com/JaroslavTulach/nb-javac) on NetBeans sources makes sense. These warnings are present even when we run [nb-javac](https://github.com/JaroslavTulach/nb-javac) internally for Java editor purposes. They are just not visible (much) and we would never feel the need to address them.
I'll see what I can do to eliminate the [annotation warnings](https://github.com/apache/netbeans/actions/runs/4459083199/jobs/7831316138?pr=5609#step:5:54988) in a future version of [nb-javac](https://github.com/JaroslavTulach/nb-javac).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1467109006
> I don't think there's any overlap in the two things. nb-javac cannot help with our requirements to update minimum JDK, and upgrading the base to JDK 11 then 17 won't remove our reliance on nb-javac for Java editing.
I didn't mean it like that, indeed bumping the bytecode level won't remove the requirement for nb-javac: it would bring us a tiny step closer though, while not bumping the version increases the distance with every single java release.
I hope that i am wrong but I do fear that this PR here is just bootstrapping the option for a future switch to frgaal with the argument: look we don't have to leave JDk 8 til 2030+ since we just made our own language with some of the language features of newer java versions.
This wouldn't fix any of the problems either btw (e.g the fact that we run out of supported libraries we can use - see that whole maven-indexer discussion and many other modules which want to use non JDK 8 libs). I am taking this purely from previous discussions on dev, and reading between lines (again: i hope that i am wrong).
I don't understand at all, why the netbeans project would have to run tests for nb-javac, this would have to be handled in the nb-javac repo. We also don't test OpenJDK or any other third party lib in CI of the cost of apache.
I read through the PR text a third time now and still can't come up with a single reason why we should try this except form a purely academical perspective.
But as someone who basically spend this weekend trying to find workarounds for maven-indexer, I am very close to -1 any compiler swap proposals until we solve some of our technical dept and the fact that we still cant use anything beyond JDK 8 in 2023 due to non-technical reasons.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] sdedic commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "sdedic (via GitHub)" <gi...@apache.org>.
sdedic commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1477378857
By the way - we probably SHOULD move this to the mailing list ... there are several 'philosophical' topics involved that should be discussed in a forum rather than in the PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] matthiasblaesing commented on a diff in pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "matthiasblaesing (via GitHub)" <gi...@apache.org>.
matthiasblaesing commented on code in PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#discussion_r1137600559
##########
nbbuild/antsrc/org/netbeans/nbbuild/CustomJavac.java:
##########
@@ -252,4 +276,144 @@ private static boolean isIgnored(
return false;
}
+ private static final class NbJavacLoader extends URLClassLoader {
+ private static final String MAIN_COMPILER_CP = "nbjavac.class.path";
+ private static final String MAIN_COMPILER_CLASS = "com.sun.tools.javac.Main";
+ private final Map<String, Class<?>> priorityLoaded;
+
+ private NbJavacLoader(URL[] urls, ClassLoader parent) {
+ super(urls, parent);
+ this.priorityLoaded = new HashMap<>();
Review Comment:
Thanks - this makes sense as I assume ant rebuilds the classloader for each javac invocation and that would explain your observation. Pulling it out into the global properties is an interesting idea to fix this.
That leaves the question regarding the missing synchronization when accessing the HashMap in a potentially racy fashion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] sdedic commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "sdedic (via GitHub)" <gi...@apache.org>.
sdedic commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1476890956
let me share my overall opinion on this. In the *current state* of the codebase, we *need_ nb-javac*. As JDK codebase continues to evolve, newer versions of nbjavac are eventually released. Given there's *automated process* for building nb-javac out of *upstream sources*, it's rather straightforward to prepare a new build. We *are using* nbjavac to analyse user's sources AND we +- support analysis by Javac itself given the IDE runs on the "correct" JDK.
I think we really want to ensure that "running on correct JDK" and "running on possible JDK" (that implies nb-javac to supply the targetted JDK behaviour) gives the same results, or *at least* does not fail, if the user switches nb-javac for javac (or vice-versa). From this point of view, **I fully support** integrating the PR and eventually running the tests **when a new version of nb-javac** is included. I would assume that maintainer will continue to update nbjavac as JDK team releases newer versions. Until that cooperation works, we can easily keep the test.
There were several opinions voiced in the thread
> @mbien: nb-javac is optional if NB is run on the right JDK
I need to remind: people who *use NetBeans* to produce code (as opposed to people *developing NetBeans*), and also people *using NetBeans as tooling platform* have very different *business* requirements that require them to run on just specific JDKs (even the IDE), and have hard requirements for the tooling platform's environment (i.e. build machines in company cluster may have some restrictions). I want to emphasize, is that it does not matter if that requirement (for those groups of our users) is 'technically sound' or not - they may exist for purely business, legal or security reasons - which may be even silly, but impossible to revert from the developer seat.
If we *require* our users to "run on the right JDK" - such groups of people may be forced to simply drop NetBeans completely and look for other options. If we're lucky, mandating 'just the newest JDK' will be an unneccessary PITA. We do not have that much 'spare users' that we can just throw these groups over the board just because it makes NetBeans development more pleasant. Oracle's Graal/Cloud team may be also affected by such approach.
So the assumption that 'everyone can use any JDK' so NB "runs on the correct JDK" is IMHO wrong. I feel that the discussion emphasizes the importance of development "our common pet project" (Apache NetBeans) too much and optimizes for it, while deprioritizing our *users*. I happen to be
- NetBeans IDE developer,
- NetBeans IDE-based application developer: the Apache NB language server, for example, but also the IdealGraphVisualizer from Graal is based on NetBeans IDE *including* java support
- NetBeans platform - based application developer (my incubating personal project).
Even as IGV maintainer, I often needed to compile+patch NB platform to trace bugs, or even develop patches or small features for NB in order to make my *real* project successful -- I do that routinely with maven and gradle (while developing NetBeans). In all situations, having the *flexibility* to work with development environment dictated by my business, rather than "obey NetBeans dictate" is very helpful and makes dev's life a lot easier as there are other pieces included with their own limitations. I will cover this in a separate discussion related to "drop JDK8".
I believe that maintaining a bundled, well-tested compiler is a benefit for those user groups with "strange requirements" - as long as the nb-javac keeps releasing updated versions after its upstream releases. Now it does, so we should act accordingly. At the same time, we can just plan for an exit strategy, which may be fairly simple. We do not need to play what-if games *now* to block a thing that would, for example, alert us when a problematic change happens in some future JDK javac version.
>> @jlahoda : One advantage of having a firm compiler during build is less dependence on specifics of the compile-time JDK.
> @matthiasblaesing: I think this is an academic discussion. If we can't build with the current JDK, it is a bug that must be fixed.
I don't think this is not an academic discussion: I agree that when we detect that our codebase is not compilable with new JDK, we should address that SOON. But I do not see that as a valid argument for or against the discussed change, rather as an unrelated test and policy that we should establish anyway. As jlahoda noted, usage of "most updated" but bundled javac *allows* modern optional modules to be written when the author decides he does not need (want) to support older JDKs. In particular modules that require e.g. JDK17 APIs to run. Doing so *without* nb-javac would require everyone working on NB codebase to upgrade to JDK17 ... which is not an option (see above).
> @mbien : I do fear that this PR here is just bootstrapping the option for a future switch to frgaal with the argument
It might. But a 'fear that something might happen' should not cause to block a change just because "it might be extended in the future, maybe". You/we should be aware of such extension may happen. Rejecting a purely 'piecemeal' approach is a valid position, if it is the (only/major) supportive "argument" if such proposal comes in the future.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] matthiasblaesing commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "matthiasblaesing (via GitHub)" <gi...@apache.org>.
matthiasblaesing commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1484213567
> Thanks @matthiasblaesing Couple of comments, in priority order.
>
> > Only run the nbjavac test job when the Java label is added to the PR.
>
> Can we please add a `nb-javac` label and only run the test on that, not every Java labelled PR as per [#5609 (comment)](https://github.com/apache/netbeans/pull/5609#issuecomment-1476006761)
Label created, added here and updated the final commit (and the comment summarizing the changes).
> > `loadClass` is not synchronized and does not synchronize access to the caching `HashMap`. Instead of introducing synchronization, use a thread safe `ConcurrentHashMap`.
>
> Also use `computeIfAbsent()`?
Decided against it. `findClass` throws a checked exception. The mapping function then become more complex than the current straight implementation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] jtulach commented on a diff in pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "jtulach (via GitHub)" <gi...@apache.org>.
jtulach commented on code in PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#discussion_r1154414097
##########
nbbuild/antsrc/org/netbeans/nbbuild/CustomJavac.java:
##########
@@ -252,4 +276,144 @@ private static boolean isIgnored(
return false;
}
+ private static final class NbJavacLoader extends URLClassLoader {
+ private static final String MAIN_COMPILER_CP = "nbjavac.class.path";
+ private static final String MAIN_COMPILER_CLASS = "com.sun.tools.javac.Main";
+ private final Map<String, Class<?>> priorityLoaded;
+
+ private NbJavacLoader(URL[] urls, ClassLoader parent) {
+ super(urls, parent);
+ this.priorityLoaded = new HashMap<>();
Review Comment:
Thanks for your fix @matthiasblaesing in 42ed77a
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] matthiasblaesing commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "matthiasblaesing (via GitHub)" <gi...@apache.org>.
matthiasblaesing commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1492853749
@jtulach thank you for considering the suggestions and for the separation of the classloader. That indeed looks better readable.
For the modifications to the CI/CD script, I'd like to try to explain what happens there:
We have several tests, that are run only if certain labels are set on the corresponding PR. The idea here is, that github and Apache can't provide infitite processing power and we need to use it were it counts. Two big blocks are PHP and full java tests. Both run in the range of an hour and should not be run on all PRs, just PRs with the label `PHP` or `Java`.
We had already a split of the CI/CD runs into separate parts to allow parallel execution and @mbien added protection to the execution blocks, that checks whether the label is set:
https://github.com/apache/netbeans/blob/e1b594034bdce520dbfbc720a6cd7b7b894fc756/.github/workflows/main.yml#L791-L794
This block says:
The `platform-modules-test1` job is only run if the label `Platform` or `ci:all-tests` is set. Additionally this will be run on non-PR events, for example the merge to master (the last part of the if).
Yes you see rust in the full CI/CD configuration, as the first steps for rust support were added by @vieiro, but are only build by the `full` cluster configuration, not by the `release` config. The line you referenced declares a variable `test_rust`, nothing more, apart from that it is identical to the guard shown above for `Platform`.
I hope the clears the idea a bit.
I just saw your comment over in my "test" PR:
> Is it wise to run the job only when a special label is added? We wouldn't catch https://github.com/apache/netbeans/pull/5748 so quickly without running this job.
I'm in a yes/no situation. I agree, that the nbjavac run was helpful to catch this. On the other hand I can understand @mbien concerns regarding false positive failures from the nbjavac run. A valid middle ground may be, to change this:
https://github.com/apache/netbeans/blob/3027e4274aaa0e8a40c1ab700db67b954d76784c/.github/workflows/main.yml#L162
to:
```
if: ${{ contains(github.event.pull_request.labels.*.name, 'nb-javac') || contains(github.event.pull_request.labels.*.name, 'ci:all-tests') || github.event_name != 'pull_request' }}
```
This is a compromise for both sides: On the one hand the nbjavac job will only be run on specially labeled PRs (`nb-javac` or `ci:all-tests`). On the other hand it will be run on merges to `master` (at least that is my understanding). So normal PRs will not yield more runtime, yet if the unlikely (it happend twice now) case happens, that an API is used which does not match the declared JDK, it can still be caught. The price is IMHO ok, my notebook needs 9 minutes for the NetBeans build and 3 minutes for the commit-validation run, on server class hardware I assume less.
@mbien @neilcsmith-net @jtulach what do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] jtulach merged pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "jtulach (via GitHub)" <gi...@apache.org>.
jtulach merged PR #5609:
URL: https://github.com/apache/netbeans/pull/5609
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] matthiasblaesing commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "matthiasblaesing (via GitHub)" <gi...@apache.org>.
matthiasblaesing commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1464926784
Further idea: My reading of the module system is, that it should be possible to selectively upgrade parts of the JDK runtime (see `--upgrade-module-path` option). That way "nb-javac" would just "be" `jdk.compiler`. No fiddling with the build environment necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] neilcsmith-net commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1477533229
With all the Oracle people talking up the benefit of nb-javac here, you'd have thought that developing and delivering from Oracle and in an Oracle namespace, as originally agreed with ASF legal for us to distribute it, wouldn't have been that hard of a position to achieve! :wink:
A backported compiler is a positive thing (it's a shame `javac` isn't developed like this). While it remains "unofficial", nb-javac remains a problem and something there is always going to be some push to move away from, not deeper ingrain into the platform.
> I need to remind: people who _use NetBeans_ to produce code (as opposed to people _developing NetBeans_), and also people _using NetBeans as tooling platform_ have very different _business_ requirements that require them to run on just specific JDKs (even the IDE), and have hard requirements for the tooling platform's environment
The former "producing code" people I'm less convinced by, at least judging by IntelliJ's popularity! The latter "tooling platform" people I am, and why I keep reiterating the platform is the entire framework.
Agree a lot of this conversation should go on the mailing list though (and realise me adding to it is probably not helping! :smile:)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1477897301
please clarify what this PR is supposed to achieve and how it benefits the project. PRs are supposed to improve the project.
if this intends to actually test nb-javac:
- I would tend to agree with @dbalek + @jlahoda and say it is ok to add a few lines to the ant task if they make it easier for the nb-javac project to run CI tests which compile NetBeans there. I am still not happy about it, since this locks every single module into using the same task in future, which should not be a requirement, since javac works just fine (and a search will show you it is still used).
- I still don't know why nb-javac has to build NetBeans as a test case, it can be literally any other project and doing this via a gradle/maven plugin has actually practical use cases, since those things are still used IRL in contrast to ant
This is however is not what I see in this PR! This PR adds a CI workflow. If nb-javac stages a new build It makes absolutely no sense to test it here, check if NB builds and if not, fix something in nb-javac which is a completely different project. The NetBeans project is also not running maven tests here to check if something fails before upgrading maven - this already happened!
Regarding reproducible builds:
1) I don't see anything indicating that anyone wants to work on this. Interest in CI topics is in general fairly low, as long CI works, nobody cares about it. (and I completely understand, since it is just a tool and a solved problem). Why do I think that would be the case? I migrated travis to gh and the feedback threads I opened on the dev list are basically empty, I almost merged some of the PRs without being able to motivate anyone to approve them (only the usual suspects took a look (thanks again)), and yes I usually added [everyone](4431). Again: as long the build works nobody cares
2) reproducible builds in java do not require to separate the compiler from the JDK, I have helped with reproducible builds before in other projects and that wasn't even on the table there. The JDK is a convenient bundle you can depend on - its a feature not a bug
3) trying to achieve reproducible builds with ~450 ant projects going to be hell.
> It might. But a 'fear that something might happen' should not cause to block a change just because "it might be extended in the future, maybe"
@sdedic I disagree. If there is no clear benefit to the project in the first place, it should be enough of a reason to not merge something (we have done that before). The argument to merge it is the exact same. Someone might find it potentially useful in hypothetical reproducible builds some time in future, just in case someone wanted to swap out a compiler. Again: if this is about a test case for nb-javac -> wrong repository.
regarding JDK 8:
It's dead Jim. For those who haven't migrated yet: maven central is full with NB versions which still support it (and there will be a few more, I am sure). If someone wants to participate, please do so and I am sure we will find a way to branch NB (assuming everyone agrees and we find a release manager) - lets get involved if you have specific requirements - now is the time.
nb-javac does not backport the java ecosystem to JDK 8
I have never been in or heard of a team who didn't allow intellij or android studio because it uses JDK 17 to run on (it wouldn't surprise me at all if they move to 21 shortly after it lands). No matter what the support contracts were. I am sure they exist somewhere but I don't think we have to take them into account in the minimum requirements (again: get involved).
yes this absolutely should have been discussed on the dev list, like I wrote in the first comment of this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] neilcsmith-net commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1476006761
> > but I'd probably limit that to nb-javac update PRs. We don't have unlimited capacity.
>
> OK, guidance to adjust [7de1422](https://github.com/apache/netbeans/commit/7de1422206891a6575c4c29bc727a1f32d25fb4d) welcomed.
Add a GitHub label (eg. `nb-javac`) and only run those specific tests when the PR has that label, such as a nb-javac update PR. The same logic is used throughout the existing CI tests, to only run those relevant to the PR.
@mbien better to answer that question, should he be happy for this to merge at all.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] jtulach commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "jtulach (via GitHub)" <gi...@apache.org>.
jtulach commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1475124147
Thank you @neilcsmith-net:
> Interesting exercise, ... adds complexity to the build,
The complexity is fairly isolated into its own `NbJavacLoader` class. Beyond that there are just [three executable lines](https://github.com/apache/netbeans/pull/5609/files#diff-2e6575e0d5f99cfd495a77cb8c35ab906d34f9dd2ea1aa9c93156a180a237ab3R132) in `CustomJavac` task. I can refactor the `NbJavacLoader` to be a separate top level class, if that feels better.
> and isn't really the best test of nb-javac IMO.
Real project test is always a good test. In fact there are already two cases where the _"dog food test"_ helps `nb-javac` get better:
- warnings that need to be addressed: https://github.com/apache/netbeans/pull/5609#discussion_r1141264240
- https://github.com/JaroslavTulach/nb-javac/pull/11
> If this change helps with testing upstream, it could go in.
Yes, it does help. The testing is ready to go in: https://github.com/JaroslavTulach/nb-javac/pull/17
> We could even consider running that build test in CI here,
Great, 7de1422 adds the GitHub Actions run and [it has succeeded](https://github.com/apache/netbeans/actions/runs/4459083199/jobs/7831316138).
> but I'd probably limit that to nb-javac update PRs. We don't have unlimited capacity.
OK, guidance to adjust 7de1422 welcomed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] matthiasblaesing commented on pull request #5609: nb-javac: "Eat your own dog food" testing
Posted by "matthiasblaesing (via GitHub)" <gi...@apache.org>.
matthiasblaesing commented on PR #5609:
URL: https://github.com/apache/netbeans/pull/5609#issuecomment-1484133563
So turning back to the PR itself and not the politics around it. I had some suggestions/thoughts that address my technical points and might also partially address @mbien concerns:
https://github.com/matthiasblaesing/netbeans/tree/BuildNetBeansWithNbJavac
The branch was rebased onto master to fix the merge conflicts. Changes:
- b4eaeaa6bcad8a4df17c9cc3db9f5311f98764f9: `loadClass` is not synchronized and does not synchronize access to the caching `HashMap`. Instead of introducing synchronization, use a thread safe `ConcurrentHashMap`.
- a655b626ce1ad1569953e882ec68e40826876b1e: Don't kill `-Werror`, instead only suppress the warnings created by the broken `ct.sym` files. A comment was added documenting, that this workaround has to be removed, once the next nbjavac release is there.
- 34902ef592bee4b0d1bb5bea2b9e55259e0d13d9: Add information about the usecase of this approach and the limits it poses.
- f6b8ece1c928d2078288a0eeb3d4de6324d7b9ad: Only run the nbjavac test job when the `Java` label is added to the PR. This is in line with other long running/additional java jobs, that should not be run everytime a change is made somewhere in the codebase
@jtulach could you please have a look at this? Feel free to use the branch or cherry pick the additional commits.
@mbien could you check if that addresses some of you concerns? We can discuss whether nbjavac is really a good idea, but at this point in time it is an integral part of the core of Java editing in NetBeans and I think it might help catching bugs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists