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 $@ >