On Tue, Nov 20, 2018 at 1:46 PM Magnus Ihse Bursie
<magnus.ihse.bur...@oracle.com> wrote:
>
> Hi Volker,
>
> Glad to see you fix and clean up the devkit scripts! And you're more
> than welcome to add documentation to the building.md.
>
OK, I've added a section about devkits to build.{md,html}. Please have
a look and let me know what you think. Any comments are welcome :)
http://cr.openjdk.java.net/~simonis/webrevs/2018/8213698.v2/
Please find my comments to the other changes inline
>
> I think all your changes are good, and can go in as-is, but I see some
> further potential for cleanup, so if you feel like some more fixes while
> you're at it, please go ahead. Otherwise you can ignore the rest of what
> I'm writing. :)
>
> > make cross_compile_target="ppc64-linux-gnu" BASE_OS=Fedora
> > BASE_OS_VERSION=17
> For consistency, maybe we should rename cross_compile_target to TARGET?
> At least as written on the make command line; you can keep the
> cross_compile_target name in the Makefile itself, and only assign to it
> from TARGET.
>
I've renamed it to TARGETS because it can actually take a whole list
of targets (and also to distinguish it from the TARGET parameter used
in Tools.gmk which only accepts a single TARGET).
> > OEL_URL :=
> > https://archives.fedoraproject.org/pub/archive/$(FEDORA_TYPE)/releases/$(BASE_OS_VERSION)/Everything/$(ARCH)/os/Packages/
> Maybe it's time to rename this to something more distro-agnostic than
> OEL_URL...
>
Renamed it to BASE_URL
> > PREFIX=$(RESULT)/$@-to-$$p
> I really like this! Devkits are much more better described as X-to-Y
> (even if that was not originally the case, as Erik said).
>
Thanks :)
> > Notice that we have to build "ccache" for each target because ccache
> > will be installed into the directory specified by "--prefix" at
> > configure time and this is now different for every target. However
> > that's not a big problem, because the time for compiling ccache is
> > negligible compared to the download time of the RPMs and the build
> > time of GCC.
> Is it still worth the effort of compiling ccache? At least in Oracle,
> we're not using it anymore. OTOH, if it works, we can just keep it
> there. It's no big cost.
>
I'm not using it either, but I leave it in for now.
>
> > - Cleanup arch selection:
> >
> > ifeq ($(ARCH),x86_64)
> > - RPM_ARCHS := x86_64 noarch
> > + RPM_ARCHS := $(ARCH) noarch
> > ifeq ($(BUILD),$(HOST))
> > ifeq ($(TARGET),$(HOST))
> > # When building the native compiler for x86_64, enable mixed mode.
> > @@ -199,7 +206,7 @@
> > endif
> > endif
> > else ifeq ($(ARCH),i686)
> > - RPM_ARCHS := i386 i686 noarch
> > + RPM_ARCHS := $(ARCH) i386 noarch
> > else ifeq ($(ARCH), armhfp)
> Looks like you could always to RPM_ARCHS := $(ARCH) noarch, and then use
> += for the few platforms that need additionals RPM archs.
>
Done.
> There was something more I thought of when I reviewed your changes, but
> it slipped my mind now.
>
There's still room for improvements :) For example the GCC version
could be made configurable, as well as the OEL version. But I leave
that to the person who updates the compiler the next time :)
> /Magnus
>
>
> On 2018-11-12 12:19, Volker Simonis wrote:
> > Hi,
> >
> > can I please have a review for the following change which ads support
> > for linux/ppc64/ppc64le/s390x devkits and hopefully improves the
> > creation of devkits a little bit :)
> >
> > http://cr.openjdk.java.net/~simonis/webrevs/2018/8213698/
> > https://bugs.openjdk.java.net/browse/JDK-8213698
> >
> > With these changes it becomes possible to say any of the following:
> >
> > make cross_compile_target="ppc64le-linux-gnu s390x-linux-gnu" BASE_OS=Fedora
> > make cross_compile_target="ppc64-linux-gnu" BASE_OS=Fedora
> > BASE_OS_VERSION=17
> > make onlytars cross_compile_target="ppc64-linux-gnu ppc64le-linux-gnu
> > s390x-linux-gnu"
> >
> > and get the following devkits under "build/devkit/result":
> >
> > sdk-x86_64-unknown-linux-gnu-to-ppc64le-linux-gnu-20181112.tar.gz
> > sdk-x86_64-unknown-linux-gnu-to-ppc64-linux-gnu-20181112.tar.gz
> > sdk-x86_64-unknown-linux-gnu-to-s390x-linux-gnu-20181112.tar.gz
> > x86_64-unknown-linux-gnu-to-ppc64le-linux-gnu/
> > x86_64-unknown-linux-gnu-to-ppc64-linux-gnu/
> > x86_64-unknown-linux-gnu-to-s390x-linux-gnu/
> >
> > Below you can find a more detailed description of the various changes.
> > Once we've discussed and agreed on the changes I'd like to add a small
> > documentation about how to build and use devkits to
> > "doc/building.{md,html}" which describes the creation and usage of
> > devkits. This documentation should be right at the top of the
> > "Cross-compiling" section which is quite complex now. It was not clear
> > to me until yet how trivial the creation and usage of a devkit can be
> > :)
> >
> >
> > - The changes required for supporting linux/ppc64/ppc64le/s390x are trivial:
> >
> > make/devkit/Tools.gmk
> >
> > +ifneq ($(filter ppc64 ppc64le s390x, $(ARCH)), )
> > + # We only support 64-bit on these platforms anyway
> > + CONFIG += --disable-multilib
> > +endif
> >
> > This is required to prevent building of multilib toolchains which
> > arent't needed anyway. The problem with the multilib toolchain build
> > is that it requires some special 32-bit headers which arn't installed
> > by default from the current RPM list.
> >
> > - The following change allows users to choose the version of Fedora
> > which is used to create the sysroot environment by setting
> > "BASE_OS_VERSION" (with "27" being the default). This works "BASE_OS"
> > will be set to "Fedora" (as opposed to "Fedora27" before). Notice that
> > older Fedora versions have a sligthly different download URL:
> >
> > make/devkit/Tools.gmk
> >
> > ifeq ($(BASE_OS), OEL6)
> > OEL_URL := http://yum.oracle.com/repo/OracleLinux/OL6/4/base/$(ARCH)/
> > LINUX_VERSION := OEL6.4
> > -else ifeq ($(BASE_OS), Fedora27)
> > - ifeq ($(ARCH), aarch64)
> > +else ifeq ($(BASE_OS), Fedora)
> > + DEFAULT_OS_VERSION := 27
> > + ifeq ($(BASE_OS_VERSION), )
> > + BASE_OS_VERSION := $(DEFAULT_OS_VERSION)
> > + endif
> > + ifeq ($(filter x86_64 armhfp, $(ARCH)), )
> > FEDORA_TYPE=fedora-secondary
> > else
> > FEDORA_TYPE=fedora/linux
> > endif
> > - OEL_URL :=
> > https://dl.fedoraproject.org/pub/$(FEDORA_TYPE)/releases/27/Everything/$(ARCH)/os/Packages/
> > - LINUX_VERSION := Fedora 27
> > + ARCHIVED := $(shell [ $(BASE_OS_VERSION) -lt $(DEFAULT_OS_VERSION)
> > ] && echo true)
> > + ifeq ($(ARCHIVED),true)
> > + OEL_URL :=
> > https://archives.fedoraproject.org/pub/archive/$(FEDORA_TYPE)/releases/$(BASE_OS_VERSION)/Everything/$(ARCH)/os/Packages/
> > + else
> > + OEL_URL :=
> > https://dl.fedoraproject.org/pub/$(FEDORA_TYPE)/releases/$(BASE_OS_VERSION)/Everything/$(ARCH)/os/Packages/
> > + endif
> > + LINUX_VERSION := Fedora $(BASE_OS_VERSION)
> > else
> > $(error Unknown base OS $(BASE_OS))
> > endif
> >
> > - Enable the creation of several different devkits at once (e.g. 'make
> > cross_compile_target="ppc64-linux-gnu ppc64le-linux-gnu
> > s390x-linux-gnu"') or one after another but all into the same
> > 'build/devkit/result' directory. The result directory will contain
> > $HOST-to-$TARGET sub-directories with the corresponding devkits:
> >
> > make/devkit/Makefile
> >
> > -submakevars = HOST=$@ BUILD=$(me) \
> > - RESULT=$(RESULT) PREFIX=$(RESULT)/$@ \
> > - OUTPUT_ROOT=$(OUTPUT_ROOT)
> > +submakevars = HOST=$@ BUILD=$(me) RESULT=$(RESULT)
> > OUTPUT_ROOT=$(OUTPUT_ROOT)
> > +
> > $(host_platforms) :
> > @echo 'Building compilers for $@'
> > @echo 'Targets: $(target_platforms)'
> > for p in $(filter $@, $(target_platforms)) $(filter-out $@,
> > $(target_platforms)); do \
> > - $(MAKE) -f Tools.gmk download-rpms $(submakevars) TARGET=$$p && \
> > + $(MAKE) -f Tools.gmk download-rpms $(submakevars) \
> > + TARGET=$$p PREFIX=$(RESULT)/$@-to-$$p && \
> > $(MAKE) -f Tools.gmk all $(submakevars) \
> > - TARGET=$$p || exit 1 ; \
> > + TARGET=$$p PREFIX=$(RESULT)/$@-to-$$p && \
> > + $(MAKE) -f Tools.gmk ccache $(submakevars) \
> > + TARGET=$@ PREFIX=$(RESULT)/$@-to-$$p
> > BUILDDIR=$(OUTPUT_ROOT)/$@/$$p || exit 1 ; \
> > done
> > - @echo 'Building ccache program for $@'
> > - $(MAKE) -f Tools.gmk ccache $(submakevars) TARGET=$@
> > @echo 'All done"'
> >
> > Notice that we have to build "ccache" for each target because ccache
> > will be installed into the directory specified by "--prefix" at
> > configure time and this is now different for every target. However
> > that's not a big problem, because the time for compiling ccache is
> > negligible compared to the download time of the RPMs and the build
> > time of GCC.
> >
> > define Mktar
> > - $(1)_tar = $$(RESULT)/sdk-$(1)-$$(today).tar.gz
> > - $$($(1)_tar) : PLATFORM = $(1)
> > - TARFILES += $$($(1)_tar)
> > - $$($(1)_tar) : $(1) $$(shell find $$(RESULT)/$(1))
> > + $(1)-to-$(2)_tar = $$(RESULT)/sdk-$(1)-to-$(2)-$$(today).tar.gz
> > + $$($(1)-to-$(2)_tar) : PLATFORM = $(1)-to-$(2)
> > + TARFILES += $$($(1)-to-$(2)_tar)
> > + $$($(1)-to-$(2)_tar) : $$(shell find $$(RESULT)/$(1)-to-$(2) -type f)
> > endef
> >
> > -$(foreach p,$(host_platforms),$(eval $(call Mktar,$(p))))
> > +$(foreach p,$(host_platforms),$(foreach t,$(target_platforms),$(eval
> > $(call Mktar,$(p),$(t)))))
> >
> > make/devkit/Tools.gmk
> >
> > -PATHEXT = $(RESULT)/$(BUILD)/bin:
> > +PATHEXT = $(PREFIX)/bin:
> >
> >
> > - Various small cleanups
> >
> > make/devkit/Tools.gmk
> >
> > - Don't set "RESULT" and "PREFIX" in Tools.gmk because the values are
> > overridden by the settings in the calling Makefile anyway:
> >
> > # Define directories
> > -RESULT := $(OUTPUT_ROOT)/result
> > BUILDDIR := $(OUTPUT_ROOT)/$(HOST)/$(TARGET)
> > -PREFIX := $(RESULT)/$(HOST)
> > TARGETDIR := $(PREFIX)/$(TARGET)
> >
> > - Cleanup arch selection:
> >
> > ifeq ($(ARCH),x86_64)
> > - RPM_ARCHS := x86_64 noarch
> > + RPM_ARCHS := $(ARCH) noarch
> > ifeq ($(BUILD),$(HOST))
> > ifeq ($(TARGET),$(HOST))
> > # When building the native compiler for x86_64, enable mixed mode.
> > @@ -199,7 +206,7 @@
> > endif
> > endif
> > else ifeq ($(ARCH),i686)
> > - RPM_ARCHS := i386 i686 noarch
> > + RPM_ARCHS := $(ARCH) i386 noarch
> > else ifeq ($(ARCH), armhfp)
> >
> > - Don't create 'devkit.info' unconditinally. Only build it as part of
> > the "all" target invocation. This prevents the creation of a
> > 'devkit.info' with the wrong "BASE_OS_VERSION" if the Makefile is
> > invoked several times with different "BASE_OS_VERSION" values:
> >
> > -$(PREFIX)/devkit.info: FRC
> > +$(PREFIX)/devkit.info:
> > @echo 'Creating devkit.info in the root of the kit'
> > rm -f $@
> > touch $@
> > @@ -611,7 +623,4 @@
> > # this is only built for host. so separate.
> > ccache : $(ccache)
> >
> > -# Force target
> > -FRC:
> >
> > - put the base directory of the devkits into the tar archives. I don't
> > like tar archives which don't have a single top-level directory and
> > expand right into the current working directory :)
> >
> > make/devkit/Makefile
> >
> > %.tar.gz :
> > @echo 'Creating compiler package $@'
> > - cd $(RESULT)/$(PLATFORM) && tar -czf $@ *
> > + cd $(RESULT) && tar -czf $@ $(PLATFORM)/*
> > touch $@
>
