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/
Thanks, Yasumasa 2019年3月20日(水) 11:25 David Holmes <david.hol...@oracle.com>: > > On 20/03/2019 11:54 am, Yasumasa Suenaga wrote: > > Hi David, > > > > 2019年3月20日(水) 10:38 David Holmes <david.hol...@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.hol...@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
