Hello,

Looks good to me.

Fixing jexec is a separate issue and not really build related.

/Erik

On 2017-02-14 17:27, Baesken, Matthias wrote:

Hello, here is  the webrev :

http://cr.openjdk.java.net/~mbaesken/webrevs/8174242/ <http://cr.openjdk.java.net/%7Embaesken/webrevs/8174242/>

bug :

https://bugs.openjdk.java.net/browse/JDK-8174242

As discussed , I deleted the macosx   jexec.c    .

Btw. I found out that jexec does not work (fully?) in jdk8 / 9 ( seems this has to do with the change 8156478: 3 Buffer overrun defect groups in jexec.c )

Executing a         helloworld.jar  with  jexec   leads to

     “invalid file (bad magic number): Exec format error”  .

Should I open another bug for this ?

Best regards, Matthias

*From:*Baesken, Matthias
*Sent:* Freitag, 10. Februar 2017 13:19
*To:* 'Erik Joelsson' <[email protected]>; [email protected]; [email protected]
*Cc:* Langer, Christoph <[email protected]>
*Subject:* RE: jdk10 : simplify jexec build settings

Hi Eric, thanks for the comment, I think I’ll remove the macosx source file .

Btw. is there already some test for jexec (a “fast grep” through the jdk tests did not show me much) ?

Best regards, Matthias

*From:*Erik Joelsson [mailto:[email protected]]
*Sent:* Freitag, 10. Februar 2017 09:49
*To:* Baesken, Matthias <[email protected] <mailto:[email protected]>>; [email protected] <mailto:[email protected]>; [email protected] <mailto:[email protected]> *Cc:* Langer, Christoph <[email protected] <mailto:[email protected]>>
*Subject:* Re: jdk10 : simplify jexec build settings

Since the file has never been built, I would say remove the macosx source file. You can point directly to the unix source dir if you prefer too. No need to have dead logic for macosx in the makefile.

/Erik

On 2017-02-09 15:27, Baesken, Matthias wrote:

    I compared the  jexec.c  versions for macosx and unix

    ( jdk/src/java.base/macosx/native/launcher/jexec.c    and
     jdk/src/java.base/unix/native/launcher/jexec.c )

    And to me it looks like  the  unix version could be used for
    macosx too  (just in case there is a need to reenable the  build
    of jexec.c  for macosx one day again).

    Comments / suggestions ?

    Best regards, Matthias

    *From:*Baesken, Matthias
    *Sent:* Donnerstag, 9. Februar 2017 14:10
    *To:* 'Erik Joelsson' <[email protected]>
    <mailto:[email protected]>; [email protected]
    <mailto:[email protected]>;
    '[email protected]
    <mailto:[email protected]>'
    <[email protected]>
    <mailto:[email protected]>
    *Cc:* Langer, Christoph <[email protected]>
    <mailto:[email protected]>
    *Subject:* RE: jdk10 : simplify jexec build settings

    Hello Erik, thanks for the comments .

    >>Then there is handling for macosx left , but the build is not enabled
    for macosx, does it still make sense to include the macosx
    handling (there is even a separate jexec.c for macosx) :

    >

    >This is indeed weird. I don't think we ever built jexec for macosx so the 
source file should likely
    be removed. That is however a question for core-libs I think.

    I include core-libs-dev , can someone comment on  jexec on macosx ?

    Currently  jexec.c  is not compiled for macosx (jdk9/10), see
     jdk/make/launcher/Launcher-java.base.gmk  .

    It is just compiled on linux .

    So I wonder -  is it ok to remove  the macosx handling  in the
    mentioned  makefile jdk/make/launcher/Launcher-java.base.gmk   for
     jexec ?

    >This will automatically look in all the directories where source files are 
supposed to be and pick
    the most specific one if there are any with the same name.

    >Since the share launcher dir contains more src files, you will need to 
leave the INCLUDE_FILES
    := jexec.c this time.

From what I observe in the linux build log, just jexec.c is compiled into jexec.o and then this single object is used to
    create the executable (“program”)  jexec.

    Other src files are not used in the compilation of jexec, so  just
    using   $(JDK_TOPDIR)/src/$(MODULE)/unix/native/launcher would be
    fine (as long as macosx can be dropped).

    Best regards, Matthias

    *From:*Erik Joelsson [mailto:[email protected]]
    *Sent:* Donnerstag, 9. Februar 2017 13:42
    *To:* Baesken, Matthias <[email protected]
    <mailto:[email protected]>>; [email protected]
    <mailto:[email protected]>
    *Cc:* Langer, Christoph <[email protected]
    <mailto:[email protected]>>
    *Subject:* Re: jdk10 : simplify jexec build settings

    Hello,

    On 2017-02-09 12:36, Baesken, Matthias wrote:

        Hello , while adjusting the jspawnhelper build settings with
        8174086, it has been noticed that the jexec build settings
        need some simplification as well.

        The bug

        https://bugs.openjdk.java.net/browse/JDK-8174242

        has been created for this.

        When looking into it,  I had some questions :

        
http://hg.openjdk.java.net/jdk10/jdk10/jdk/file/ae7afa9abe67/make/launcher/Launcher-java.base.gmk

        The makefile  (make/launcher/Launcher-java.base.gmk )  handles
Solaris 32bit, but is this really supported in jdk 9 or 10 ( I think it was removed in 9 and only 64bit Solaris support
        remains ) ?

    We do not support 32bit Solaris. The issue with jexec for Solaris
    was raised a while back and the conclusion was to scrap support,
    so no need to build it for Solaris at all.

        ifeq ($(OPENJDK_TARGET_OS), solaris)

          ifeq ($(OPENJDK_TARGET_CPU_BITS), 32)

            BUILD_JEXEC := 1

          endif

        endif

        ifeq ($(OPENJDK_TARGET_OS), linux)

          BUILD_JEXEC := 1

        endif # OPENJDK_TARGET_OS

        #

        # jdk/make/java/jexec/Makefile

        #

        ifeq ($(BUILD_JEXEC), 1)

        Then there is handling for macosx left , but the build is not
        enabled for macosx, does it still make sense to include the
        macosx handling (there is even a separate jexec.c for macosx) :

    This is indeed weird. I don't think we ever built jexec for macosx
    so the source file should likely be removed. That is however a
    question for core-libs I think.

    The makefile logic shouldn't need to be that specific though. Our
    modern pattern for this is something like this:

    SRC := $(call uniq, $(wildcard \
    $(JDK_TOPDIR)/src/$(MODULE)/$(OPENJDK_TARGET_OS)/native/launcher \
    $(JDK_TOPDIR)/src/$(MODULE)/$(OPENJDK_TARGET_OS_TYPE)/native/launcher
    \
    $(JDK_TOPDIR)/src/$(MODULE)/share/native/launcher))

    This will automatically look in all the directories where source
    files are supposed to be and pick the most specific one if there
    are any with the same name. Since the share launcher dir contains
    more src files, you will need to leave the INCLUDE_FILES :=
    jexec.c this time.

    For libraries, we have abstracted this construct in the macro
    FindSrcDirForLib (LibCommon.gmk) but we haven't done the same for
    launchers yet.

    /Erik

          ifeq ($(OPENJDK_TARGET_OS), windows)

          else ifeq ($(OPENJDK_TARGET_OS), macosx)

            BUILD_JEXEC_SRC :=
        $(JDK_TOPDIR)/src/java.base/macosx/native/launcher

          else

            BUILD_JEXEC_SRC :=
        $(JDK_TOPDIR)/src/java.base/unix/native/launcher

          endif

        Should I remove  the solaris 32bit / macosx handling  for
        jexec  from  make/launcher/Launcher-java.base.gmk ?

        Regards, Matthias


Reply via email to