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

extending filesystem support for Hermit #115984

Merged
merged 2 commits into from
Apr 8, 2024
Merged

extending filesystem support for Hermit #115984

merged 2 commits into from
Apr 8, 2024

Conversation

stlankes
Copy link
Contributor

@stlankes stlankes commented Sep 19, 2023

Extending std to create, change and read a directory for Hermit.

Hermit is a tier 3 platform and this PR changes only files, wich are related to the tier 3 platform.

@rustbot
Copy link
Collaborator

rustbot commented Sep 19, 2023

r? @thomcc

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added O-hermit Operating System: Hermit S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 19, 2023
@rustbot
Copy link
Collaborator

rustbot commented Sep 19, 2023

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

@stlankes
Copy link
Contributor Author

Cc: @mkroening

Copy link
Member

@thomcc thomcc left a comment

Choose a reason for hiding this comment

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

I'll give it a more thorough pass later this weekend. It mostly looks fine.


macro_rules! offset_ptr {
($entry_ptr:expr, $field:ident) => {{
const OFFSET: isize = {
Copy link
Member

Choose a reason for hiding this comment

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

} else {
#[allow(deref_nullptr)]
{
ptr::addr_of!((*ptr::null::<dirent>()).$field)
Copy link
Member

Choose a reason for hiding this comment

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

This is very gnarly, even if the branch is never taken....

}
};

macro_rules! offset_ptr {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure why this macro exists. Most/all of the use sites could be fine with another construct (either addr_of, or just (*p).field), no?

The only place I see something like it being needed is for the name field of the dirent (since it might not be entirely there) but at that point you can just use offset_of and casting, no?

@stlankes
Copy link
Contributor Author

Hm, you are right. I will revise it.

@mkroening
Copy link
Contributor

mkroening commented Sep 24, 2023

The offset_of macro seems to have its origin and explanation here: sys/unix/fs.rs#L705-L736.

Since we control both sides of this API, we might be able to avoid having to use it.

@thomcc
Copy link
Member

thomcc commented Sep 24, 2023

I see, so that code is mostly copied from sys::unix? Hm... That's probably fine, then. It would be nice if it could be somehow reused, admittedly.

@bors
Copy link
Contributor

bors commented Sep 25, 2023

☔ The latest upstream changes (presumably #116029) made this pull request unmergeable. Please resolve the merge conflicts.

@stlankes
Copy link
Contributor Author

stlankes commented Sep 26, 2023

Hm, I agree, that this will be useful. However, the handling of error numbers are different and also a few system calls (e.g. readdir) have a different interface. Maybe I overlook something. But I think that this isn't so easy.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Oct 22, 2023

☔ The latest upstream changes (presumably #116956) made this pull request unmergeable. Please resolve the merge conflicts.

@mkroening
Copy link
Contributor

Rebased on latest master.

@thomcc, are you okay with the macro for now?

@bors
Copy link
Contributor

bors commented Nov 18, 2023

☔ The latest upstream changes (presumably #118046) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Jan 13, 2024

☔ The latest upstream changes (presumably #117285) made this pull request unmergeable. Please resolve the merge conflicts.

@stlankes
Copy link
Contributor Author

@thomcc We revised the system call to read directory entries. I hope that interface is now more understandable.

@rust-log-analyzer

This comment has been minimized.

@thomcc
Copy link
Member

thomcc commented Feb 1, 2024

I'm going to be away for a few months, so I'm rerolling my PRs so that folks don't have to wait for me. Sorry/thanks.

r? libs

@bors
Copy link
Contributor

bors commented Feb 29, 2024

☔ The latest upstream changes (presumably #121800) made this pull request unmergeable. Please resolve the merge conflicts.

@rustbot rustbot added A-meta Area: Issues about the rust-lang/rust repository. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) WG-trait-system-refactor The Rustc Trait System Refactor Initiative labels Mar 1, 2024
@rustbot
Copy link
Collaborator

rustbot commented Mar 1, 2024

Some changes occurred in engine.rs, potentially modifying the public API of ObligationCtxt.

cc @lcnr, @compiler-errors

triagebot.toml has been modified, there may have been changes to the review queue.

cc @davidtwco, @wesleywiser

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

simonschoening and others added 2 commits March 1, 2024 09:20
The new interface has some similarities to Linux system call
getdents64. The system call reads several dirent64 structures.
At the end of each dirent64 is stored the name of the file.
The length of file name is implictly part of dirent64 because
d_reclen contains size of dirent64 plus the length of the file
name.
@@ -48,6 +48,11 @@ impl FileDesc {
pub fn set_nonblocking(&self, _nonblocking: bool) -> io::Result<()> {
unsupported()
}

pub fn fstat(&self, stat: *mut abi::stat) -> io::Result<()> {
cvt(unsafe { abi::fstat(self.fd.as_raw_fd(), stat) })?;
Copy link
Contributor

Choose a reason for hiding this comment

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

...Why did I have to trace through three different directories just to discover that this is referring to the hermit_abi crate? That is so confusing.

Copy link
Contributor Author

@stlankes stlankes Mar 17, 2024

Choose a reason for hiding this comment

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

Excuse me for my late response.

I am a little bit confused. Why do you have trace three different directories? Do you mean I should move fstat from from fd.rs to fs.rs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or do you want that rename abi to hermit-abi? Maybe it is easier to understand...

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I was a bit confused by the levels of indirection in general, but yes, I think probably using hermit_abi might be a bit clearer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I would prefer to create an additional PR for these changes because I have to touch every file in library/std/src/sys/pal/hermit/. In addition, it doesn't belong to this PR.

@m-ou-se
Copy link
Member

m-ou-se commented Apr 8, 2024

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Apr 8, 2024

📌 Commit b1c7804 has been approved by m-ou-se

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 8, 2024
@m-ou-se m-ou-se removed T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) WG-trait-system-refactor The Rustc Trait System Refactor Initiative A-meta Area: Issues about the rust-lang/rust repository. labels Apr 8, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 8, 2024
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#115984 (extending filesystem support for Hermit)
 - rust-lang#120144 (privacy: Stabilize lint `unnameable_types`)
 - rust-lang#122807 (Add consistency with phrases "meantime" and "mean time")
 - rust-lang#123089 (Add invariant to VecDeque::pop_* that len < cap if pop successful)
 - rust-lang#123595 (Documentation fix)
 - rust-lang#123625 (Stop exporting `TypeckRootCtxt` and `FnCtxt`.)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit beaca9c into rust-lang:master Apr 8, 2024
11 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Apr 8, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 8, 2024
Rollup merge of rust-lang#115984 - hermit-os:fuse, r=m-ou-se

extending filesystem support for Hermit

Extending `std` to create, change and read a directory for Hermit.

Hermit is a tier 3 platform and this PR changes only files, wich are related to the tier 3 platform.
@stlankes stlankes deleted the fuse branch April 21, 2024 18:43
stlankes added a commit to hermit-os/rust that referenced this pull request Apr 23, 2024
Take up suggestion from the discussions within rust-lang#115984
to increase readability.
stlankes added a commit to hermit-os/rust that referenced this pull request Apr 23, 2024
Take up suggestion from the discussions within rust-lang#115984
to increase readability.
compiler-errors added a commit to compiler-errors/rust that referenced this pull request May 19, 2024
revise the interpretation of ReadDir for HermitOS

HermitOS supports getdents64. As under Linux, the dirent64 entry `d_off` is not longer used, because its definition is not clear. Instead of `d_off` the entry `d_reclen` is used to determine the end of the dirent64 entry.

In addition, take up `@workingjubilee`  suggestion from the discussions in rust-lang#115984 to increase the readability.

Hermit is a tier 3 platform and this PR changes only files, wich are related to the tier 3 platform.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 19, 2024
Rollup merge of rust-lang#124304 - hermit-os:fuse, r=joboet

revise the interpretation of ReadDir for HermitOS

HermitOS supports getdents64. As under Linux, the dirent64 entry `d_off` is not longer used, because its definition is not clear. Instead of `d_off` the entry `d_reclen` is used to determine the end of the dirent64 entry.

In addition, take up `@workingjubilee`  suggestion from the discussions in rust-lang#115984 to increase the readability.

Hermit is a tier 3 platform and this PR changes only files, wich are related to the tier 3 platform.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-hermit Operating System: Hermit S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants