Skip to content

feat: axerrno compatibility #25

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: axerrno compatibility #25

wants to merge 1 commit into from
+96 −1

Conversation

@AsakuraMizu
Copy link
Contributor

@AsakuraMizu AsakuraMizu commented Nov 21, 2025 •
edited
Loading

This feature has been discussed in arceos-org/allocator#6.

Copilot finished reviewing on behalf of AsakuraMizu November 21, 2025 03:59
Copilot AI reviewed Nov 21, 2025
View reviewed changes
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds optional axerrno compatibility to the page_table_multiarch crate by implementing a From<PagingError> trait for converting paging errors into axerrno error types. The PR also decouples version management by assigning specific versions to each crate instead of using a workspace-level version.

  • Added optional axerrno feature with error type conversion from PagingError to AxError
  • Bumped page_table_multiarch version to 0.5.7 (separated from workspace version)
  • Set page_table_entry version to 0.5.6 (independent versioning)

Reviewed Changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
page_table_multiarch/src/lib.rs Implements From<PagingError> conversion trait to map paging errors to axerrno error types when the axerrno feature is enabled
page_table_multiarch/Cargo.toml Adds optional axerrno dependency (v0.1), creates axerrno feature flag, and sets independent version 0.5.7
page_table_entry/Cargo.toml Sets independent version 0.5.6, no longer using workspace version
Cargo.toml Removes workspace-level version field as packages now manage their own versions
Cargo.lock Updates lock file with axerrno dependencies (0.1.2 and its transitive dependency 0.2.0) and related crates

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@equation314
Copy link
Member

equation314 commented Nov 21, 2025

It's better to keep the versions of page_table_entry and page_table_multiarch consistent for easier management.

@AsakuraMizu
Copy link
Contributor Author

AsakuraMizu commented Nov 21, 2025

It's better to keep the versions of page_table_entry and page_table_multiarch consistent for easier management.

Hmm, what I'm considering is avoiding releasing an unchanged version to another crate when only one crate is being updated. Another point is that if there are semver-breaking changes to page_table_multiarch in the future (which will happen soon), I don't want to break the compatibility of page_table_entry as well.

@AsakuraMizu
Copy link
Contributor Author

AsakuraMizu commented Nov 21, 2025 •
edited
Loading

The CI is failing because quote crate is using cfg(doc) to simplify documentation: https://github.com/dtolnay/quote/blob/b550c7ee34b90c54a05e2272fcec1d5346a38ca9/src/lib.rs#L627-L635. This usage is actually correct, but we are using --cfg doc to trick the compiler in documentation and testing. I will create a separate PR to remove --cfg doc first.

This was referenced Nov 21, 2025
Open
Open
@Azure-stars
Copy link

Azure-stars commented Nov 21, 2025

It's better to keep the versions of page_table_entry and page_table_multiarch consistent for easier management.

Hmm, what I'm considering is avoiding releasing an unchanged version to another crate when only one crate is being updated. Another point is that if there are semver-breaking changes to page_table_multiarch in the future (which will happen soon), I don't want to break the compatibility of page_table_entry as well.

LGTM. I think there are no strong unified release requirements for our packages in the same workspace. So to seperate them is OK for me.

@AsakuraMizu
Copy link
Contributor Author

AsakuraMizu commented Nov 21, 2025 •
edited
Loading

Let's postpone the changes for version bumping. Keep this PR focusing on adding axerrno compatibility only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@equation314 equation314 Awaiting requested review from equation314

@Azure-stars Azure-stars Awaiting requested review from Azure-stars

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@AsakuraMizu @equation314 @Azure-stars