Closed dukescript closed 4 years ago
-1, for now. By itself doesn't fix a key issue with FOD.
@neilcsmith-net what is the key issue with FOD? Requiring a GPLv2-CPE to be installed can't be it, as it was/is fine to require nbjavac on JDK8. Being asked for each project after an upgrade to install a dependency, can't be, as this was the case also before 11.2 on JDK 8.
I only see one real issue: the bundling of nbjavac and JavaFX on JDKs 14+. This is bad enough, but at this point we have little time and a diffuse problem, so is this the right summary?
@matthiasblaesing we had some dispensation to allow nb-javac to be required early on, and there has been a lot of work to make it more optional. The move from recommended to required for JavaFX was done without full discussion. There was some justification to nb-javac that people are accepting javac to use Java support, which is not necessarily the case with JavaFX. Moving to JavaFX cluster makes sense there.
But that's not the key problem - FOD doesn't handle a requirement not being there gracefully. Whether people want to opt out of installation by choice, or because they can't connect to the download server (there are multiple JIRA issues on this - 3805 and 3810 at least), they need to be able to cancel and not have the IDE lock up and become unusable.
Yes this issue probably does exist for JDK 8 with regard to nb-javac, and may need fixing there too, but making that issue happen on every JDK isn't an improvement.
Interestingly, having JavaFX required in FOD also seems to be the cause of https://issues.apache.org/jira/browse/NETBEANS-3732 bizarrely - see https://lists.apache.org/thread.html/r7ce0738e6bcdfa37c7500ca4b4dc7c82104732d57c8c5cb6cb08bc02%40%3Cusers.netbeans.apache.org%3E
Is this release critical? The autoupdate code is fickely and from my perspective the FOD code falls into the same category. For NETBEANS-3732 there is to much stackato to get a clear message. I would not touch the FOD code now, it will become even more messy.
@matthiasblaesing the only thing I'd like to see for the release is to go back to the recommended behaviour for JavaFX that we had pre-11.3. Something along lines of #2135 or something else minimal. Then we rethink how external dependencies fit in with FOD in 12.1. The existing fix for https://issues.apache.org/jira/browse/NETBEANS-2439 only partially handles the problem and is causing all sorts of other (IMO worse) issues.
As there is not yet consensus, what we will do, I'll add my prototype:
https://github.com/matthiasblaesing/netbeans/commit/3422d1adcbda90618e921c3b3218ecd10b12f626
That retrofits the dialog panel used in the new project wizard into the project open code path. This decouples the nbjavac and javafx decision and only forces JavaFX.
I won't repeat, that I think it was a bad decision of Oracle and the ASF to move NetBeans to an organisation that cuts it off from major parts of the ecosystem.
I think we’re agreed at least on the point that for 12.0, we need the most minimal solution, while 12.1 (just 4 months down the road) we’ll have a more comprehensive solution (javafx functionality in its own cluster, thanks @dukescript), which we could/should even backport to 12.0.
So, if we agree on this, can we agree on what the most minimal solution for 12.0 is, or at least list the alternatives and then choose the most minimal one?
@geertjanw releases are by vote not consensus, so we don't have to agree on a solution. However, as said before, I would probably -1 the release if we go ahead with JavaFX marked as required unless we find another solution to all the knock on issues.
This isn't so much the ASF policy issue (although I think that's still potentially a blocker) it's technical - the fact that this relies on a perfect network connection (no firewall, no wifi problems, no osuosl problems) so basically renders the IDE unusable on first run and drives people to force quit. Before 11.3 they just had one broken wizard panel.
See the related issues in https://issues.apache.org/jira/projects/NETBEANS/issues/NETBEANS-4328 - two of them you've recommended people just don't install nb-javac - that's more difficult to achieve with this in play!
What are the most minimal solutions that we should choose between for 12.0? Going ahead as is can’t be the right approach since we know up front this is a blocker.
@geertjanw well, is it even agreed it's a blocker?! We should probably move this discussion to dev@? - not everyone follows every PR. I only commented here because this PR, while a good step forward IMO, doesn't in itself solve the network issue.
That’s true, yes. :-)
Do you think you could explain this super succinctly on dev@? It seems there are two or three different items at play at the same time and most of us, me especially, may not be looking at the full picture.
...for 12.1 (just 4 months down the road) we’ll have a more comprehensive solution (javafx functionality in its own cluster, thanks @dukescript),
Let's get some reviews, approvals and let's make it now!
If this means that anything JavaFX related, including that nb-javac/JavaFX dialog in the New Project dialog (which will then not be needed) is only shown if JavaFX is enabled, then this is great.
In general this looks good. However, I think JavaFX needs to be hard dependency in this case.
@matthiasblaesing I don't think anyway is disputing that JavaFX should be a hard dependency for this wizard, or that that particular issue is a problem. But handling it via the require flag in FOD isn't the answer. And unless we change how that works, it triggers loads of other issues. It breaks import settings, open project and the options window, sometimes even in cases where the user does download the modules (eg. NETBEANS-3732) It also doesn't help if the user bypasses FOD by enabling the cluster directly, migrates from JDK 8, or if we don't ship a build with ergonomics.
I think we need to rethink how to fix NETBEANS-2439, and any other time a feature / wizard has a hard requirement on an optional dependency. But does this PR need changing to get there?
I am a hacker but why can't you just select a javafx sdk the way you do with a java jdk "undo the split" in Netbeans"
WARNING [org.netbeans.modules.ide.ergonomics.fod.ModulesActivator]: java.lang.IllegalArgumentException: Not all requested modules can be enabled: [StandardModule:org.netbeans.modules.javafx2.kit jarFile: /home/matthias/x/netbeans/javafx/modules/org-netbeans-modules-javafx2-kit.jar]
The error message in the dialog shall be improved by c17a078. The bc31e77 then enables the download button as a hint of what to do next.
Can anybody enumerate the TODOs that have to be fulfilled before merging this PR? I see Matthias requested some changes, but his comment doesn't state which ones.
... rethink how to fix NETBEANS-2439, ... But does this PR need changing to get there?
The DukeScript supporters agreed to unblock the release 12.0 by allowing regression in NETBEANS-2439 which renders DukeScript support in NetBeans 12.0 useless for almost everyone who tries it.
I don't see any problem in requiring download of JavaFX framework when one wants to use JavaFX(!) functionality.
Let us fix NETBEANS-2439 by requiring download of JavaFX libraries when using JavaFX cluster. Should any other problems appear then, report a bug and we fix it by 12.1.
@dukescript the "fix" for NETBEANS-2439 missed out multiple ways that 2439 was triggered, and caused a load of other (including blocking) issues. No one seems to be suggesting not requiring JavaFX where it's required. Just not using the required flag in FOD to achieve it, unless the issues that triggers can all be addressed first. Have you read all the JIRA issues that the fix for 2439 caused?
Frankly, looking at all this, I wonder if it is time for another attempt to either improve the policy, or at least get an exception from the policy(*), to allow us to simply include GPLv2+CPE in convenience binaries. The list of problems that could be improved by that is getting longer, not shorter (as of recently, pack200 could be added to that list as well - the license is not the only problem, but definitely one of them).
(*) I think the fact that NetBeans is an end-user tool could be helpful in that discussion. And the list of problems users experience could be at least a supporting factor.
Can anybody enumerate the TODOs that have to be fulfilled before merging this PR? I see Matthias requested some changes, but his comment doesn't state which ones.
Sorry - my point was, that the activate button in the dialog is useless, as it does not work. Download and Activate was required. But given the issue Neil mentioned about this, maybe as an intermediate step, this should be merged as-is, as it lessens the pain.
I agree with @jlahoda we should try to get the option to redistribute GPLv2-CPE artifacts. But that is a different discussion IMHO. I can understand that @dukescript wants this merged and while not perfect, I think it improves the situation.
Have you read all the JIRA issues that the fix for 2439 caused?
All the reports were associated with Java projects. None of them is going to be reproducible after this PR moves the bits to JavaFX cluster. All of them will have to be reproduced again differently, with more unusual scenarios. Making all of them low priority problems. Certainly less priority than 2439.
another attempt to either improve the policy, or at least get an exception from the policy
+10, but outside of this PR please.
Download and Activate was required.
I see. Yes, it is required and it would be correct to state it in the ergonomics config, if Neil let it happen.
Btw. Right now it’s just the DukeScript wizard, but maybe the JavaFX guys José & Johan would be also glad that they can use JavaFX in the JavaFX cluster when coding the NetBeans support for the JavaFX framework.
All the reports were associated with Java projects. None of them is going to be reproducible after this PR moves the bits to JavaFX cluster.
Not sure that's true. At the least it may just move the same problems only to JavaFX projects, or parts of it might trigger if someone, at some point, hit the JavaFX tab in settings.
Making all of them low priority problems. Certainly less priority than 2439.
a) the fix you had for 2439 didn't address multiple triggers of it - worked through various with users during releases last year; and b), non-cancellable dialogs and lockups on network issues, even if they only affect a smaller subset of users, are still higher priority issues. As probably is opening existing projects not working without IDE restart, which needs testing with the cluster move.
Download and Activate was required.
I see. Yes, it is required and it would be correct to state it in the ergonomics config, if Neil let it happen.
So fix the UI to only have one button now we have checkboxes? And even revert my change to leave the checkbox enabled. The problem is not the UI or the fact this project needs JavaFX downloading, it's the behaviour the internal required flag triggers elsewhere.
But ideally we'd address 2439 in a way that works if users bypass FOD by enabling the cluster directly (which some people still do), or use JDK 8 initially, etc. Do we not have a better way that projects / wizards can request specific plugins be found (in any UC) and install UI initiated?!
maybe as an intermediate step, this should be merged as-is
Yes, please. If anyone feels this way, please approve the PR to be merged as it is. Let's fix the related bugs by 12.1.
...Not sure that's true...might trigger...
Well, that's not a blocker. On contrary. The sooner we merge, the more time we have to address the real issues. E.g.
address multiple triggers of 2439
Let's fix multiple triggers then.
non-cancellable dialogs
Let's make them cancellable then.
opening existing projects
Opening existing (non-JavaFX) projects is certainly going to work smoothly. Possibly even the JavaFX ones could work without restart.
Do we not have a better way
There isn't a better way right now and we are not going to have one, unless there is a problem to fix. Again, let's merge soon and fix by 12.1, please.
Yes, please. If anyone feels this way, please approve the PR to be merged as it is. Let's fix the related bugs by 12.1.
Is anyone still holding this up? @matthiasblaesing requested changes but then suggested merging as is. I reviewed and approved already 4 days ago when requested. Anyone else got an issue?
I haven't checked fully the commits post-review, but I'm assuming fine - I was more concerned that addressing Matthias' comment would involve reverting https://github.com/apache/netbeans/pull/2135/files That quick fix will need looking at, but as long as it's not reverted in this PR, I'm not holding it up.
Let's make them cancellable then.
I did already, we just need a better approach. It's not just JavaFX that's a problem here either.
Let's fix multiple triggers then.
I'm not sure you can fix all the triggers without using something other than ergonomics to do it. Which would be a good move, and something to look at next? Let's discuss on dev@ Don't get me wrong, I want it to work too!
...Not sure that's true...might trigger...
Please don't selectively quote if you're going to change the meaning of what I said!
Rather than regressing with respect to NETBEANS-2439 as #2135 suggests, let's move the functionality that depends on JavaFX to JavaFX cluster. By doing it regular users of plain Java functionality are not going to be bothered to download JavaFX libraries at all.
Only those who care about JavaFX will be asked to download JavaFX libraries - which seems more acceptable.
The change deserves a lot of testing. It would be better to apply it after 12.0, not now. However rather than integrating #2135, let's risk this change.