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

Drop jprocesses in favor of java.lang.ProcessHandle #16521

Closed
famod opened this issue Apr 14, 2021 · 7 comments · Fixed by #16529
Closed

Drop jprocesses in favor of java.lang.ProcessHandle #16521

famod opened this issue Apr 14, 2021 · 7 comments · Fixed by #16529
Assignees
Labels
area/housekeeping Issue type for generalized tasks not related to bugs or enhancements
Milestone

Comments

@famod
Copy link
Member

famod commented Apr 14, 2021

Description

As per #16249 (comment)

jprocesses has not seen any activity for 3 years and with JDK 8 support now dropped (in main), the OOTB ProcessHandle should be used instead.

Implementation ideas

Remove code, write a tiny bit of new code, remove dependency - be happy! 😉

@famod famod added the area/housekeeping Issue type for generalized tasks not related to bugs or enhancements label Apr 14, 2021
@famod
Copy link
Member Author

famod commented Apr 14, 2021

/cc @gastaldi @geoand

@geoand
Copy link
Contributor

geoand commented Apr 14, 2021

Interesting, I didn't even know about jprocesses.

I actually had custom code for the IDE link handling that I just removed, so I'm obviously +1 for this

@gastaldi
Copy link
Contributor

@famod fancy providing a PR?

@gastaldi
Copy link
Contributor

I'm guessing the code in DevModeTestUtils would be something like:

    public static void killProcesses(String... cmdParts) {
        ProcessHandle.allProcesses()
                .filter(processHandle -> {
                    String cmdLine = processHandle.info().commandLine().orElse(null);
                    if (cmdLine == null) {
                        return false;
                    }
                    for (String part : cmdParts) {
                        if (cmdLine.contains(part)) {
                            return true;
                        }
                    }
                    return false;
                })
                .forEach(ProcessHandle::destroyForcibly);
    }

@famod
Copy link
Member Author

famod commented Apr 14, 2021

Looks like you implemented it already. 😄

@famod
Copy link
Member Author

famod commented Apr 14, 2021

But yeah, I can provide a PR later this week.

@gastaldi
Copy link
Contributor

@famod I put my code in a PR for your appreciation: #16529

@famod famod assigned gastaldi and unassigned famod Apr 14, 2021
gastaldi added a commit to gastaldi/quarkus that referenced this issue Apr 14, 2021
gastaldi added a commit to gastaldi/quarkus that referenced this issue Apr 14, 2021
gastaldi added a commit to gastaldi/quarkus that referenced this issue Apr 14, 2021
gastaldi added a commit to gastaldi/quarkus that referenced this issue Apr 14, 2021
gastaldi added a commit to gastaldi/quarkus that referenced this issue Apr 15, 2021
gastaldi added a commit to gastaldi/quarkus that referenced this issue May 19, 2021
gastaldi added a commit to gastaldi/quarkus that referenced this issue May 19, 2021
@quarkus-bot quarkus-bot bot added this to the 2.0 - main milestone May 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/housekeeping Issue type for generalized tasks not related to bugs or enhancements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants