RFR(M): 8233787: Break cycle in vm_version* includes

[Schmidt, Lutz]

Thank you, Erik,
for confirming the build change. And sorry for not including you immediately. 
Regards,
Lutz

P.S.: And thanks, Daniel, for including the build team.

On 15.11.19, 17:31, "Erik Joelsson" <erik.joelsson at oracle.com> wrote:

    Build change looks ok.
    
    /Erik
    
    On 2019-11-15 07:08, Daniel D. Daugherty wrote:
    > Adding build-dev at ... since this changeset now touches
    > make/hotspot/lib/CompileJvm.gmk. The Build team has a standing
    > request to be included on reviews that touch makefiles...
    >
    > Dan
    >
    >
    > On 11/14/19 11:26 AM, Schmidt, Lutz 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.
    >>
    >> Other than that, I hope the new webrev is even closer to your dreams:
    >>     http://cr.openjdk.java.net/~lucy/webrevs/8233787.02/
    >>
    >> Thanks,
    >> Lutz
    >>
    >>
    >> On 14.11.19, 00:34, "Kim Barrett" <kim.barrett at oracle.com> wrote:
    >>
    >>      > On Nov 13, 2019, at 11:42 AM, Schmidt, Lutz 
    >> <lutz.schmidt at sap.com> wrote:
    >>      >
    >>      > Hi Kim,
    >>      >
    >>      > there is a new webrev at 
    >> http://cr.openjdk.java.net/~lucy/webrevs/8233787.01/
    >>      >
    >>      > It should be pretty close to what you view as the "right 
    >> approach". There weren't too many changes relative to 8233787.00. 
    >> Most files already had #include runtime/vm_version.hpp.
    >>           This looks much better to me, but many (most?) of the changed
    >>      #includes need to be moved into sort order.
    >>
    >> R: tried my best to fix the sort order. Sorry for not paying 
    >> attention in the first place.
    >> ------------------------------------------------------------------------------
    >>      src/hotspot/share/runtime/vm_version.cpp
    >>           Abstract_VM_Version definitions should be moved to 
    >> abstract_vm_version.cpp.
    >>      Maybe just rename the file; I think the only thing that would be 
    >> left
    >>      for vm_version.cpp would be VM_Version_init().  But maybe that 
    >> should
    >>      be left behind in vm_version.cpp?  Though that makes the review 
    >> messier.
    >>
    >> R: Everything moved as you suggested. Doesn't make sense to have 
    >> Abstract_VM_Version:: methods in vm_version.cpp file.
    >> ------------------------------------------------------------------------------
    >>      src/hotspot/share/runtime/abstract_vm_version.hpp
    >>           Should #include globalDefinitions.hpp.
    >>      - uint64_t features()
    >>      - #define SUPPORTS_NATIVE_CX8
    >>
    >> R: Done.
    >>           Should forward-declare class outsputStream.
    >>      - print_virtualization_info
    >>      - print_matching_lines_from_file (I wonder why this is *here*, 
    >> but not your problem)
    >>
    >> R: Done.
    >> ------------------------------------------------------------------------------
    >>
    >