On 20/03/2019 4:03 pm, Yasumasa Suenaga wrote:
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.
Or you could add
LIBRARIES = .../libzip.a
inside the existing MINGW section and then change the external
LIBRARIES = ...
to
LIBRARIES += ...
I'm unclear whether MINGW is only an issue on Linux when building for
Windows or whether also of use on Windows itself?
David
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
