Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove downloader from NBI #7542

Merged
merged 1 commit into from
Jul 8, 2024
Merged

Conversation

mbien
Copy link
Member

@mbien mbien commented Jul 3, 2024

  • the download manager relied on suspend/resume/stop from java.lang.Thread which were deprecated since Java 1.2
  • JDK 23 removes suspend/resume, others are already no-ops
  • fixing this would require a completely different approach, and since NBI is no longer maintained and in the process to be replaced by nbpackage this is the easiest option

how to test:

  1. setup JDK 23
  2. checkout this branch
  3. build (with zip):
ant clean -q
ant build -Dcluster.config=release
  1. build installers (download installer.sh and run using bash)
bash installer.sh nbbuild/NetBeans-dev-dev-baa644f9077e618cb3555695e0d3f0a07a0db380-release.zip 42 121212

this should create the windows installer:

tree dist/
dist/
├── bundles
│   ├── Apache-NetBeans-42-bin-linux-x64.sh
│   └── Apache-NetBeans-42-bin-windows-x64.exe

which is now testable on windows (not testable on JDK 23ea since the native launcher can't parse ea java versions)

this PR would unblock #7525 (and #7484)
full test on JDK 23 and nb-javac 23 ran in #7538
related to #4952

@mbien mbien added this to the NB23 milestone Jul 3, 2024
This was referenced Jul 3, 2024
Copy link
Contributor

@matthiasblaesing matthiasblaesing left a comment

Choose a reason for hiding this comment

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

At this point I want to Veto this. I understand why it was done, but what I'm missing is analysis of the change for downstream users of NBI and considering alternative approaches.

Given that the JDK has now an async capable http client, there should be a better option, that randomly kill functions without warning.

Edit: I'll have a look at this, but I can't promise how fast.

Copy link
Contributor

@matthiasblaesing matthiasblaesing left a comment

Choose a reason for hiding this comment

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

I just realized that indeed fixing NBI is riding a dead horse and I don't want to focus on it.

I pull my Veto.

Looked through the changes and they look good to me.

@mbien
Copy link
Member Author

mbien commented Jul 3, 2024

@matthiasblaesing tbh my first thought was also to try to reimplement it which I gave up on fairly quickly after going through the code. My second thought was to remove the time slicing scheduler and sequentialize the download to get rid of the suspend/resume at least. But since the threading model leaked through the API, this wouldn't really work (interrupts etc) - and testing this would have been a pain. Any modern downloader implementation would be trivial today but couldn't really be put behind that API.

@neilcsmith-net did also take a closer look through NBI long ago and also decided that it was not worth it, this is what spawned https://github.com/apache/netbeans-nbpackage as you know.

Last but not least: unmaintained net code isn't ideal, even if it would continue to work. We would keep releasing it even though all tests are disabled and nobody is looking at it anymore.

@mbien

This comment was marked as off-topic.

@mbien
Copy link
Member Author

mbien commented Jul 6, 2024

I am planing to merge this relatively soon so that we can start building/testing on JDK 23. I tested this once more on win 10 (in conjunction with #7550) and saw no problems.

@mbien
Copy link
Member Author

mbien commented Jul 8, 2024

will rebase and then merge.

once done, I will do the same with #7525, assuming everything there is still green (can't remember if it also requires nb-javac 23)

edit: actually I believe the nb-javac PR is a precondition of the CI PR

@mbien
Copy link
Member Author

mbien commented Jul 8, 2024

lets wait with merge #7387 (comment) / #7553

 - the download manager relied on suspend/resume/stop from
   java.lang.Thread which were deprecated since Java 1.2
 - JDK 23 removes suspend/resume, others are already no-ops
 - fixing this would require a completely different approach,
   and since NBI is no longer maintained and in the process
   to be replaced by nbpackage this is th easiest option
@mbien mbien merged commit 76e1f35 into apache:master Jul 8, 2024
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants