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

Merge unblob-native #1096

Merged
merged 235 commits into from
Feb 4, 2025
Merged

Merge unblob-native #1096

merged 235 commits into from
Feb 4, 2025

Conversation

vlaci
Copy link
Contributor

@vlaci vlaci commented Feb 1, 2025

Resolves #1034

vlaci and others added 30 commits May 3, 2023 18:04
- Code is from https://github.com/onekey-sec/unblob/tree/main/rust
- Initialized packaging
- Pyright-specific changes to ginore issues from native code
- Pre-commit hooks
- One check -> test -> build -> release pipeline
- Auto-update nix environment job (experimental)
Port rust binding from Unblob
By default maturin uses a cross builder docker image which I found to have
issues. It is better to be slow, but stable
As of now, it doesn't impact performance, as we are not doing
multithreading, but it doesn't make sense to keep the GIL in native
non-Python code.

Benchmark had to be adjusted to call the wrapped pure function.
Fenix uses a technique, called import from derivation, to acquire
details for a rust toolchain version: It looks-up the version number,
than downloads a manifest file, and build nix expressions from that
which will be imported at the end.

Nix flake check trips on IFD, with error messages like

> a 'aarch64-darwin' with features {} is required to build
'/nix/store/...', but I am a 'x86_64-linux' with features {}

Rust-overlay on the other hand ships with toolchain manifests, not
requiring IFD (but it is larger in size).

This problem is hidden at this moment, as the manifest from Rust 1.60
leaks into the nix store somehow on the builders.
It has been released in last November and is in use for `pyperscan`
already, where that version is a hard requirement.
Version is a required property, setting it to dynamic means, that it
will be generated during build-time.

This required for tools like `pdm` to be able to install this package
in an editable manner directly from a remote.
chore: set version in pyproject.toml as dynamic
Flake lock file updates:

• Updated input 'advisory-db':
    'github:rustsec/advisory-db/d72795ee51634b8c5bcf6b3bc98d08ec5888d191' (2023-04-25)
  → 'github:rustsec/advisory-db/50bed3ba4066e6255dab434dc845e7f655812ce1' (2023-05-05)
• Updated input 'crane':
    'github:ipetkov/crane/7d74bfafea4cfbfa30128c053fc0a1ec7455da74' (2023-05-03)
  → 'github:ipetkov/crane/8708b19627b2dfc2d1ac332b74383b8abdd429f0' (2023-05-03)
• Updated input 'crane/flake-utils':
    'github:numtide/flake-utils/93a2b84fc4b70d9e089d029deacc3583435c2ed6' (2023-03-15)
  → 'github:numtide/flake-utils/cfacdce06f30d2b68473a46042957675eebb3401' (2023-04-11)
• Added input 'crane/flake-utils/systems':
    'github:nix-systems/default/da67096a3b9bf56a91d16901293e51ba5b49a27e' (2023-04-09)
• Updated input 'crane/rust-overlay':
    'github:oxalica/rust-overlay/7ec2ff598a172c6e8584457167575b3a1a5d80d8' (2023-04-03)
  → 'github:oxalica/rust-overlay/d59c3fa0cba8336e115b376c2d9e91053aa59e56' (2023-05-03)
• Updated input 'nixpkgs':
    'github:NixOS/nixpkgs/44f30edf5661d86fb3a95841c35127f3d0ea8b0f' (2023-05-02)
  → 'github:NixOS/nixpkgs/eb751d65225ec53de9cf3d88acbf08d275882389' (2023-05-07)
chore: bumbed version to 0.1.1
Flake lock file updates:

• Updated input 'crane':
    'github:ipetkov/crane/8708b19627b2dfc2d1ac332b74383b8abdd429f0' (2023-05-03)
  → 'github:ipetkov/crane/7b5bd9e5acb2bb0cfba2d65f34d8568a894cdb6c' (2023-05-08)
• Updated input 'nixpkgs':
    'github:NixOS/nixpkgs/eb751d65225ec53de9cf3d88acbf08d275882389' (2023-05-07)
  → 'github:NixOS/nixpkgs/635a306fc8ede2e34cb3dd0d6d0a5d49362150ed' (2023-05-11)
• Updated input 'rust-overlay':
    'github:oxalica/rust-overlay/75b07756c3feb22cf230e75fb064c1b4c725b9bc' (2023-05-08)
  → 'github:oxalica/rust-overlay/7ec9793168e4c328f08d10ab7ef4a1ada2dbf93e' (2023-05-13)
pdm by default installs all dependency group. Selecting a subset is opt-in
It is needed to prevent import errors as the repo root sometimes gets
into the PYTHONPATH, e.g. when python interactive is used. In that
case the native extension may not be loadable.

See also PyO3/maturin#490
vlaci added 5 commits February 1, 2025 23:54
It shows

    error: unexpected `cfg` condition value: `gil-refs`

during clippy execution

The error is due to a new lint turned on on Rust 1.84 which trips
older PyO3 code. It'll go away when upgrading to 0.23[^1], but it needs
API changes, so for the time being the lint is disabled instead.

[^1]: PyO3/pyo3#4743
@vlaci vlaci force-pushed the merge-native branch 5 times, most recently from 9c3b7a0 to 740da88 Compare February 1, 2025 23:31
@vlaci vlaci marked this pull request as ready for review February 1, 2025 23:34
@qkaiser qkaiser changed the title Merge ublob-native Merge unblob-native Feb 2, 2025
Copy link
Contributor

@qkaiser qkaiser left a comment

Choose a reason for hiding this comment

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

Some nitpicks but otherwise looks good to me. Did a first pass yesterday and a second one this morning.

rust/Cargo.toml Show resolved Hide resolved
tests/rust/test_sandbox.py Show resolved Hide resolved
docs/development.md Outdated Show resolved Hide resolved
.github/workflows/CI.yml Outdated Show resolved Hide resolved
vlaci added 9 commits February 2, 2025 10:36
As unblob now contains an extension module, we need to build multiple
wheels for multiple platforms. Even for Docker images we need a
different wheel per architecture.

In order to keep unnecessary rebuilds to a minimum, wheels are built
only by the `build_linux` and `build_macos` jobs, and both PyPi
publishing and Docker image building reuses them.

This change needs dependency relationships between the affected jobs,
so the `run-tests`, `release` and `build-publish-image` workflows are merged
to a single `CI` workflow. Technically the `run-tests` job could have
been left out, but it makes sense to have a dependency relationship
between that an the release publishing job.

The existing already parts, like pre-commit, pyright, pytest and
docker build steps are lifted from the now deleted workflow files.

The template of the wheel build and publishing parts of this job was generated using:

    maturin generate-ci --platform manylinux musllinux macos --pytest -- github

The generated code is cleaned up, instead of running pytest for
created wheels, an `unblob --version` test command is added (it is a
PITA having 3rd party extractors available in all these platforms)
For wheel builds, we shouldn't create a virtualenv, as it conflicts
with the check phase
tests/rust/test_sandbox.py Dismissed Show dismissed Hide dismissed
@vlaci vlaci merged commit 3a7c6a1 into main Feb 4, 2025
26 of 27 checks passed
@vlaci vlaci deleted the merge-native branch February 4, 2025 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RFC: Merge back unblob-native into this repo
3 participants