openharmony: add servoshell for ohos by mukilan · Pull Request #33295 · servo/servo

mukilan · 2024-09-03T04:52:41Z

./mach build -d does not report any errors
./mach test-tidy does not report any errors

support/openharmony/entry/oh-package.json5

support/openharmony/oh-package.json5

jschwe · 2024-09-03T09:06:47Z

python/servo/command_base.py

                         'will choose a default target architecture.',
                ),
+                CommandArgument(
+                    '--ohos', default=None, action='store_true',


What benefit does adding a new argument give us? In principle the target triple already tells us that we are targetting ohos.

It is just an alias to build the default architecture for the platform. The only value is the UX for the user, which is debatable, but we have it for android, so I've added for OH as well.

Considering that the long term plan is to remove mach, I'm not convinced having users get used to convenience options that "cargo build" does not have is desirable.

Fair, but I don't see mach being replaced very soon, so I don't see the harm in having a consistent UX while we still depend upon it.

mach will not be removed, but our plan is to make mach build/check/cargo-* commands more thin (ideally just cargo commands aliases with some options aliases).

jschwe · 2024-09-03T09:08:58Z

.github/workflows/ohos.yml

      - name: Build (arch ${{ matrix.arch }} profile ${{ inputs.profile }})
        env:
          OHOS_SDK_NATIVE: ${{ steps.setup_sdk.outputs.ohos_sdk_native }}
+          OHOS_BASE_SDK_HOME: ${{ steps.setup_sdk.outputs.ohos_sdk_native }}


Is there something that expects this variable name instead of OHOS_SDK_NATIVE?

hvigor needs this to find the other components like toolchain and ets that it needs for building the shell. The value should actually come from the ohos_base_sdk_home output of the setup_sdk step. I'll fix this typo.

Interesting. Is this perhaps only needed for hvigor 4.x? I don't think I'm setting this variable when building on the commandline locally.

Just tried locally with hvigor 5.x:

Info: The OpenHarmony SDK 4.1.7.5 is targeting API-level 11 > hvigor ERROR: Unable to find 'sdk.dir' in 'local.properties' or 'OHOS_BASE_SDK_HOME' in the system environment path. at /home/mukilan/servo-ohos/target/openharmony/aarch64-unknown-linux-ohos/release/local.properties > hvigor ERROR: BUILD FAILED in 1 s 797 ms

Do perhaps have sdk.dir in local.properties or DEVECO_SDK_HOME set?

jschwe · 2024-09-03T15:44:32Z

Regarding signing - I updated the servodemo with a small patch (jschwe/ServoDemo#5) which lets hvigor read the signing configuration from a seperate file. I also added an example file to the repo.
In principle hvigor can be extended by arbitrary typescript code, so you could do something more complex. But I think for our usecase reading the signing configuration from a file which is not in git should be sufficient.

.github/workflows/ohos.yml

jschwe

Left some comments, but I think this PR is basically fine as is.

Just as a note, I also have a branch on my ServoDemo repo jschwe/ServoDemo#6, which allows building servo directly via the IDE (and without requiring mach, pure cargo build).
I think both approaches can co-exist, just wanted to mention that the OH build can in principle work fully without mach already (and with only very little CMake code).

support/openharmony/build-profile.json5

support/openharmony/entry/src/main/ets/pages/Index.ets

jschwe · 2024-09-12T05:34:40Z

python/servo/command_base.py

                         'will choose a default target architecture.',
                ),
+                CommandArgument(
+                    '--ohos', default=None, action='store_true',


Considering that the long term plan is to remove mach, I'm not convinced having users get used to convenience options that "cargo build" does not have is desirable.

jschwe · 2024-09-12T05:37:24Z

python/servo/package_commands.py

+            # hvigor doesn't support an option to place output files in a specific directory
+            # so copy the source files into the target/openharmony directory first.
+            ohos_app_dir = path.join(self.get_top_dir(), "support", "openharmony")
+            ohos_target_dir = path.join(


The source files of OH app, shouldn't be different for release / debug mode, so I don't think adding build_type.directory_name() is strictly necessary.

The idea is to allow the HAP (which will be generated inside the source folder hierarchy) for debug/release/production profiles to co-exist like we do for other targets.

Hmm, does the output folder in hvigor not differ between release / debug profiles? (To be fair I've only ever built with the default profile, which I think is debug)

I'm not sure either. The output currently ends up inside <top-level for app source>/entry/build/default/outputs/default . Is one of the default in this path referring to the debug/release type? There is a buildMode declaration, but I don't follow how that translates to "default". Also, shouldn't the invocation of the hvigor differ for debug/release? It is currently the same for both.

Just tried invoking hvigor with -p buildMode=release and while the build successfully completes successfully, it does give this warning

> hvigor Finished :servoshell:default@CompileResource... after 53 ms > hvigor WARN: Dependency libservoshell.so not found.

but the HAP does contain the libservoshell.so. The path is still the same as without buildMode=release i.e entry/build/default/outputs/default so the path doesn't seem to be affected by the debug/release. Perhaps we'll need to map the debug/release to different targets?

@jschwe the "dependency not found" error seems to originate from 'obfustcation-config-resolver.js' in the ohos-plugin module. I don't know why it says the .so is not found, but still the HAP has the library and it runs as expected.

I've added code in a797e9a to pass bulidMode to hvigor. I've not removed the profile subfolder under target/openharmony/<triple> since the output path doesn't seem to change.

python/servo/package_commands.py

support/openharmony/entry/build-profile.json5

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

mukilan marked this pull request as draft September 3, 2024 04:54

mukilan force-pushed the ohos-ci branch 2 times, most recently from 1097ac8 to a029151 Compare September 3, 2024 06:43

jschwe reviewed Sep 3, 2024

View reviewed changes

mukilan force-pushed the ohos-ci branch from a029151 to e0b51fb Compare September 12, 2024 02:58

mukilan marked this pull request as ready for review September 12, 2024 03:02

mukilan commented Sep 12, 2024

View reviewed changes

.github/workflows/ohos.yml Outdated Show resolved Hide resolved

mukilan force-pushed the ohos-ci branch from e0b51fb to 1484533 Compare September 12, 2024 04:10

mukilan requested review from jschwe and mrobinson and removed request for jschwe September 12, 2024 04:11

mukilan force-pushed the ohos-ci branch from 1484533 to 1329a6c Compare September 12, 2024 04:13

jschwe reviewed Sep 12, 2024

View reviewed changes

mukilan force-pushed the ohos-ci branch 3 times, most recently from caf7dbc to a797e9a Compare September 17, 2024 08:24

mukilan force-pushed the ohos-ci branch 2 times, most recently from 1996717 to 3ce19ee Compare September 19, 2024 11:41

mrobinson approved these changes Sep 19, 2024

View reviewed changes

support/openharmony/entry/build-profile.json5 Outdated Show resolved Hide resolved

openharmony: add servoshell for ohos

ac570b1

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

mukilan force-pushed the ohos-ci branch from 3ce19ee to ac570b1 Compare September 20, 2024 05:21

mukilan enabled auto-merge September 20, 2024 05:23

mukilan added this pull request to the merge queue Sep 20, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 20, 2024

ohos: handle missing signing config on forks

7ce9c38

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>

mukilan enabled auto-merge September 20, 2024 07:54

mukilan added this pull request to the merge queue Sep 20, 2024

Merged via the queue into servo:main with commit 157e28c Sep 20, 2024

mukilan deleted the ohos-ci branch September 20, 2024 09:32

Uh oh!

Conversation

mukilan commented Sep 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mukilan Sep 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jschwe commented Sep 3, 2024

Uh oh!

Uh oh!

jschwe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mukilan Sep 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mukilan Sep 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mukilan commented Sep 3, 2024 •

edited

Loading

mukilan Sep 3, 2024 •

edited

Loading

mukilan Sep 12, 2024 •

edited

Loading

mukilan Sep 17, 2024 •

edited

Loading