RFR: 8220784: hsdis cannot be built with MinGW64

[Yasumasa Suenaga]

Hi David,

2019年3月20日(水) 14:25 David Holmes <david.holmes at oracle.com>:
>
> Hi Yasumasa,
>
> On 20/03/2019 3:01 pm, Yasumasa Suenaga wrote:
> > Thanks David!
> >
> > I uploaded new webrev.
> > This change affects Windows only, and I confirmed it works fine for
> > Linux x64 and MinGW64.
> >    http://cr.openjdk.java.net/~ysuenaga/JDK-8220784/webrev.01/
>
> I this is for MINGW only then shouldn't the change be made within this
> section:

Did you say we should determine with $(MINGW) whether libz.a is added
to LDFLAGS?
webrev is here:
  http://cr.openjdk.java.net/~ysuenaga/JDK-8220784/webrev.02/

The code is added to outside of the section you pointed.

$(LIBRARIES) is available after OS-specific code.
So we need to change like webrev.02 to minimize changes.


Yasumasa


>    ## OS = Linux ##
>    ifeq         ($(OS),Linux)
>      ifneq           ($(MINGW),)
>        LIB_EXT          = .dll
>        CPPFLAGS += -I$(TARGET_DIR)/include
>        LDFLAGS += -L$(TARGET_DIR)/lib
>        OS=windows
>        ifneq           ($(findstring x86_64-,$(MINGW)),)
>          ARCH=amd64
>        else
>          ARCH=i386
>        endif
>        CC               = $(MINGW)-gcc
>        CONFIGURE_ARGS= --host=$(MINGW) --target=$(MINGW)
>      else   #linux
> ?
>
> David
> -----
>
> >
> > Thanks,
> >
> > Yasumasa
> >
> >
> > 2019年3月20日(水) 11:25 David Holmes <david.holmes at oracle.com>:
> >>
> >> On 20/03/2019 11:54 am, Yasumasa Suenaga wrote:
> >>> Hi David,
> >>>
> >>> 2019年3月20日(水) 10:38 David Holmes <david.holmes at oracle.com>:
> >>>>
> >>>> Hi Yasumasa,
> >>>>
> >>>> I'm not familar with building hsdis, but if the only currnet problem is
> >>>> on Windows, why is the fix not restricted to only building on Windows?
> >>>
> >>> For Solaris and Linux, -lz is not added to LDFLAGS. I think it means
> >>> initial committer want to use zlib in binutils.
> >>> And zlib in binutils will be built implicitly in build phase in binutils.
> >>>
> >>>> Otherwise we need people who build hsdis on other platforms to comment
> >>>> on the appropriateness of the fix.
> >>>
> >>> If using different zlib is used by platforms is allowed, I will upload
> >>> webrev which affects Windows only.
> >>> What do you think?
> >>
> >> That seems preferable/simpler. Then you only need to know that your
> >> change works for non-MinGW64 environments.
> >>
> >> Thanks,
> >> David
> >>
> >>>
> >>> Thanks,
> >>>
> >>> Yasumasa
> >>>
> >>>
> >>>> Thanks,
> >>>> David
> >>>>
> >>>> On 20/03/2019 11:07 am, Yasumasa Suenaga wrote:
> >>>>> Hi Erik, David,
> >>>>>
> >>>>> I checked this change on Linux x64 and MinGW for Windows.
> >>>>> According to hsdis README, we need to use MinGW cross compiler to
> >>>>> build hsdis [1]. So I think Cygwin is not required.
> >>>>>
> >>>>> I do not have macOS and AIX. So I cannot check this change on them.
> >>>>>
> >>>>> BTW is hsdis included Java SE spec?
> >>>>> hsdis seems not to be included jtreg tests, and it is not contained in
> >>>>> OpenJDK binaries.
> >>>>> I think it is allowed even if we lack some test for hsdis if hsdis is
> >>>>> not required for Java SE.
> >>>>> If not so, I want sponsor(s) for this change.
> >>>>>
> >>>>>
> >>>>> Thanks,
> >>>>>
> >>>>> Yasumasa
> >>>>>
> >>>>>
> >>>>> [1] http://hg.openjdk.java.net/jdk/jdk/file/ddfb658c8ce3/src/utils/hsdis/README#l91
> >>>>>
> >>>>> 2019年3月20日(水) 6:54 David Holmes <david.holmes at oracle.com>:
> >>>>>>
> >>>>>> CC'ing hotspot-dev. I agree this needs to be checked on every platform
> >>>>>> affected. I can't comment on the fix itself.
> >>>>>>
> >>>>>> David
> >>>>>>
> >>>>>> On 20/03/2019 2:36 am, Erik Joelsson wrote:
> >>>>>>> I think this needs to be reviewed by at least someone in hotspot who
> >>>>>>> regularly builds hsdis. I can't really comment on the validity of the
> >>>>>>> patch as I'm unfamiliar with both hsdis as well as this makefile. Have
> >>>>>>> you at least verified the build on all the platforms which you affect
> >>>>>>> with this change (which would be at least Macos, AIX and Windows in a
> >>>>>>> normal Cygwin VS env)?
> >>>>>>>
> >>>>>>> /Erik
> >>>>>>>
> >>>>>>> On 2019-03-18 17:56, Yasumasa Suenaga wrote:
> >>>>>>>> Hi all,
> >>>>>>>>
> >>>>>>>> Please review this change:
> >>>>>>>>
> >>>>>>>>       JBS: https://bugs.openjdk.java.net/browse/JDK-8220784
> >>>>>>>>       webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8220784/webrev.00/
> >>>>>>>>
> >>>>>>>> I attempt to build hsdis for Windows on WSL Ubuntu 18.04 with
> >>>>>>>> gcc-mingw-w64-x86-64, but I saw error messages that some functions
> >>>>>>>> which are provided by zlib are unresolved.
> >>>>>>>> We need to link to zlib.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Thanks,
> >>>>>>>>
> >>>>>>>> Yasumasa