runAndStreamAux reads the stdout buffer (and, when output is suppressed,
the combinedOutput buffer) for its error message after handler.wait()
returns, but the goroutine that fills those buffers by draining the
command's output isn't awaited, so the reads raced its final writes.
Own the goroutine here rather than letting the onRun callbacks spawn it,
and join it before reading the buffers. The pty reader reaches EOF on its
own once the process exits, but the non-pty pipe never does, so its
handler now closes the read end to unblock the reader; the pipe is
synchronous, so by the time the command has exited all of its output has
already been read and nothing is lost. This also plugs the goroutine that
the non-pty streaming path previously leaked on every command.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
runAndStreamAux funnels a command's stdout and stderr into a single
cmdWriter (the command-log panel, or a buffer when output is suppressed)
from two separate goroutines: stderr through the MultiWriter set on
cmd.Stderr, and stdout through the onRun callback. Those goroutines
wrote the shared writer without any synchronization, racing on the
prefixWriter's prefixWritten flag and interleaving the two streams.
Wrap the writer so its writes are serialized.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The per-platform getCmdHandlerPty split existed because the Unix side
had creack/pty and the Windows side had nothing — so it fell back to a
non-pty handler. Now that oscommands.StartPty provides a pty on both
platforms, the two files collapse into one cross-platform
implementation and the stub is gone.
cmdHandler grows a 'wait' field because the pty path on Windows spawns
via CreateProcess and never runs exec.Cmd.Start — so cmd.Wait wouldn't
work there. Non-pty handlers set wait = cmd.Wait; pty handlers set it
to the wait closure StartPty returns.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
However, show it when there was an error. This is important for the case that a
fork that you have as a remote was deleted, in which case the command log is the
only way to get notified about that.
Some users reported that en_US.UTF-8 is not available on their systems, leading
to warnings in the command log. "C" also forces the language to English, and is
guaranteed to be available everywhere.
As we just did for tasks, close their stdout pipe instead. This makes the called
process terminate more gracefully.
This isn't a change that we *need* to make, it's just a bit nicer.
Previously we would enter a newline at the password prompt, which would cause
the fetch to fail. The problem with this was that if you have many remotes, the
fetch would sometimes hang for some reason; I don't totally understand how that
happened, but I guess the many ssh processes requesting passwords would somehow
interfere with each other. Avoid this by simply killing the git fetch process
the moment it requests the first password.
This decouples StreamOutput from whether a PTY is used. In most cases we just
want to see the output in the log window, but don't have to use a PTY, e.g. for
the bisect commands.
This has the implication that custom commands that are using "stream: true" no
longer use a PTY. In most cases that's probably a good thing, but we're going to
add a separate pty config for those who really wanted this.
Scanners can return errors (e.g. ErrTooLong), and if we don't handle it, the
cmd.Wait() call below will block forever because nobody drains the command's
output.
This happens for CommitLoader.GetCommits when there's a commit whose subject
line is longer than approx. 65500 characters; in that case, lazygit would lock
up completely. With this fix it remains usable, but the commit list is truncated
before the bad commit, which is not good enough. We'll improve that in the
remaining commits of this branch.
This reverts commit 90613056ce, or the part that removed
a goroutine at least.
Reverting because this has caused an infinite wait for push/pull on windows.
We'll need to find out why that happens separately
This happens consistently for my when I close my MacBook's lid. It seems that
MacOS locks the user's keychain in this case, and since I have my keychain
provide the pass phrases for my ssh keys, fetching fails because it tries to
prompt me for a pass phrase.
This all worked correctly already, we have the FailOnCredentialRequest()
mechanism specifically for this situation, so all is great. The only problem was
that it was trying to pause the ongoing task while prompting the user for input;
but the task is nil for a background fetch (and should be).
It's not clear what was happening but it seemed like we sometimes weren't
fully writing to our stdout buffer (which is used for the error message)
even though we had returned from cmd.Wait().
Not sure what the cause was but removing an unnecessary goroutine fixed it.
The global counter approach is easy to understand but it's brittle and depends on implicit behaviour that is not very discoverable.
With a global counter, if any goroutine accidentally decrements the counter twice, we'll think lazygit is idle when it's actually busy.
Likewise if a goroutine accidentally increments the counter twice we'll think lazygit is busy when it's actually idle.
With the new approach we have a map of tasks where each task can either be busy or not. We create a new task and add it to the map
when we spawn a worker goroutine (among other things) and we remove it once the task is done.
The task can also be paused and continued for situations where we switch back and forth between running a program and asking for user
input.
In order for this to work with `git push` (and other commands that require credentials) we need to obtain the task from gocui when
we create the worker goroutine, and then pass it along to the commands package to pause/continue the task as required. This is
MUCH more discoverable than the old approach which just decremented and incremented the global counter from within the commands package,
but it's at the cost of expanding some function signatures (arguably a good thing).
Likewise, whenever you want to call WithWaitingStatus or WithLoaderPanel the callback will now have access to the task for pausing/
continuing. We only need to actually make use of this functionality in a couple of places so it's a high price to pay, but I don't
know if I want to introduce a WithWaitingStatusTask and WithLoaderPanelTask function (open to suggestions).
I don't know if this is a hack or not: we run a git command and increment the pending action
count to 1 but at some point the command requests a username or password, so we need to prompt
the user to enter that. At that point we don't want to say that there is a pending action,
so we decrement the action count before prompting the user and then re-increment it again afterward.
Given that we panic when the counter goes below zero, it's important that it's not zero
when we run the git command (should be impossible anyway).
I toyed with a different approach using channels and a long-running goroutine that
handles all commands that request credentials but it feels over-engineered compared to this
commit's approach.
The 8.2 release of OpenSSH added support for FIDO/U2F hardware
authenticators, which manifests in being able to create new types of SSH
key, named `ecdsa-sk` nad `ed25519-sk`. This is relevant to lazygit,
as those SSH keys can be used to authorise git operations over SSH, as
well as signing git commits. Actual code changes are required for
correct support, as the authentication process for these types of keys
is different than the process for types supported previously.
When an operation requiring credentials is initialised with a U2F
authenticator-backed key, the first prompt is:
Enter PIN for ${key_type} key ${path_to_key}:
at which point the user is supposed to enter a numeric (and secret) PIN,
specific to the particular FIDO/U2F authenticator using which the SSH
keypair was generated. Upon entering the correct key, the user is
supposed to physically interact with the authenticator to confirm
presence. Sometimes this is accompanied by the following text prompt:
Confirm user presence for key ${key_type} ${key_fingerprint}
This second prompt does not always occur and it is presumed that the
user will know to perform this step even if not prompted specifically.
At this stage some authenticator devices may also begin to blink a LED
to indicate that they're waiting for input.
To facilitate lazygit's interoperability with these types of keys, add
support for the first PIN prompt, which allows "fetch", "pull", and
"push" git operations to complete.
more
and more
move rebase commit refreshing into existing abstraction
and more
and more
WIP
and more
handling clicks
properly fix merge conflicts
update cheatsheet
lots more preparation to start moving things into controllers
WIP
better typing
expand on remotes controller
moving more code into controllers
