search

arxanas / git-branchless

High-velocity, monorepo-scale workflow for Git
Apache License 2.0
3.47k stars 89 forks source link

reword invokes vi instead of nano #371

Closed AaronLieberman closed 2 years ago

AaronLieberman commented 2 years ago

Description of the bug

When I run git reword, vi comes up. When I run other git commands that invoke an editor (e.g. git amend), nano comes up. I didn't have core.editor set on this machine, when I set it to nano, nano shows up as expected.

Expected behavior

Actual behavior

Version of git-branchless

git-branchless 0.3.12

Version of git

git version 2.25.1

Version of rustc

rustc 1.57.0

Automated bug report

Software version

git-branchless 0.3.12

Operating system

Linux 5.10.16.3-microsoft-standard-WSL2

Command-line

/home/aaron/.cargo/bin/git-branchless bug-report

Environment variables

SHELL=/bin/bash
EDITOR=<not set>

Git version

> git version 
git version 2.25.1

Events

Show 5 events ##### Event ID: 11, transaction ID: 24 1. `CommitEvent { timestamp: 1650765291.0, event_tx_id: EventTransactionId(24), commit_oid: NonZeroOid(8cc55d6085aaeceef2a1e5ac4c87db6a51e6fdb9) }` 1. `RewriteEvent { timestamp: 1650765292.4729295, event_tx_id: EventTransactionId(24), old_commit_oid: f148352480d981f078b609f5a49870baa2e6d107, new_commit_oid: 8cc55d6085aaeceef2a1e5ac4c87db6a51e6fdb9 }` ``` : O 79d815c 13d xxx xxxxxxx |\ | @ 8cc55d6 6m xxx xx xxxxxxx xxxxxx | X f148352 13d (rewritten as 8cc55d60) (remote origin/main) xxx xx xxxxxxx xxxxxx |\ | o 6f6a4ae 17m xxx | o 44087e1 17m xxxx ``` ##### Event ID: 9, transaction ID: 21 1. `UnobsoleteEvent { timestamp: 1650765273.3941846, event_tx_id: EventTransactionId(21), commit_oid: NonZeroOid(6f6a4aebcb2212ade943d71e0f2b014bcfe8bbb8) }` 1. `UnobsoleteEvent { timestamp: 1650765273.3941846, event_tx_id: EventTransactionId(21), commit_oid: NonZeroOid(44087e17f545f048b596afe484d5f69e5ed1c0b5) }` ``` : O 79d815c 13d xxx xxxxxxx |\ | @ 8cc55d6 6m xxx xx xxxxxxx xxxxxx | X f148352 13d (remote origin/main) xxx xx xxxxxxx xxxxxx |\ | o 6f6a4ae 17m xxx | o 44087e1 17m xxxx ``` ##### Event ID: 7, transaction ID: 14 1. `ObsoleteEvent { timestamp: 1650764708.9390576, event_tx_id: EventTransactionId(14), commit_oid: NonZeroOid(6f6a4aebcb2212ade943d71e0f2b014bcfe8bbb8) }` 1. `ObsoleteEvent { timestamp: 1650764708.9390576, event_tx_id: EventTransactionId(14), commit_oid: NonZeroOid(44087e17f545f048b596afe484d5f69e5ed1c0b5) }` ``` : O 79d815c 13d xxx xxxxxxx |\ | @ 8cc55d6 6m xxx xx xxxxxxx xxxxxx | X f148352 13d (remote origin/main) xxx xx xxxxxxx xxxxxx |\ | o 6f6a4ae 17m (manually hidden) xxx | o 44087e1 17m (manually hidden) xxxx ``` ##### Event ID: 6, transaction ID: 9 1. `RefUpdateEvent { timestamp: 1650764674.0052335, event_tx_id: EventTransactionId(9), ref_name: "HEAD", old_oid: 44087e17f545f048b596afe484d5f69e5ed1c0b5, new_oid: f148352480d981f078b609f5a49870baa2e6d107, message: None }` ``` : O 79d815c 13d xxx xxxxxxx |\ | @ 8cc55d6 6m xxx xx xxxxxxx xxxxxx | X f148352 13d (remote origin/main) xxx xx xxxxxxx xxxxxx |\ | o 6f6a4ae 17m xxx | o 44087e1 17m xxxx ``` ##### Event ID: 3, transaction ID: 7 1. `RefUpdateEvent { timestamp: 1650764659.421517, event_tx_id: EventTransactionId(7), ref_name: "refs/heads/redacted-ref-0", old_oid: c8e87fac7ee66188b07f1adb3a7ab9f65e9ee431, new_oid: 44087e17f545f048b596afe484d5f69e5ed1c0b5, message: None }` 1. `RewriteEvent { timestamp: 1650764659.457445, event_tx_id: EventTransactionId(7), old_commit_oid: c8e87fac7ee66188b07f1adb3a7ab9f65e9ee431, new_commit_oid: 44087e17f545f048b596afe484d5f69e5ed1c0b5 }` 1. `RefUpdateEvent { timestamp: 1650764659.4946306, event_tx_id: EventTransactionId(7), ref_name: "HEAD", old_oid: c8e87fac7ee66188b07f1adb3a7ab9f65e9ee431, new_oid: 44087e17f545f048b596afe484d5f69e5ed1c0b5, message: None }` ``` : O 79d815c 13d xxx xxxxxxx |\ | @ 8cc55d6 6m xxx xx xxxxxxx xxxxxx | X f148352 13d (remote origin/main) xxx xx xxxxxxx xxxxxx |\ | o 6f6a4ae 17m xxx | o 44087e1 17m xxxx ```
claytonrcarter commented 2 years ago

Hi! Thanks for the report and sorry it's not working as expected. You already mentioned what you have for core.editor, but can you clarify a couple more things: do you have anything set for the following env vars: $GIT_EDITOR, $VISUAL and $EDITOR?

We try to follow the order of resolution from https://git-scm.com/docs/git-var#Documentation/git-var.txt-GITEDITOR.

AaronLieberman commented 2 years ago

I just checked on my Windows WSL2 Ubuntu box and I don't have any of them set. 

aaron@aaron-bld:~/repos/ExpressionTree$ echo "EDITOR=$EDITOR, VISUAL=$VISUAL, GIT_EDITOR=$GIT_EDITOR, core.editor=$(git config --get core.editor)"
EDITOR=, VISUAL=, GIT_EDITOR=, core.editor=

On a native Linux Ubuntu box, I'm getting even different behavior:

aaron@####:~/repos/ExpressionTree$ echo "EDITOR=$EDITOR, VISUAL=$VISUAL, GIT_EDITOR=$GIT_EDITOR, core.editor=$(git config --get core.editor)"
EDITOR=, VISUAL=, GIT_EDITOR=, core.editor=nano

But git reword is still invoking vi

And when I run the same command on that first machine from PowerShell (native Windows terminal), I actually get a crash. 

[101] ! D:\Projects\Git\ExpressionTree > git reword 777ac1f
The application panicked (crashed).
Message:  A fatal error occurred:
   0: Access is denied. (os error 5)

Location:
   /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c\library\core\src\result.rs:1914

  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ SPANTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

   0: git_branchless::commands::reword::build_messages with repo=<Git repository at: "D:/Projects/Git/ExpressionTree/.git/"> messages=Messages([]) commits=[Commit { inner: Commit { id: 777ac1f503f886aa09ea66499a7a993642b726ed, summary: "file1" } }]
      at C:\Users\aaron\.cargo\registry\src\github.com-1ecc6299db9ec823\git-branchless-0.3.12\src\commands\reword.rs:243
   1: git_branchless::commands::reword::reword with effects=<Output fancy=true> hashes=["777ac1f"] messages=Messages([]) git_run_info=<GitRunInfo path_to_git="git" working_directory="D:\\Projects\\Git\\ExpressionTree" env=not shown>
      at C:\Users\aaron\.cargo\registry\src\github.com-1ecc6299db9ec823\git-branchless-0.3.12\src\commands\reword.rs:41

  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ BACKTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
   1: git_filter_source_repo<unknown>
      at <unknown source file>:<unknown line>
   2: git_filter_source_repo<unknown>
      at <unknown source file>:<unknown line>
   3: git_filter_source_repo<unknown>
      at <unknown source file>:<unknown line>
   4: git_patch_owner<unknown>
      at <unknown source file>:<unknown line>
   5: git_filter_source_path<unknown>
      at <unknown source file>:<unknown line>
   6: git_patch_owner<unknown>
      at <unknown source file>:<unknown line>
   7: git_smart_subtransport_git<unknown>
      at <unknown source file>:<unknown line>
   8: BaseThreadInitThunk<unknown>
      at <unknown source file>:<unknown line>
   9: RtlUserThreadStart<unknown>
      at <unknown source file>:<unknown line>
claytonrcarter commented 2 years ago

I'm rapidly approaching "stumped". I mentioned that we try to follow git's behavior, but technically it's closer to this:

  1. We first try $GIT_EDITOR, then core.editor, using the first that is set.
  2. If neither is set, then we just let the Dialoguer library handle it; they try $VISUAL, then $EDITOR.
  3. If neither of those is set, then Dialoguer defaults to vi (or notepad on Windows).

The difference here is that Dialoguer defaults to vi/notepad but, as I understand it, git's default editor can be configured at compile time. So it's possible that the system you're on has been configured to default nano or something, but that's just a guess. I think that you can try git var -l (or even git var -l | grep EDITOR) to see what git is configured to use. I found a little bit of context here: https://askubuntu.com/questions/709029/how-do-i-find-out-what-my-default-terminal-text-editor-is. For me, git var -l | grep EDITOR shows vi, even though I don't have any of these envs/configs set. I'm assuming that this means that my git was compiled to default to vi, but that's just another guess.

In your first case (WSL) w/ nothing set, it makes sense that you're seeing vi (per above), but in your second case, it sure seems like a defect that nano isn't being used. Why isn't not being used is stumping me. I just tried git config core.editor nano ; git reword and it loaded nano. (This is on a mac.)

As for the panic ... uh oh. For the 0.3.12 release, item 0 in the spantrace (reword.rs:243) is reword::build_messages(). My first assumption is that the "Access is denied" error has to do w/ creating the temporary file to edit (which is done by Dialoguer, and happens w/i build_messages(), but I don't know how to dig into this any further. It could also be related to reading some config values from git, I suppose. That's above my head atm; we'll have to get input from @arxanas.

arxanas commented 2 years ago

git amend as offered by git-branchless does not open an editor at all; is it possible that you have a git amend alias which manually sets the editor?

Can you run git var GIT_EDITOR and paste the output here? Also, can you try git config --show-origin --get core.editor? I wouldn't be surprised if it didn't work because the config var was being resolved in one way by Git but another way by libgit2

Re. the Powershell crash, I think we should assume it's the same issue as in https://github.com/arxanas/git-branchless/issues/370, as they both involve being unable to find/execute a shelled-out executable.

I wasn't able to reproduce the core.editor issue, i.e. this worked:

$ git config core.editor nano
$ git reword  # opens in nano
$ git config --unset core.editor
$ git reword  # opens in vim
arxanas commented 2 years ago

Is it possible that you haven't installed the latest version from source in a while? git reword only started respecting core.editor as of https://github.com/arxanas/git-branchless/commit/91d9401af5019a4593289211a6a19328f025e5c1, so you could have missed that change.

AaronLieberman commented 2 years ago

Sorry I didn't reply earlier, got distracted by other projects.

git amend as offered by git-branchless does not open an editor at all; is it possible that you have a git amend alias which manually sets the editor? Ah right. I do have an alias for git amend (from before branchless added this feature) but it doesn't set the editor: amend = commit --amend --date=now I should probably rename my alias and try out git branchless' amend, but that's a different topic.

The interesting thing is that I'm seeing the same thing on my native Ubuntu 20.04 box, even with a recent git and the newest branchless release:

> git version
git version 2.34.1
> git branchless --version
git-branchless 0.3.12
> git var -l | grep EDITOR
GIT_EDITOR=editor
> git config --show-origin --get core.editor
<empty>
> git config --show-scope --get core.editor
<empty>

Just tried it again. When I run git reword, I get vi but when I run git commit --amend, I get nano.

Here's something:

> which editor
/usr/bin/editor
> editor
<nano shows up>

Not sure if that's somehow related.

Now that I know to use core.editor, it's not a big deal to me, low pri bug I guess. Also, just to mention it: reword itself is great, I'm using it quite often now that I have it in my toolbox. Thanks for adding it!

arxanas commented 2 years ago

@AaronLieberman can you try https://github.com/arxanas/git-branchless/pull/387 and see if it resolves the issue on your end? You can install with something like cargo install --git https://github.com/arxanas/git-branchless --branch reword.

AaronLieberman commented 2 years ago

No dice. Looks like it can't find git? 

 ~/repos/firmware3 > RUST_BACKTRACE=1 git reword 2773b4a
The application panicked (crashed).
Message:  A fatal error occurred: 
   0: No such file or directory (os error 2)

Location:
   /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/core/src/result.rs:1914

  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ SPANTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

   0: git_branchless::commands::reword::prepare_messages with repo=<Git repository at: "/repos/aaron/firmware3/.git/"> messages=Messages([]) commits=[Commit { inner: Commit { id: 2773b4a9c5825c2ebc2954c38905649e5e02eeb3, summary: "[...]" } }]
      at git-branchless/src/commands/reword.rs:322
   1: git_branchless::commands::reword::reword with effects=<Output fancy=true> hashes=["2773b4a"] messages=Messages([]) git_run_info=<GitRunInfo path_to_git="git" working_directory="/repos/aaron/firmware3" env=not shown>
      at git-branchless/src/commands/reword.rs:45

  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ BACKTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
                                ⋮ 2 frames hidden ⋮                               
   3: git_branchless::commands::reword::prepare_messages::h8b20dc6c3a1edc6a
      at <unknown source file>:<unknown line>
   4: git_branchless::commands::reword::reword::h97d5ec5402a446c9
      at <unknown source file>:<unknown line>
   5: git_branchless::commands::do_main_and_drop_locals::hb35f1f9f2874e242
      at <unknown source file>:<unknown line>
   6: git_branchless::commands::main::h47f8d83b735eba77
      at <unknown source file>:<unknown line>
   7: std::sys_common::backtrace::__rust_begin_short_backtrace::h72a81c5f115e6fd0
      at <unknown source file>:<unknown line>
   8: std::rt::lang_start::{{closure}}::h45f83d954752380a
      at <unknown source file>:<unknown line>
   9: core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::hc56adab7a77ec6e3
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/core/src/ops/function.rs:259
  10: std::panicking::try::do_call::h29f013120c5abc65
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panicking.rs:403
  11: std::panicking::try::h86d5b2b66caec4cf
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panicking.rs:367
  12: std::panic::catch_unwind::h7dd136d787f51397
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panic.rs:133
  13: std::rt::lang_start_internal::{{closure}}::h4a199351e630a8a5
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/rt.rs:128
  14: std::panicking::try::do_call::h20ceb9e5dff838c6
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panicking.rs:403
  15: std::panicking::try::hc2abb46a5e41bd43
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panicking.rs:367
  16: std::panic::catch_unwind::h52c3eb4408ad6dfb
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panic.rs:133
  17: std::rt::lang_start_internal::hd15a47be08101c28
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/rt.rs:128
  18: main<unknown>
      at <unknown source file>:<unknown line>
  19: __libc_start_main<unknown>
      at <unknown source file>:<unknown line>
  20: _start<unknown>
      at <unknown source file>:<unknown line>

Run with COLORBT_SHOW_HIDDEN=1 environment variable to disable frame filtering.
Run with RUST_BACKTRACE=full to include source snippets.
Location: git-branchless/src/commands/mod.rs:317

  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ BACKTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
                                ⋮ 8 frames hidden ⋮                               
   9: core::result::Result<T,E>::expect::h79a883e99a520bf2
      at <unknown source file>:<unknown line>
  10: git_branchless::commands::main::h47f8d83b735eba77
      at <unknown source file>:<unknown line>
  11: std::sys_common::backtrace::__rust_begin_short_backtrace::h72a81c5f115e6fd0
      at <unknown source file>:<unknown line>
  12: std::rt::lang_start::{{closure}}::h45f83d954752380a
      at <unknown source file>:<unknown line>
  13: core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::hc56adab7a77ec6e3
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/core/src/ops/function.rs:259
  14: std::panicking::try::do_call::h29f013120c5abc65
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panicking.rs:403
  15: std::panicking::try::h86d5b2b66caec4cf
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panicking.rs:367
  16: std::panic::catch_unwind::h7dd136d787f51397
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panic.rs:133
  17: std::rt::lang_start_internal::{{closure}}::h4a199351e630a8a5
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/rt.rs:128
  18: std::panicking::try::do_call::h20ceb9e5dff838c6
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panicking.rs:403
  19: std::panicking::try::hc2abb46a5e41bd43
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panicking.rs:367
  20: std::panic::catch_unwind::h52c3eb4408ad6dfb
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panic.rs:133
  21: std::rt::lang_start_internal::hd15a47be08101c28
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/rt.rs:128
  22: main<unknown>
      at <unknown source file>:<unknown line>
  23: __libc_start_main<unknown>
      at <unknown source file>:<unknown line>
  24: _start<unknown>
      at <unknown source file>:<unknown line>

Run with COLORBT_SHOW_HIDDEN=1 environment variable to disable frame filtering.
Run with RUST_BACKTRACE=full to include source snippets.

My git info:

~/repos/firmware3 > which git
git: aliased to git-branchless wrap
~/repos/firmware3 > bash
~/repos/firmware3 > unalias git
~/repos/firmware3 > which git
/usr/bin/git
arxanas commented 2 years ago

Hmm, that's a bit strange, considering that we actually ignore failure to launch git in the code I added.

The spantrace unfortunately doesn't make it clear where exactly the issue is happening. I pushed a version with additional instrumentation to the reword branch. Could you install it and try again? Also, use the --debug flag this time, and maybe it'll give better backtraces:

cargo install --debug --git https://github.com/arxanas/git-branchless --branch reword

It's also possible that it's some unrelated regression on master, but either way, the added instrumentation should help diagnose the issue.

AaronLieberman commented 2 years ago

Even with the debug flag specified, I'm surprised that I'm not getting line numbers in those callstacks. I see lines 45 and 323 but they seem to correspond to function definitions, not lines in the functions. 

~/repos/firmware3/components/UI $ RUST_BACKTRACE=1 RUST_BACKTRACE=full COLORBT_SHOW_HIDDEN=1 git reword a9988a8 
The application panicked (crashed).
Message:  A fatal error occurred: 
   0: Invoking editor: editor
   0: 
   1: No such file or directory (os error 2)

Location:
   git-branchless/src/commands/reword.rs:80

  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ SPANTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

   0: git_branchless::commands::reword::prepare_messages with repo=<Git repository at: "/repos/aaron/firmware3/.git/"> messages=Messages([]) commits=[Commit { inner: Commit { id: a9988a863f0827c730501a274f54367a2fe0dd35, summary: "enabled debugmenu" } }]
      at git-branchless/src/commands/reword.rs:323
   1: git_branchless::commands::reword::reword with effects=<Output fancy=true> hashes=["a9988a8"] messages=Messages([]) git_run_info=<GitRunInfo path_to_git="git" working_directory="/repos/aaron/firmware3/components/UI" env=not shown>
      at git-branchless/src/commands/reword.rs:45

  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ BACKTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
   1: color_eyre::config::EyreHook::default::he2b2ed0becaae79d
      at <unknown source file>:<unknown line>
   2: color_eyre::config::EyreHook::into_eyre_hook::{{closure}}::he0d22b2e5aca9291
      at <unknown source file>:<unknown line>
   3: eyre::capture_handler::h34c94b221354d542
      at <unknown source file>:<unknown line>
   4: eyre::error::<impl eyre::Report>::from_msg::ha5db9ea5cee82e6c
      at <unknown source file>:<unknown line>
   5: <E as eyre::context::ext::StdError>::ext_report::h5ce39199296aa87c
      at <unknown source file>:<unknown line>
   6: eyre::context::<impl eyre::WrapErr<T,E> for core::result::Result<T,E>>::wrap_err_with::h1460d868da91cda7
      at <unknown source file>:<unknown line>
   7: eyre::context::<impl eyre::WrapErr<T,E> for core::result::Result<T,E>>::with_context::h1fb341f2035075af
      at <unknown source file>:<unknown line>
   8: git_branchless::commands::reword::reword::{{closure}}::h8757844e4fa1dc16
      at <unknown source file>:<unknown line>
   9: git_branchless::commands::reword::prepare_messages::h7729549d6e2fb3f1
      at <unknown source file>:<unknown line>
  10: git_branchless::commands::reword::reword::h56002d5d11e2cef4
      at <unknown source file>:<unknown line>
  11: git_branchless::commands::do_main_and_drop_locals::hcaea6a9c8d4a70c3
      at <unknown source file>:<unknown line>
  12: git_branchless::commands::main::h16bd78d9b0b5482e
      at <unknown source file>:<unknown line>
  13: git_branchless::main::hea01478994f57b6b
      at <unknown source file>:<unknown line>
  14: core::ops::function::FnOnce::call_once::hb3cd1f65379a43ad
      at <unknown source file>:<unknown line>
  15: std::sys_common::backtrace::__rust_begin_short_backtrace::hf617344285ce7510
      at <unknown source file>:<unknown line>
  16: std::rt::lang_start::{{closure}}::h3b265e1e24727799
      at <unknown source file>:<unknown line>
  17: core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::hc56adab7a77ec6e3
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/core/src/ops/function.rs:259
  18: std::panicking::try::do_call::h29f013120c5abc65
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panicking.rs:403
  19: std::panicking::try::h86d5b2b66caec4cf
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panicking.rs:367
  20: std::panic::catch_unwind::h7dd136d787f51397
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panic.rs:133
  21: std::rt::lang_start_internal::{{closure}}::h4a199351e630a8a5
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/rt.rs:128
  22: std::panicking::try::do_call::h20ceb9e5dff838c6
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panicking.rs:403
  23: std::panicking::try::hc2abb46a5e41bd43
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panicking.rs:367
  24: std::panic::catch_unwind::h52c3eb4408ad6dfb
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panic.rs:133
  25: std::rt::lang_start_internal::hd15a47be08101c28
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/rt.rs:128
  26: std::rt::lang_start::hf48e1908950d3779
      at <unknown source file>:<unknown line>
  27: main<unknown>
      at <unknown source file>:<unknown line>
  28: __libc_start_main<unknown>
      at <unknown source file>:<unknown line>
  29: _start<unknown>
      at <unknown source file>:<unknown line>

Run with COLORBT_SHOW_HIDDEN=1 environment variable to disable frame filtering.
Location: git-branchless/src/commands/mod.rs:317

  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ BACKTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
   1: color_eyre::config::PanicHook::panic_report::haae5ee0f84be8542
      at <unknown source file>:<unknown line>
   2: color_eyre::config::PanicHook::into_panic_hook::{{closure}}::h84b8f71406806e55
      at <unknown source file>:<unknown line>
   3: std::panicking::rust_panic_with_hook::h50680ff4b44510c6
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panicking.rs:628
   4: std::panicking::begin_panic_handler::{{closure}}::h9371c0fbb1e8465a
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panicking.rs:521
   5: std::sys_common::backtrace::__rust_end_short_backtrace::h9b3efa22a5768c0f
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/sys_common/backtrace.rs:139
   6: rust_begin_unwind<unknown>
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panicking.rs:517
   7: core::panicking::panic_fmt::h23b9203e89cc61cf
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/core/src/panicking.rs:100
   8: core::result::unwrap_failed::h32ef6b3156e8fc57
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/core/src/result.rs:1616
   9: core::result::Result<T,E>::expect::he02be75c6bdd1876
      at <unknown source file>:<unknown line>
  10: git_branchless::commands::main::h16bd78d9b0b5482e
      at <unknown source file>:<unknown line>
  11: git_branchless::main::hea01478994f57b6b
      at <unknown source file>:<unknown line>
  12: core::ops::function::FnOnce::call_once::hb3cd1f65379a43ad
      at <unknown source file>:<unknown line>
  13: std::sys_common::backtrace::__rust_begin_short_backtrace::hf617344285ce7510
      at <unknown source file>:<unknown line>
  14: std::rt::lang_start::{{closure}}::h3b265e1e24727799
      at <unknown source file>:<unknown line>
  15: core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::hc56adab7a77ec6e3
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/core/src/ops/function.rs:259
  16: std::panicking::try::do_call::h29f013120c5abc65
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panicking.rs:403
  17: std::panicking::try::h86d5b2b66caec4cf
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panicking.rs:367
  18: std::panic::catch_unwind::h7dd136d787f51397
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panic.rs:133
  19: std::rt::lang_start_internal::{{closure}}::h4a199351e630a8a5
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/rt.rs:128
  20: std::panicking::try::do_call::h20ceb9e5dff838c6
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panicking.rs:403
  21: std::panicking::try::hc2abb46a5e41bd43
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panicking.rs:367
  22: std::panic::catch_unwind::h52c3eb4408ad6dfb
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panic.rs:133
  23: std::rt::lang_start_internal::hd15a47be08101c28
      at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/rt.rs:128
  24: std::rt::lang_start::hf48e1908950d3779
      at <unknown source file>:<unknown line>
  25: main<unknown>
      at <unknown source file>:<unknown line>
  26: __libc_start_main<unknown>
      at <unknown source file>:<unknown line>
  27: _start<unknown>
      at <unknown source file>:<unknown line>
claytonrcarter commented 2 years ago

Thank you @AaronLieberman for sticking with this! If I'm reading this correctly (which may be a big if!), it looks like the error is happening within Dialoguer, specifically when we call .edit() to actually invoke the editor. That fn is here: https://github.com/mitsuhiko/dialoguer/blob/master/src/edit.rs#L91-L126

Reading through that, it makes me wonder if either of these could be an issue:

  1. There could be an issue creating the tempfile for the editor, or reading some of the metadata.
  2. There could be an issue executing the editor.

FWIW, I was able to reproduce the error message ("No such file or directory (os error 2)") by running reword with a bogus editor like GIT_EDITOR=does-not-exist git reword. I also found this issue that also generated "No such file or directory (os error 2)" b/c Dialoguer couldn't find/run the executable.

Can you play around w/ different iterations of stuff like this to see if any of them work?

AaronLieberman commented 2 years ago

Thank you AaronLieberman for sticking with this! No, thank you for working to chase this down!

Using release version of 0.3.12:

git reword

vi

GIT_EDITOR=editor git reword

nano

GIT_EDITOR=/usr/bin/editor git reword

nano

GIT_EDITOR=nano git reword

nano

GIT_EDITOR=/path/to/nano git reword

nano (/usr/bin/nano)

So it seems like setting GIT_EDITOR works fine. Also, as before, setting core.editor also works.

arxanas commented 2 years ago 
Message:  A fatal error occurred: 
   0: Invoking editor: editor
   0: 
   1: No such file or directory (os error 2)

Oops. The 0: is appearing twice there because the error framework is prepending it to each line. The editor string has a newline at the end because it was the result of a shell command. I meant to trim it 🙃. Want to try https://github.com/arxanas/git-branchless/pull/387 again?

If it still doesn't work, then we might want to use resolve the full path of the executable manually, perhaps using the which crate.

Even with the debug flag specified, I'm surprised that I'm not getting line numbers in those callstacks

That's surprising to me; I don't know why the backtrace doesn't include them.

I see lines 45 and 323 but they seem to correspond to function definitions, not lines in the functions.

This is correct for the spantrace, as the #[instrument] annotation only applies to the manually-annotated function entries. It's also possible to insert explicit span! calls, but I haven't been doing that. (The spantrace is the system by which you can see the actual parameter values in the span-trace). I've also added .context(...) calls to my bit of the code, which appear in the numbered list, but the library code doesn't include anything like that, so it's a black box for the spantrace.

AaronLieberman commented 2 years ago

Nice find! Some good news and some bad news:

> echo a >> a.txt
> git add a.txt
> git commit -m 'foo'
branchless: processing 1 update: ref HEAD
branchless: processed commit: b2e185e foo
[detached HEAD b2e185e7a894] foo
 1 file changed, 1 insertion(+)
 create mode 100644 components/UI/a.txt
>  git reword b2e185e7a894

image

Also, thanks for explaining a bit about the way the backtraces in rust work. Rust feels like quite a big learning curve from C++!

arxanas commented 2 years ago

Good news is that it works now, nano comes up even when I don't have my core.editor set.

Great!

Bad news is that it seems to prepend the git command to the top of the buffer:

This is actually intentional — if you're rewording multiple commits, they'll each have a header starting with ++ reword. For the case of one commit, it looks like this. It will work fine if you start editing on the second line.

That being said, @claytonrcarter, maybe it would make sense to remove the header if only one commit is being reworded? In addition to the user surprise here, I also personally have the problem that I want to immediately start typing on the focused line, but then I accidentally overwrite the reword header.

Also, thanks for explaining a bit about the way the backtraces in rust work.

For some things, Rust chose to draw on the extensive body of existing PL research. For example, algebraic data types and pattern matching date from the 1970s, but C++ still doesn't have an ergonomic solution to implement the same.

For other things, they chose to start from scratch, like backtraces for exceptions. I don't know why 🙃. It's the case that backtraces become misleading when used in combination with async/await, so maybe they wanted to hold off on the design and implementation in order to account for that? The spantrace business is entirely in userland, though.

AaronLieberman commented 2 years ago

Awesome. I hadn't successfully used reword yet so didn't know about that behavior. Makes sense and will be even nicer if/when #397 is implemented. I pulled master branch after your merge and tried it, working great now. Thanks for chasing this down!