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.


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.

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...

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).

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.


- 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.

There was something more I thought of when I reviewed your changes, but it slipped my mind now.

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

Reply via email to