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

Editing file breaks hard link #11279

Closed
Nemowang2003 opened this issue Jul 22, 2024 · 5 comments · Fixed by #11340
Closed

Editing file breaks hard link #11279

Nemowang2003 opened this issue Jul 22, 2024 · 5 comments · Fixed by #11340
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug

Comments

@Nemowang2003
Copy link

Nemowang2003 commented Jul 22, 2024

Summary

After saving the file, the hard link would be broken.
If it is working as intended, is there an option to disable this feature?

Reproduction Steps

I tried this:

  1. touch a
  2. ln a b
  3. hx b and insert test, and save by :x
  4. cat a

I expected this to happen:
cat a outputs test.
Instead, this happened:
File a remains empty, and the hard link between a and b was broken.

Helix log

~/.cache/helix/helix.log
2024-07-23T02:10:03.063 globset [DEBUG] glob converted to regex: Glob { glob: "*/Dockerfile.*", re: "(?-u)^.*/Dockerfile\\..*$", opts: GlobOptions { case_insensitive: false, literal_separator: false, backslash_escape: true, empty_alternates: false }, tokens: Tokens([ZeroOrMore, Literal('/'), Literal('D'), Literal('o'), Literal('c'), Literal('k'), Literal('e'), Literal('r'), Literal('f'), Literal('i'), Literal('l'), Literal('e'), Literal('.'), ZeroOrMore]) }
2024-07-23T02:10:03.064 globset [DEBUG] glob converted to regex: Glob { glob: "*/dockerfile.*", re: "(?-u)^.*/dockerfile\\..*$", opts: GlobOptions { case_insensitive: false, literal_separator: false, backslash_escape: true, empty_alternates: false }, tokens: Tokens([ZeroOrMore, Literal('/'), Literal('d'), Literal('o'), Literal('c'), Literal('k'), Literal('e'), Literal('r'), Literal('f'), Literal('i'), Literal('l'), Literal('e'), Literal('.'), ZeroOrMore]) }
2024-07-23T02:10:03.064 globset [DEBUG] glob converted to regex: Glob { glob: "*/Containerfile.*", re: "(?-u)^.*/Containerfile\\..*$", opts: GlobOptions { case_insensitive: false, literal_separator: false, backslash_escape: true, empty_alternates: false }, tokens: Tokens([ZeroOrMore, Literal('/'), Literal('C'), Literal('o'), Literal('n'), Literal('t'), Literal('a'), Literal('i'), Literal('n'), Literal('e'), Literal('r'), Literal('f'), Literal('i'), Literal('l'), Literal('e'), Literal('.'), ZeroOrMore]) }
2024-07-23T02:10:03.064 globset [DEBUG] glob converted to regex: Glob { glob: "*/containerfile.*", re: "(?-u)^.*/containerfile\\..*$", opts: GlobOptions { case_insensitive: false, literal_separator: false, backslash_escape: true, empty_alternates: false }, tokens: Tokens([ZeroOrMore, Literal('/'), Literal('c'), Literal('o'), Literal('n'), Literal('t'), Literal('a'), Literal('i'), Literal('n'), Literal('e'), Literal('r'), Literal('f'), Literal('i'), Literal('l'), Literal('e'), Literal('.'), ZeroOrMore]) }
2024-07-23T02:10:03.064 globset [DEBUG] glob converted to regex: Glob { glob: "*/.*ignore", re: "(?-u)^.*/\\..*ignore$", opts: GlobOptions { case_insensitive: false, literal_separator: false, backslash_escape: true, empty_alternates: false }, tokens: Tokens([ZeroOrMore, Literal('/'), Literal('.'), ZeroOrMore, Literal('i'), Literal('g'), Literal('n'), Literal('o'), Literal('r'), Literal('e')]) }
2024-07-23T02:10:03.064 globset [DEBUG] glob converted to regex: Glob { glob: "*/BUILD.*", re: "(?-u)^.*/BUILD\\..*$", opts: GlobOptions { case_insensitive: false, literal_separator: false, backslash_escape: true, empty_alternates: false }, tokens: Tokens([ZeroOrMore, Literal('/'), Literal('B'), Literal('U'), Literal('I'), Literal('L'), Literal('D'), Literal('.'), ZeroOrMore]) }
2024-07-23T02:10:03.064 globset [DEBUG] glob converted to regex: Glob { glob: "*/.env.*", re: "(?-u)^.*/\\.env\\..*$", opts: GlobOptions { case_insensitive: false, literal_separator: false, backslash_escape: true, empty_alternates: false }, tokens: Tokens([ZeroOrMore, Literal('/'), Literal('.'), Literal('e'), Literal('n'), Literal('v'), Literal('.'), ZeroOrMore]) }
2024-07-23T02:10:03.064 globset [DEBUG] glob converted to regex: Glob { glob: "*/.envrc.*", re: "(?-u)^.*/\\.envrc\\..*$", opts: GlobOptions { case_insensitive: false, literal_separator: false, backslash_escape: true, empty_alternates: false }, tokens: Tokens([ZeroOrMore, Literal('/'), Literal('.'), Literal('e'), Literal('n'), Literal('v'), Literal('r'), Literal('c'), Literal('.'), ZeroOrMore]) }
2024-07-23T02:10:03.064 globset [DEBUG] glob converted to regex: Glob { glob: "*/conf/*/*.{inc,conf}", re: "(?-u)^.*/conf/.*/.*\\.(?:conf|inc)$", opts: GlobOptions { case_insensitive: false, literal_separator: false, backslash_escape: true, empty_alternates: false }, tokens: Tokens([ZeroOrMore, Literal('/'), Literal('c'), Literal('o'), Literal('n'), Literal('f'), Literal('/'), ZeroOrMore, Literal('/'), ZeroOrMore, Literal('.'), Alternates([Tokens([Literal('c'), Literal('o'), Literal('n'), Literal('f')]), Tokens([Literal('i'), Literal('n'), Literal('c')])])]) }
2024-07-23T02:10:03.064 globset [DEBUG] glob converted to regex: Glob { glob: "*/Jenkinsfile.*", re: "(?-u)^.*/Jenkinsfile\\..*$", opts: GlobOptions { case_insensitive: false, literal_separator: false, backslash_escape: true, empty_alternates: false }, tokens: Tokens([ZeroOrMore, Literal('/'), Literal('J'), Literal('e'), Literal('n'), Literal('k'), Literal('i'), Literal('n'), Literal('s'), Literal('f'), Literal('i'), Literal('l'), Literal('e'), Literal('.'), ZeroOrMore]) }
2024-07-23T02:10:03.064 globset [DEBUG] built glob set; 1 literals, 0 basenames, 0 extensions, 0 prefixes, 127 suffixes, 9 required extensions, 10 regexes
2024-07-23T02:10:03.077 helix_view::clipboard [DEBUG] Using pbcopy+pbpaste to interact with the system clipboard
2024-07-23T02:10:03.085 helix_vcs [DEBUG] Error {
    context: "failed to open git repo",
    source: Discover(
        NoGitRepository {
            path: "/tmp",
        },
    ),
}
2024-07-23T02:10:03.085 helix_vcs [DEBUG] failed to open diff base for /tmp/b
2024-07-23T02:10:03.085 helix_vcs [DEBUG] Error {
    context: "failed to open git repo",
    source: Discover(
        NoGitRepository {
            path: "/tmp",
        },
    ),
}
2024-07-23T02:10:03.085 helix_vcs [DEBUG] failed to obtain current head name for /tmp/b
2024-07-23T02:10:03.086 helix_view::editor [DEBUG] editor status: Loaded 1 file.
2024-07-23T02:10:03.090 helix_tui::backend::crossterm [DEBUG] The keyboard enhancement protocol is supported in this terminal (checked in 1.891292ms)
2024-07-23T02:10:03.091 helix_view::document [DEBUG] id 1 modified - last saved: 0, current: 0
2024-07-23T02:10:03.329 helix_term::application [DEBUG] received editor event: IdleTimer
2024-07-23T02:10:03.757 helix_view::document [DEBUG] id 1 modified - last saved: 0, current: 0
2024-07-23T02:10:03.882 helix_view::document [DEBUG] id 1 modified - last saved: 0, current: 0
2024-07-23T02:10:04.007 helix_term::application [DEBUG] received editor event: IdleTimer
2024-07-23T02:10:04.009 helix_view::document [DEBUG] id 1 modified - last saved: 0, current: 0
2024-07-23T02:10:04.518 helix_view::document [DEBUG] id 1 modified - last saved: 0, current: 0
2024-07-23T02:10:04.640 helix_view::document [DEBUG] id 1 modified - last saved: 0, current: 0
2024-07-23T02:10:04.705 helix_view::document [DEBUG] id 1 modified - last saved: 0, current: 0
2024-07-23T02:10:04.828 helix_view::document [DEBUG] id 1 modified - last saved: 0, current: 0
2024-07-23T02:10:04.875 helix_view::document [DEBUG] id 1 modified - last saved: 0, current: 0
2024-07-23T02:10:04.998 helix_view::document [DEBUG] id 1 modified - last saved: 0, current: 0
2024-07-23T02:10:05.088 helix_view::document [DEBUG] id 1 modified - last saved: 0, current: 0
2024-07-23T02:10:05.211 helix_view::document [DEBUG] id 1 modified - last saved: 0, current: 0
2024-07-23T02:10:05.340 helix_view::document [DEBUG] id 1 modified - last saved: 0, current: 0
2024-07-23T02:10:05.341 helix_term::application [DEBUG] received editor event: IdleTimer
2024-07-23T02:10:05.380 helix_view::document [DEBUG] id 1 modified - last saved: 0, current: 1
2024-07-23T02:10:05.631 helix_term::application [DEBUG] received editor event: IdleTimer
2024-07-23T02:10:05.915 helix_view::document [DEBUG] id 1 modified - last saved: 0, current: 1
2024-07-23T02:10:06.168 helix_term::application [DEBUG] received editor event: IdleTimer
2024-07-23T02:10:06.174 helix_view::document [DEBUG] id 1 modified - last saved: 0, current: 1
2024-07-23T02:10:07.651 helix_view::document [DEBUG] submitting save of doc 'Some("/tmp/b")'
2024-07-23T02:10:07.651 helix_term::job [DEBUG] waiting on jobs...
2024-07-23T02:10:07.660 helix_lsp::file_event [DEBUG] Received file event for "/tmp/b"
2024-07-23T02:10:07.660 helix_view::document [DEBUG] doc 1 revision updated 0 -> 2
2024-07-23T02:10:07.660 helix_term::commands::typed [DEBUG] quitting...
2024-07-23T02:10:07.660 helix_view::document [DEBUG] id 1 modified - last saved: 2, current: 2
2024-07-23T02:10:07.660 helix_term::job [DEBUG] waiting on jobs...
2024-07-23T02:10:07.661 helix_term::job [DEBUG] waiting on jobs...

Platform

macOS

Terminal Emulator

iterm2 3.5.3

Installation Method

brew

Helix Version

helix 24.7 (079f544)

@Nemowang2003 Nemowang2003 added the C-bug Category: This is a bug label Jul 22, 2024
@kirawi
Copy link
Member

kirawi commented Jul 22, 2024

Per Vim: https:/neovim/neovim/blob/4e5c633ed4871a948aff7338b793ac5f93484153/src/nvim/bufwrite.c#L736-L742

The current system is coded such that symbolic links work with backup files, but we should disable it when it's a hardlink in

let backup = if path.exists() {
let path_ = write_path.clone();
// hacks: we use tempfile to handle the complex task of creating
// non clobbered temporary path for us we don't want
// the whole automatically delete path on drop thing
// since the path doesn't exist yet, we just want
// the path
tokio::task::spawn_blocking(move || -> Option<PathBuf> {
tempfile::Builder::new()
.prefix(path_.file_name()?)
.suffix(".bck")
.make_in(path_.parent()?, |backup| std::fs::rename(&path_, backup))
.ok()?
.into_temp_path()
.keep()
.ok()
})
.await
.ok()
.flatten()
} else {
None
};

@kirawi kirawi added E-easy Call for participation: Experience needed to fix: Easy / not much A-helix-term Area: Helix term improvements E-good-first-issue Call for participation: Issues suitable for new contributors E-has-instructions Call for participation: Has instructions for fixing the issue and opening a PR labels Jul 22, 2024
@noktoborus
Copy link
Contributor

noktoborus commented Jul 23, 2024

I can add additional check to #11153
in fact, this is not the only problem with the current saving procedure, because there is also a security problem: file access rights are copied only after the file is finally written, which is not very good. Perhaps the temporary file creation function exposes private access to the temporary file, which is not obvious to me and I have not checked.

@kirawi
Copy link
Member

kirawi commented Jul 23, 2024

I will be reading through Vim's write system in full in the near future to see what needs to be changed in Helix.

@cunha
Copy link

cunha commented Jul 27, 2024

Encountered this last Wednesday while teaching. Created a hardlink, hx'd one file, and was going to show the students that the other file was changed too and... wut?!

I was utterly confused for several seconds... then figured out the reference counters were decreased and resorted to using vim for the example.

@kirawi kirawi removed E-easy Call for participation: Experience needed to fix: Easy / not much E-good-first-issue Call for participation: Issues suitable for new contributors E-has-instructions Call for participation: Has instructions for fixing the issue and opening a PR labels Jul 27, 2024
@kirawi
Copy link
Member

kirawi commented Jul 27, 2024

#11340 should fix this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants