-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
history merge
has mmap issues with certain remote filesystems
#10434
Comments
Um, I think I know where the problem lies now. This switch-case: Line 120 in 80394ea
does not correctly handle my storage type, which leads to subsequent use of mmap. Our storage returns a special f_type, which is not included in the enumerated data. |
great work, can you share the file type magic number? Or the relevant line from |
The type of the magic num is #include <iostream>
#include <sys/statfs.h>
int main() {
const char* path = "/mnt/hwfile/";
struct statfs fs_info;
statfs(path, &fs_info);
unsigned int fs = static_cast<unsigned int>(fs_info.f_type);
std::cout << std::hex << fs << std::endl;
return 0;
} The result of
Hmm, it seems quite challenging to differentiate remote file systems through simple operations, as various providers' file systems don't adhere to strict standards (perhaps innovation?). I think it would be simpler if Fish could offer a toggle to control whether to enable mmap, or allow users to configure whether it belongs to a remote FS. |
It seems we should do diff --git a/src/path.rs b/src/path.rs
index 92ecefd1a..97a1b8ff8 100644
--- a/src/path.rs
+++ b/src/path.rs
@@ -660,7 +660,7 @@ fn path_remoteness(path: &wstr) -> DirRemoteness {
=> DirRemoteness::remote,
_ => {
// Other FSes are assumed local.
- DirRemoteness::local
+ DirRemoteness::unknown
}
}
} We'll still use flock for unknown remoteness but won't mmap the file. We can still return DirRemoteness::local for some most file systems. |
But still, if there are meaningful difference between remote and local filesystems, I don't think applications should be required to list all possible local filesystems. I'm not sure if we want to stop using mmap. Maybe we can detect that mmap is failing and then fall back to reading the file?
I don't know how else it could fail, you don't get garbage data, right? |
history merge
history merge
has mmap issues with certain remote filesystems
Let me try this out to see if it works. I'm not really familiar with Rust. Can you give me some simple instructions on how to build from source using Rust? |
you can use something like
|
@krobelus, I've updated this line and built fish from source, but the issue persists. It seems like the Here are some discussions related to Here are some discussions related to
Perhaps the problem lies in the improper configuration of the Perhaps this problem lies in the improper configuration of the |
Fish version
fish, version 3.7.0
System information
My problem
I seem to have encountered the issue mentioned in this GitHub issue. I'm a user of Slurm, and my daily working directory is mounted on the Slurm's management node in a similar way to NFS. Similarly, my fish history file is also stored in the mounted directory.
Recently, while experimenting with fish, I noticed that fish's history doesn't seem to work properly. Since I'm just a beginner with fish and my understanding of Linux isn't as deep as some others, please allow me to present my issue through a video.
The process in the video roughly goes like this:
hello fish
in terminal a.hello fish
was properly recorded.hello fish
was not there.history merge
in terminal b, buthello fish
still didn't appear in the history.history merge
again in terminal b and found thathello fish
appeared.20240410-033016.mp4
According to my understanding, I should have been able to find
hello fish
in terminal b's history in step 5, but it didn't happen (didn't notice the update of the history file? Not sure of the exact reason). When I manually edited the history file and saved it, then calledhistory merge
, the result was normal.I'm not sure if the cause of this phenomenon is the same as mentioned in this issue. Looking for an answer.
The text was updated successfully, but these errors were encountered: