8255240: Mobile builds need to exclude some modules #20

johanvos · 2024-03-23T16:30:46Z

This PR enables to build the static libs for ios/android for the native code in most of the modules.
It also adds GA scripts to build those libs on Android and iOS.
Note that building hotspot is disabled by this PR.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8255240: Mobile builds need to exclude some modules (Task - P4)

Reviewers

Magnus Ihse Bursie (@magicus - Reviewer) ⚠️ Review applies to 42bcaf36

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/mobile.git pull/20/head:pull/20
$ git checkout pull/20

Update a local copy of the PR:
$ git checkout pull/20
$ git pull https://git.openjdk.org/mobile.git pull/20/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 20

View PR using the GUI difftool:
$ git pr show -t 20

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/mobile/pull/20.diff

Webrev

Link to Webrev Comment

Add sample configure script for ios

* Add android build * fix env variable use * fix condition * update conf and build for android * add config name * remove build jdk * update id * fix command * add ls to debug * fix build command * update conf name * update directory name * remove ${{ }} to fix condition * do not remove bundle for android * add debug * add debug * do not build debug * s/x64/aarch64 * add debug logs * fix condition * create only release * build on push * add ios build * fix platform name * download cups from source * fix command * skip running make for product-bundles and test-bundles * update upload to support ios static

Fix conflict related to major changes in NativeCompilation.gmk

…lar to macos and aix)

bridgekeeper · 2024-03-23T16:31:33Z

👋 Welcome back jvos! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2024-03-23T16:32:24Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

johanvos · 2024-03-25T17:52:37Z

changed description to match the JBS issue that should get fixed with this PR.

johanvos · 2024-03-25T17:55:36Z

/reviewers 0

openjdk · 2024-03-25T17:56:20Z

@johanvos
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 1 (with at least 1 Reviewer).

mlbridge · 2024-03-25T17:58:51Z

Webrevs

magicus · 2024-03-27T09:50:32Z

make/modules/jdk.jdwp.agent/Lib.gmk

@@ -70,7 +70,9 @@ $(eval $(call SetupJdkLibrary, BUILD_LIBJDWP, \

 $(BUILD_LIBJDWP): $(call FindLib, java.base, java)

-TARGETS += $(BUILD_LIBJDWP)


While this work, this is not normally how we do things in the build.

If you ever want to have this upstreamed, you need to put the entire SetupJdkLibrary inside the ifeq block.

Only the $(eval $(call SetupJdkLibrary, BUILD_LIBJDWP block or also the

$(BUILD_LIBJDWP): $(call FindLib, java.base, java) TARGETS += $(BUILD_LIBJDWP)

?

Yeah, everything needed for the compilation. Have a look at how other libraries that are only built on specific platforms are defined (e.g. in CoreLibraries.gmk, if you struggle to find examples.)

magicus · 2024-03-27T09:52:55Z

make/modules/jdk.hotspot.agent/Lib.gmk

@@ -77,4 +77,7 @@ $(eval $(call SetupJdkLibrary, BUILD_LIBSAPROC, \
    LIBS_windows := dbgeng.lib, \
 ))

-TARGETS += $(BUILD_LIBSAPROC)
+ifeq ($(call isTargetOs, android ios), false)
+  TARGETS += $(BUILD_LIBSA)


The same goes here. Also note that this is actually broken for non-android or ios builds, BUILD_LIBSA should be BUILD_LIBSAPROC...

thanks for noticing, this is embarrassing!
I'll change that in the same way as I'll fix the above -- once I know if only the call to SetupJdkLibrary should be inside the test.

magicus · 2024-03-27T09:53:07Z

make/modules/java.instrument/Lib.gmk

@@ -63,4 +63,8 @@ else
 endif
 $(BUILD_LIBINSTRUMENT): $(call FindLib, java.base, java)

-TARGETS += $(BUILD_LIBINSTRUMENT)
+ifeq ($(call isTargetOs, android ios), false)


And here as well...

thanks, will fix that too.

magicus · 2024-03-27T10:00:24Z

make/modules/java.desktop/Gensrc.gmk

@@ -32,7 +32,7 @@ ifeq ($(call isTargetOs, windows), false)
  include gensrc/GensrcIcons.gmk
 endif

-ifeq ($(call isTargetOs, linux aix), true)
+ifeq ($(call isTargetOs, linux aix android ios), true)


Should ios really have X11Wrappers?

no. Will fix.

magicus · 2024-03-27T10:01:42Z

src/jdk.management/unix/native/libmanagement_ext/OperatingSystemImpl.c

@@ -314,6 +317,9 @@ Java_com_sun_management_internal_OperatingSystemImpl_getOpenFileDescriptorCount0
    free(fds);

    return nfiles;
+#else
+    return (100);


This seem like a hack?

Also, why the parenthesis?

magicus · 2024-03-27T10:03:54Z

The GHA scripts are interesting. I did not realize it could be possible to cross-compile to ios and android on GHA.

I have tried to review them, but I did not go into too much detail. If they are ever to be upstreamed, I think they will need some cleanup and be made to fit better into the existing structure, but for a separate port I assume they are okay.

magicus · 2024-03-27T10:04:28Z

And why are you disabling the hotspot build? How can anything good come out of a JDK build without hotspot?

openjdk · 2024-03-27T15:10:36Z

@johanvos this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout staticlibs
git fetch https://git.openjdk.org/mobile.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

fix whitespace issue

magicus

I guess this looks good enough now, but the PR/JBS issue title does not seem to correlate at all to what the code does...

magicus · 2024-04-18T10:37:04Z

.github/workflows/main.yml

@@ -88,7 +90,7 @@ jobs:
                echo 'false'
                return
              else
-                input='${{ secrets.JDK_SUBMIT_PLATFORMS }}'
+                input='android-linux-aarch64, ios-macos-aarch64'


Actually, this should be reverted, otherwise you can never run a user-specified set of platforms on GHA.

magicus · 2024-04-18T10:38:07Z

configure-ios.sh

@@ -0,0 +1,10 @@
+# sample configure command for ios


I also don't think this belongs in the repo. A command example like this can be documented on the wiki or the JDK project web page.

magicus · 2024-04-18T10:39:29Z

.github/actions/upload-bundles/action.yml

@@ -41,6 +41,10 @@ runs:
      id: bundles
      run: |
        # Rename bundles to consistent names
+        echo "build/${{ inputs.platform }} contents..."


This (and more below) looks like debug code that could/should be removed once you have made sure the builds are working.

magicus · 2024-04-18T10:43:41Z

.github/workflows/build-android-static.yml

+        id: build
+        uses: ./.github/actions/do-build
+        with:
+          make-target: '${{ inputs.make-target }} ${{ inputs.make-arguments }}'


Suggested change

make-target: '${{ inputs.make-target }} ${{ inputs.make-arguments }}'

make-target: 'static-libs-image ${{ inputs.make-arguments }}'

Then you will not need the change in do-build/action.yml

magicus · 2024-04-18T10:44:08Z

.github/actions/do-build/action.yml

@@ -39,12 +39,21 @@ inputs:
 runs:
  using: composite
  steps:
+    - name: 'Build Android or iOS'
+      id: build-android-ios
+      run: |


This is a hack, put in the wrong place. See below for a better solution.

bridgekeeper · 2024-05-16T14:36:14Z

@johanvos This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

johanvos and others added 6 commits February 14, 2024 16:53

Changes required to build static libs for Android with JDK 23-ea

4b65522

Add required ios changes for building static libs.

76bbe92

Add sample configure script for ios

Merge branch 'master' into minimal-android-23

760b5a5

Fix conflict related to major changes in NativeCompilation.gmk

When building for ios, deal with non-supported @objectfilenames (simi…

918cf86

…lar to macos and aix)

Merge branch 'master' into staticlibs

66381da

johanvos changed the title ~~Static libs~~ 8255240: Mobile builds need to exclude some modules Mar 25, 2024

openjdk bot added the rfr label Mar 25, 2024

magicus reviewed Mar 27, 2024

View reviewed changes

Process reviewer comments

9f14ed6

openjdk bot added merge-conflict and removed rfr labels Mar 27, 2024

re-enable gensrc for X11 on Android

42bcaf3

fix whitespace issue

openjdk bot added the rfr label Mar 27, 2024

magicus approved these changes Apr 10, 2024

View reviewed changes

Merge branch 'master' into staticlibs

53d9920

openjdk bot removed the merge-conflict label Apr 14, 2024

magicus reviewed Apr 18, 2024

View reviewed changes

openjdk bot added the merge-conflict label May 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8255240: Mobile builds need to exclude some modules #20

8255240: Mobile builds need to exclude some modules #20

johanvos commented Mar 23, 2024 •

edited by openjdk bot

bridgekeeper bot commented Mar 23, 2024

openjdk bot commented Mar 23, 2024

johanvos commented Mar 25, 2024

johanvos commented Mar 25, 2024

openjdk bot commented Mar 25, 2024

mlbridge bot commented Mar 25, 2024 •

edited

magicus Mar 27, 2024

johanvos Mar 27, 2024

magicus Mar 27, 2024

magicus Mar 27, 2024

johanvos Mar 27, 2024

magicus Mar 27, 2024

johanvos Mar 27, 2024

magicus Mar 27, 2024

johanvos Mar 27, 2024

magicus Mar 27, 2024

magicus commented Mar 27, 2024

magicus commented Mar 27, 2024

openjdk bot commented Mar 27, 2024

magicus left a comment

magicus Apr 18, 2024

magicus Apr 18, 2024

magicus Apr 18, 2024

magicus Apr 18, 2024

magicus Apr 18, 2024

bridgekeeper bot commented May 16, 2024

		@@ -70,7 +70,9 @@ $(eval $(call SetupJdkLibrary, BUILD_LIBJDWP, \

		$(BUILD_LIBJDWP): $(call FindLib, java.base, java)

		TARGETS += $(BUILD_LIBJDWP)

	make-target: '${{ inputs.make-target }} ${{ inputs.make-arguments }}'
	make-target: 'static-libs-image ${{ inputs.make-arguments }}'

8255240: Mobile builds need to exclude some modules #20

Are you sure you want to change the base?

8255240: Mobile builds need to exclude some modules #20

Conversation

johanvos commented Mar 23, 2024 • edited by openjdk bot

Progress

Issue

Reviewers

Reviewing

Webrev

bridgekeeper bot commented Mar 23, 2024

openjdk bot commented Mar 23, 2024

johanvos commented Mar 25, 2024

johanvos commented Mar 25, 2024

openjdk bot commented Mar 25, 2024

mlbridge bot commented Mar 25, 2024 • edited

Webrevs

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

magicus commented Mar 27, 2024

magicus commented Mar 27, 2024

openjdk bot commented Mar 27, 2024

magicus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bridgekeeper bot commented May 16, 2024

johanvos commented Mar 23, 2024 •

edited by openjdk bot

mlbridge bot commented Mar 25, 2024 •

edited