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(ohos): fix some compilation errors and add some features #2587

Merged
merged 3 commits into from
Feb 6, 2025

Conversation

richerfu
Copy link
Contributor

What does this PR do

This PR fix some compilation errors for OpenHarmony. I'll try to explain the following changes.

  1. The basic c library for OpenHarmony is forked from musl(1.25), so it's reasonable for us to follow the musl condition compilation.
  2. According to the document, some symbols don't export. So aio and mqueue should be hidden for OpenHarmony.
  3. OpenHarmony support more enum for memfd, the following code is the content of memfd.h:
 /****************************************************************************
 ****************************************************************************
 ***
 ***   This header was automatically generated from a Linux kernel header
 ***   of the same name, to make information necessary for userspace to
 ***   call into the kernel available to libc.  It contains only constants,
 ***   structures, and macros generated from the original header, and thus,
 ***   contains no copyrightable information.
 ***
 ***   To edit the content of this header, modify the corresponding
 ***   source file (e.g. under external/kernel-headers/original/) then
 ***   run bionic/libc/kernel/tools/update_all.py
 ***
 ***   Any manual change here will be lost the next time this script will
 ***   be run. You've been warned!
 ***
 ****************************************************************************
 ****************************************************************************/
#ifndef _UAPI_LINUX_MEMFD_H
#define _UAPI_LINUX_MEMFD_H
#include <asm-generic/hugetlb_encode.h>
#define MFD_CLOEXEC 0x0001U
#define MFD_ALLOW_SEALING 0x0002U
#define MFD_HUGETLB 0x0004U
#define MFD_HUGE_SHIFT HUGETLB_FLAG_ENCODE_SHIFT
#define MFD_HUGE_MASK HUGETLB_FLAG_ENCODE_MASK
#define MFD_HUGE_64KB HUGETLB_FLAG_ENCODE_64KB
#define MFD_HUGE_512KB HUGETLB_FLAG_ENCODE_512KB
#define MFD_HUGE_1MB HUGETLB_FLAG_ENCODE_1MB
#define MFD_HUGE_2MB HUGETLB_FLAG_ENCODE_2MB
#define MFD_HUGE_8MB HUGETLB_FLAG_ENCODE_8MB
#define MFD_HUGE_16MB HUGETLB_FLAG_ENCODE_16MB
#define MFD_HUGE_32MB HUGETLB_FLAG_ENCODE_32MB
#define MFD_HUGE_256MB HUGETLB_FLAG_ENCODE_256MB
#define MFD_HUGE_512MB HUGETLB_FLAG_ENCODE_512MB
#define MFD_HUGE_1GB HUGETLB_FLAG_ENCODE_1GB
#define MFD_HUGE_2GB HUGETLB_FLAG_ENCODE_2GB
#define MFD_HUGE_16GB HUGETLB_FLAG_ENCODE_16GB
#endif
  1. NDK include the epoll.h and eventfd.h file, so i think epoll and eventfd can be used for OHOS

This PR don't change any nix's API, so i think i don't need to add a changelog right? Hope you can give me some suggestion thanks.

Checklist:

  • I have read CONTRIBUTING.md
  • I have written necessary tests and rustdoc comments
  • A change log has been added if this PR modifies nix

@richerfu
Copy link
Contributor Author

richerfu commented Feb 5, 2025

And i use it to implement a simple traceroute, everything seems to be in order.

Copy link
Member

@SteveLauC SteveLauC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late review, thanks for the ping

Appreciate the explanation, it helps code review a lot!

src/sys/memfd.rs Outdated Show resolved Hide resolved
src/sys/memfd.rs Outdated Show resolved Hide resolved
src/sys/memfd.rs Outdated Show resolved Hide resolved
src/sys/memfd.rs Outdated Show resolved Hide resolved
src/sys/mman.rs Show resolved Hide resolved
src/sys/ptrace/linux.rs Outdated Show resolved Hide resolved
src/sys/time.rs Outdated Show resolved Hide resolved
src/time.rs Outdated Show resolved Hide resolved
test/sys/test_aio.rs Outdated Show resolved Hide resolved
test/test_mq.rs Outdated Show resolved Hide resolved
@SteveLauC
Copy link
Member

This PR don't change any nix's API, so i think i don't need to add a changelog right? Hope you can give me some suggestion thanks.

APIs are not changed, but if you want, you can leave a changelog like this:

file name: 2587.fixed.md

fixes the build on OpenHarmony

@richerfu richerfu force-pushed the master branch 2 times, most recently from 69196d3 to 3d9404b Compare February 5, 2025 11:37
@richerfu
Copy link
Contributor Author

richerfu commented Feb 5, 2025

This PR don't change any nix's API, so i think i don't need to add a changelog right? Hope you can give me some suggestion thanks.

APIs are not changed, but if you want, you can leave a changelog like this:

file name: 2587.fixed.md
fixes the build on OpenHarmony

Thanks for your review, i've already finished all of comments. Please see it again, thanks

src/lib.rs Outdated Show resolved Hide resolved
src/sys/mman.rs Outdated Show resolved Hide resolved
src/sys/statfs.rs Outdated Show resolved Hide resolved
@SteveLauC
Copy link
Member

Regarding the changes in tests, does cargo test work under ohos?

@SteveLauC
Copy link
Member

I am thinking about adding CI for:

  1. aarch64-unknown-linux-ohos
  2. x86_64-unknown-linux-ohos

Are you willing to do this after merging this PR? It is simple:

  1. Add these 2 targets in the CI file:
    - target: x86_64-unknown-netbsd
  2. Add them to the supported tier2 targets (The list needs to be ordered):

    nix/README.md

    Lines 82 to 92 in 299feba

    <li>aarch64-apple-ios</li>
    <li>aarch64-linux-android</li>
    <li>arm-linux-androideabi</li>
    <li>arm-unknown-linux-musleabi</li>
    <li>armv7-linux-androideabi</li>
    <li>i686-linux-android</li>
    <li>loongarch64-unknown-linux-gnu</li>
    <li>s390x-unknown-linux-gnu</li>
    <li>x86_64-linux-android</li>
    <li>x86_64-unknown-illumos</li>
    <li>x86_64-unknown-netbsd</li>

@richerfu
Copy link
Contributor Author

richerfu commented Feb 5, 2025

Regarding the changes in tests, does cargo test work under ohos?

Unfortunately we can't run cargo test within harmony device, there isn't available emulator to run it.

@richerfu
Copy link
Contributor Author

richerfu commented Feb 5, 2025

I am thinking about adding CI for:

  1. aarch64-unknown-linux-ohos
  2. x86_64-unknown-linux-ohos

Are you willing to do this after merging this PR? It is simple:

  1. Add these 2 targets in the CI file:
    - target: x86_64-unknown-netbsd
  2. Add them to the supported tier2 targets (The list needs to be ordered):

    nix/README.md

    Lines 82 to 92 in 299feba

    <li>aarch64-apple-ios</li>
    <li>aarch64-linux-android</li>
    <li>arm-linux-androideabi</li>
    <li>arm-unknown-linux-musleabi</li>
    <li>armv7-linux-androideabi</li>
    <li>i686-linux-android</li>
    <li>loongarch64-unknown-linux-gnu</li>
    <li>s390x-unknown-linux-gnu</li>
    <li>x86_64-linux-android</li>
    <li>x86_64-unknown-illumos</li>
    <li>x86_64-unknown-netbsd</li>

Sure, i'll add them later

@SteveLauC
Copy link
Member

Regarding the changes in tests, does cargo test work under ohos?

Unfortunately we can't run cargo test within harmony device, there isn't available emulator to run it.

Get it

Copy link
Member

@SteveLauC SteveLauC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@SteveLauC SteveLauC added this pull request to the merge queue Feb 6, 2025
Merged via the queue into nix-rust:master with commit 3d4f311 Feb 6, 2025
0 of 3 checks passed
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