RFR: 8220784: hsdis cannot be built with MinGW64

[David Holmes]

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:

   ## 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