Hi David, 2019年3月20日(水) 14:25 David Holmes <david.hol...@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.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
