lbruun opened a new pull request #2700: URL: https://github.com/apache/netbeans/pull/2700
This is a fix for [NETBEANS-1428] This PR replaces #2021. This PR builds upon the fixes made by **Peter Hull** (I've cherry-picked commits from PR-2021) but corrects a number of issues, changes copyright from Oracle to ASF and finally adds a GitHub Actions workflow which builds the native binary on Windows, Linux and MacOS in a manner which is consistent, transparent and reproducible. The core component of the fix is still Peter's. Peter analyzed the bug, diagnosed it and came up with a solution (partly based on discussion on the OpenJDK mailing list) so all credit to Peter!!. The reason for the bug is that the original C code made an incorrect assumption about how to convert from `jint` to `jmethodID` and in doing so effectively relied on an undocumented JVM internal. This assumption was broken with Java 9 on Windows, hence the crash. Peter's fix is to have a HashMap in the C code which can translate from `jint` to `jmethodID` but only do so on platforms where the two are of different implementation and therefore need conversion. It can be said that the bug existed on all platforms, but because of some JVM internals only _manifests_ itself on the Windows platform. Peter's fix targets _all_ platforms and this is important as we are then immune to the bug if the JVM in future changes its (undocumented) internals for Linux or MacOS. Therefore it is important to re-build binaries not only for Windows, but for other platforms too. The GitHub Workflow added in this PR produce binaries for - Linux x86_64 , 32-bit and 64-bit - Windows x86_64, 32-bit and 64-bit - MacOS x86_64, 64-bit only as all other platforms are now considered unsupported from NetBeans Profiler point of view. The GitHub workflow produces a ZIP file, `profiler-external-binaries-ASF.zip` which can be downloaded from the GitHub Actions UI and which is suitable for upload to https://netbeans.osuosl.org/binaries/. ### Why another PR? Although the base of the fix in #2021, the C code, was/is fully correct, I felt there were too many changes required to make the PR work (reproducible build, rather than as a one-time lucky punch) and only the PR creator would have permission to make those changes. It could potentially take many ping-pong discussions. Also this PR has a clear view of how the fix can be incorporated into the NetBeans distribution following acceptance of the PR. See below. ### Testing this PR or rather : testing the new binaries which are a result of this PR. 1. Be sure that your NetBeans IDE uses a 64-bit JDK. In order to actually see the bug you need to be on Windows and your IDE must be using Java 9 or later. Start a "Methods" Profiling session and you'll immediately experience a crash. (If you are not on this combination then your testing is still valuable because there's a need to verify that the news binaries _still_ works ... the fix to the C code applies to all 64-bit platforms even if the bug only manifests itself on Windows) 1. Download the `profiler-external-binaries-ASF.zip` from the GitHub Actions UI and explode it onto your NetBeans IDE installation directory, folder `netbeans/profiler/` so that files from the ZIP overwrite what you already have on your harddisk. (you should make a backup prior to this of the files that will be overwritten) 1. Restart the IDE. 1. Start a "Methods" Profiling session. It must now work. ### Next steps This is essentially a two-step process which requires not one, but two, PRs. This is the first of those PRs. Once this PR is accepted the GitHub workflow will run and produce artifact. Someone with proper permission must then upload this to https://netbeans.osuosl.org/binaries/. (naming the file `profiler-external-binaries-ASF.zip`) I'll then produce a **follow-up PR** which incorporates the new ZIP in the Ant build scripts like this; Currently the Ant build script looks like this: ```xml <target name="-process.release.files"> <unzip src="external/profiler-external-binaries-8.2.zip" overwrite="false" dest="${profiler.cluster}"/> </target> ``` in the future it will look like this: ```xml <target name="-process.release.files"> <!-- CDDL licensed binaries, from the Oracle days --> <unzip src="external/profiler-external-binaries-8.2.zip" overwrite="false" dest="${profiler.cluster}"/> </target> <!-- AL2 licensed binaries, build from ASF code --> <target name="-process.release.files"> <unzip src="external/profiler-external-binaries-ASF.zip" overwrite="true" dest="${profiler.cluster}"/> </target> ``` In other words: We retain all esoteric binaries from the CDDL licensed bundle but those are **overlayed** with ones created in ASF (meaning those that are AL2 licensed). The new ASF bundle only include those platforms listed at the top and which are probably the only platforms we can truly support going forward. So for the Windows, MacOS and Linux platforms it will be the binaries from the ASF build which are effectively being used ... and since these are the only ones which has been rebuild it is also the only ones which includes the bug fix for [NETBEANS-1428] but I think that is acceptable as we have no knowledge that the bug will manifest itself on those esoteric platforms. Nor do we have any means of re-building for those platforms. So we leave them in the dust. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected] For further information about the NetBeans mailing lists, visit: https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
