Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: zig cc support openharmony #21809

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

richerfu
Copy link

This PR is derived from PR #20020, aimed at adding OpenHarmony operating system support for the zig cc command. Since OpenHarmony's C standard library is based on musl implementation, we believe that only adding the available_libcs is necessary.

Additionally, we have implemented several benchmark tests based on some fundamental capabilities to ensure usability. I will provide the testing steps and my testing conclusions below.

How to Test?

  1. You need to register on the official website and download the latest version of DevEco Studio. After installation, you need to install the emulator to provide the testing environment.

  2. Next, you need to set an environment variable to provide platform-specific capabilities for the compiler:

    export OHOS_NDK_HOME=/Applications/DevEco-Studio.app/Contents/sdk/default/openharmony
    export PATH=$PATH:$OHOS_NDK_HOME/native/build-tools/cmake/bin:/Applications/DevEco-Studio.app/Contents/tools/ohpm/bin
    export PATH=$OHOS_NDK_HOME/toolchains:$OHOS_NDK_HOME/native/llvm/bin:$PATH
    
  3. Before starting the tests, we need to install some tools: cargo-zigbuild just Rust and install some toolchains.

    rustup target add aarch64-unknown-linux-ohos
    rustup target add armv7-unknown-linux-ohos
    rustup target add x86_64-unknown-linux-ohos
  4. Clone the ohos-rs repository and proceed with the build

    Note: branch should be ohos

    Build example as first and just run command with below:

    just zig-build
  5. Open the harmony-example project from ohos-rs in the IDE, then right-click to run the unit tests as shown

    image
  6. Run the complete test suite via command line to view the results

    hdc shell aa test -b com.ohos.napi -m entry_test -s unittest /ets/testrunner/OpenHarmonyTestRunner

Result

Based on my current observations, the test results are consistent with those built using the NDK. All unit test cases are sourced from napi-rs. While some test cases haven't passed due to system implementation issues, I believe these can be disregarded. The successfully passed tests are sufficient to confirm the functionality of using zig cc as the compiler

lib/std/zig/target.zig Outdated Show resolved Hide resolved
@alexrp
Copy link
Member

alexrp commented Oct 26, 2024

We have these in test/llvm_targets.zig:

  • riscv32-linux-ohos
  • riscv64-linux-ohos
  • x86-linux-ohos

Should those be added here as well?

@alexrp
Copy link
Member

alexrp commented Oct 26, 2024

Since OpenHarmony's C standard library is based on musl implementation, we believe that only adding the available_libcs is necessary.

As long as OpenHarmony hasn't altered any of musl's headers or changed any of the exported symbols in libc.so, this should at least be true for dynamic linking scenarios. For static linking scenarios, it's true as long as OpenHarmony has made no important changes to musl's implementation code.

@richerfu
Copy link
Author

We have these in test/llvm_targets.zig:

  • riscv32-linux-ohos
  • riscv64-linux-ohos
  • x86-linux-ohos

Should those be added here as well?

I think it's not. There are currently no good test plans for these architectural goals.

@richerfu
Copy link
Author

richerfu commented Oct 26, 2024

Since OpenHarmony's C standard library is based on musl implementation, we believe that only adding the available_libcs is necessary.

As long as OpenHarmony hasn't altered any of musl's headers or changed any of the exported symbols in libc.so, this should at least be true for dynamic linking scenarios. For static linking scenarios, it's true as long as OpenHarmony has made no important changes to musl's implementation code.

OpenHarmony does have some changes, which can be seen on the official website, but I think the current support is enough before being merged in #20020 . what you think?

For HarmonyNext docs , i18n support is not good

Or is there any good way to test this ability? If so and i can try to test it with device.

@alexrp
Copy link
Member

alexrp commented Oct 26, 2024

I think it's not. There are currently no good test plans for these architectural goals.

The main thing we care about is whether the support exists in some form at all, no matter how limited/early/experimental. There doesn't necessarily have to be a test plan. Is this the case? (And are there others we aren't covering?)

OpenHarmony does have some changes, which can be seen on the official website, but I think the current support is enough before being merged in #20020 . what you think?

I hadn't seen that #20020 was closed. AFAIK, Andrew goes over all draft PRs and closes ones with no activity every once in a while, so it looks like #20020 got caught in that without him noticing that his input was needed there. 🙂

Ok, some questions:

  • Does OpenHarmony support statically linking musl or only dynamically?
  • For every supported target triple, can I ask you to do a diff between the OpenHarmony musl headers vs the upstream musl headers (i.e. the ones Zig ships)?

The answers to these questions will determine how we should proceed with this.

@richerfu
Copy link
Author

richerfu commented Oct 26, 2024

ok. I'll try to answer these questions.

  1. For openharmony ndk, it just provides arm64/arm/x86_64 's libc.so/libc.a. You can find them in $OHOS_NDK_HOME/native/sysroot/usr/lib

    image
  2. For openharmony, I think its processing logic should be similar to Android. Developers should only use NDK or the libc provided by the system. Non-ndk provided musl should not be linked against during application build

  3. Starting from Openharmony 5.0, musl has been upgraded to version 1.2.5. According to the documentation, some symbols are not exported. If you want to give the complete differences, it may take a lot of time.

@alexrp
Copy link
Member

alexrp commented Oct 26, 2024

For openharmony ndk, it just provides arm64/arm/x86_64 's libc.so/libc.a. You can find them in $OHOS_NDK_HOME/native/sysroot/usr/lib

Do you know if the OpenHarmony project has experimental support for more architectures in their official development repos?

Starting from Openharmony 5.0, musl has been upgraded to version 1.2.5. According to the documentation, some symbols are not exported. If you want to give the complete differences, it may take a lot of time.

I'm basically just asking for the moral equivalent of diff -ru path/to/zig/lib/libc/include/arm-linux-musl path/to/ohos/sysroot/usr/include and likewise for aarch64-linux-ohos and x86_64-linux-ohos (may need to filter out non-musl headers from the NDK). You could probably even just do the diff against the directories you added in PR #20020. The point is just to get a sense of how big the differences, if any, are.

@richerfu
Copy link
Author

I can provide some embedded repositories, but it seems that there is no support for riscv and other architectures.

https://gitee.com/openharmony/kernel_liteos_a

And i'll provide this difference later. Can the difference content be directly followed in the comments of this PR?

@alexrp
Copy link
Member

alexrp commented Oct 26, 2024

I can provide some embedded repositories, but it seems that there is no support for riscv and other architectures.

Ok, that's good to know. In that case, we should remove those target triples from test/llvm_targets.zig. Thanks for confirming.

And i'll provide this difference later. Can the difference content be directly followed in the comments of this PR?

Putting them in a gist and posting the link here would probably be ideal.

@richerfu
Copy link
Author

richerfu commented Oct 27, 2024

https://gist.github.com/richerfu/f411e0fc01892cc8a517f26b430b3616

Judging from the results, most of the differences are in file formats, and a few files have added or removed some definitions.

Note: You can use Beyond Compare to load these patch files.

@alexrp
Copy link
Member

alexrp commented Oct 27, 2024

It seems like a lot of these diffs are caused by line endings. Running dos2unix on all of them before diffing should produce more readable output.

@richerfu
Copy link
Author

It seems like a lot of these diffs are caused by line endings. Running dos2unix on all of them before diffing should produce more readable output.

Updated. I remove all the new line at the end of file and convert crlf to lf with dos2unix.

@alexrp
Copy link
Member

alexrp commented Oct 27, 2024

Ok, that's a lot better, thank you.

I think there are enough significant differences in these header files that we can't just simply use upstream musl headers; we have to ship distinct OpenHarmony musl headers as in #20020. Fortunately, if dynamically linking libc is the norm for OpenHarmony, we can probably get away with not shipping an entire copy of the OpenHarmony musl sources as well.

(Aside: Is the implementation code for OpenHarmony's musl fork actually public?)

So, I think #20020 was on the right track. But as I said then, I think @andrewrk should sign off on actually adding OpenHarmony libc support to Zig.

If Andrew is on board with it, then I think the changes in #20020 should be revived in this PR along with the changes you've already proposed here. We'll also need:

  • A change in lib/std/zig/target.zig to pick the right musl include directory for ohos/ohoseabi. Note that this will be much easier to do once Some initial work on x32 and n32 ABI support #21717 is merged, so I suggest waiting for that.
  • Changes in src/Compilation/Config.zig to bail out if static linking is requested for ohos/ohoseabi, and to default to dynamic linking (unlike other musl targets).

@richerfu
Copy link
Author

richerfu commented Oct 27, 2024

For ohos-musl you can see it with https://gitee.com/openharmony/third_party_musl. If you follow the above content, please let me know it and I will add the ohos header file to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants