RFR(M): 8233787: Break cycle in vm_version* includes
Schmidt, Lutz
lutz.schmidt at sap.com
Fri Nov 15 12:19:29 UTC 2019
Hi Kim,
thanks for reviewing - I understand your comments that way. One more review to go. :-)
I made abstract_vm_version.cpp #include vm_version.hpp, and I updated the copyrights. See the webrev#03:
http://cr.openjdk.java.net/~lucy/webrevs/8233787.03/
I ran the initial webrev iteration through dev-submit and had it active SAP-internally. The current webrev is active since last night SAP-internally. All builds are green. The test show only unrelated issues (some JIT compiler asserts). Of course I will run the final webrev through dev-submit.
Re test coverage: we do not cover 32-bit platforms. And we do not have zero or minimal builds.
Thanks,
Lutz
On 15.11.19, 07:17, "Kim Barrett" <kim.barrett at oracle.com> wrote:
> On Nov 14, 2019, at 11:26 AM, Schmidt, Lutz <lutz.schmidt at sap.com> wrote:
>
> Hi Kim,
>
> that wasn't straightforward. Had to adapt make/hotspot/lib/CompileJvm.gmk. Build settings like HOTSPOT_VERSION_STRING have to flow into the compile step of abstract_vm_version.cpp now. For the details, see my comments below.
Ick, that's unfortunate. Almost makes me regret suggesting the new
file. Oh well. I was going to suggest you should get a review from a
build expert for this, but the changes are quite mechanical and
obvious.
> Other than that, I hope the new webrev is even closer to your dreams:
> http://cr.openjdk.java.net/~lucy/webrevs/8233787.02/
I think abstract_vm_version.cpp should #include vm_version.hpp. That
way, if Abstract_VM_Version provides any shared helper functions that
are defined in terms VM_Version values, it can get any potentially
overridden values.
There currently isn't anything like that, though there could be (and
perhaps should be, though currently presumably doesn't matter). See
jvm_version(), and the initializers for _s_vm_release and
_s_internal_vm_info_string. I think you shouldn't do anyhing about
these references in this change.
There are also a bunch of files with out of date copyrights. I should
have mentioned that before.
Other than that, this looks good.
I assume you are running this through dev-submit and SAPs build farm
to check various platforms. If there are any not covered by that, you
should reach out to appropriate platform maintainers.
More information about the hotspot-dev
mailing list