-
Notifications
You must be signed in to change notification settings - Fork 6.2k
8374372: Move OSX Serviceability Agent to macosx namespace #29003
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
base: master
Are you sure you want to change the base?
Conversation
The Mac OSX implementation of the Serviceability Agent and related code is quite different from what's needed from the BSD implementation. Still we have tried to keep the coexisting in one codebase in the out-of-tree BSD port, as that's where the OSX code has been living. This sometimes cause problems when updates to the Mac OSX port breaks the BSD implementation. As we are working on getting the BSD port into a state for future upstreaming to the mainline repo, this patch clears the path by moving the Mac OSX implementation of the Servicability Agent to a more fitting namespace. This should allow us to proceed with the BSD implementation undisturbed, but also without risking breaking the OSX port. This work was sponsored by The FreeBSD Foundation
|
👋 Welcome back haraldei! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
dholmes-ora
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about the approach here. My understanding was that "bsd" covered both macOS and potentially other BSD derivatives, while "darwin" provided macOS specific code. You seem to be indicating that the existing "bsd" code only works for macOS and so have renamed it all. If so that is fine (though a little surprising), but then having "macos" and "darwin" as alternatives really makes no sense to me. ??
Also while we have become stuck with using "macosx" in many places in the code, if we can we should use macos/macOS here.
Yes, traditionally it seems the OS X port was grown from the BSD port, but have since diverged significantly. In the BSD port maintained out of tree, a lot of the code under the
The BSD code was broken in commit d3083ac, and in trying to figure out how to fix it again it seemed easier to split the two implementations than to try to force them to coexist. This way the platforms would have less risk of breaking each others implementations.
I can rename it to darwin instead, if you feel that is better. It seemed like "macosx" was more established when I looked around the source tree, but I'm happy to adapt. |
My conclusion is that this is all rather poorly thought out and very inconsistent. I don't yet have a suggestion on how to proceed. I just wanted to better describe where things are at now. |
|
"darwin" seems to be mainly used within the SA code and it seems to be an alias for macOS/macosx - though why it was done that way I'm unsure. May be uname has "darwin" in it and so it is exposed via a system property? (Theoretically you could have a different OS built upon the XNU - aka Darwin - kernel, but we are not trying to support that in any way. I also see this in java_props_md.c Regardless it makes no sense to have macOS and Darwin supported by SA so some major renaming would also be needed there. The macosx naming is legacy from the days of the mac port when it was known as OS X. For a long time now it is just macOS but we didn't do a major renaming as that would be too disruptive. But if we are creating new files here then we can use the most appropriate naming scheme and not be caught up in legacy issues. |
Okay that is very recent - and very unfortunate. But as we don't know if/when the out-of-tree BSD port will come back into mainline, I would not want to see the existing BSD support effectively gone in JDK 27+. To that end we may need to look at specifically fixing what broke in that commit. Though the BSD specific changes there look innocuous to me. |
The Mac OSX implementation of the Serviceability Agent and related code is quite different from what's needed from the BSD implementation. Still we have tried to keep the coexisting in one codebase in the out-of-tree BSD port, as that's where the OSX code has been living.
This sometimes cause problems when updates to the Mac OSX port breaks the BSD implementation.
As we are working on getting the BSD port into a state for future upstreaming to the mainline repo, this patch clears the path by moving the Mac OSX implementation of the Servicability Agent to a more fitting namespace.
This should allow us to proceed with the BSD implementation undisturbed, but also without risking breaking the OSX port.
This work was sponsored by The FreeBSD Foundation
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/29003/head:pull/29003$ git checkout pull/29003Update a local copy of the PR:
$ git checkout pull/29003$ git pull https://git.openjdk.org/jdk.git pull/29003/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 29003View PR using the GUI difftool:
$ git pr show -t 29003Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/29003.diff
Using Webrev
Link to Webrev Comment