Skip to content

Credentials helper launching in all its variety#496

Merged
Sebastian Thiel (Byron) merged 244 commits into
mainfrom
filter-refs-by-spec
Sep 5, 2022
Merged

Credentials helper launching in all its variety#496
Sebastian Thiel (Byron) merged 244 commits into
mainfrom
filter-refs-by-spec

Conversation

@Byron

@Byron Sebastian Thiel (Byron) commented Aug 22, 2022

Copy link
Copy Markdown
Member

Tasks

  • consider protocol.*.allow
  • credentials
    • support helper functions (instead of programs) no need, only interesting for testing actually
    • git-command to spawn commands 'git-style' and eventually also hooks, somewhat similar to run-command.c.
    • support for custom credential helper programs and usage of the credential.helper configuration (via git-sec)
    • credentials context
    • custom credential helper programs and test
    • custom prompts via git-prompt
      • askpass
      • proper prompts
      • an expectrl driven test
  • configuration of credential-helpers with url-based config.
    • empty string clears list
    • url normalization to not stumble over trailing slashes (minimal, probably not as complete)
    • useHttpPath
    • patterns in the domain
    • case-sensitivity
    • username (if not provided by the url we matched against)
  • find a way to not use hardcoded defaults. Do it by injecting a global configuration file based on git invocations.
    - on MacOS /usr/bin/git the 'system' config is scope 'unknown' and would need to be parsed via -l --show-origin and the first line is the path. '--system' is not present. On windows, --system works as expected and these count as installation directory.
  • support overriding the credentials helper (while allowing it to launch the built-in one) with a custom function
  • take another look at the tests skipped on linux
  • gix remote refs for non-symbolic remote, like in PR checkouts
  • gix credential as equivalent to git credential, just for fun

Next PR

  • filter remote refs by ref-spec and provide information on which local refs would be updated (TBD next)
  • refactor: git-sec remove tag from Access, make a check that only returns on Option, ignoring Deny.

Out of scope

  • proxy support
  • mirror support
  • All information related to pushing
  • promisor objects
  • branch.<name>.merge support, as we don't yet merge anything and fetching still fetches the entire refspec of as configured in the remote.

Predecessor

It's special as it creates its own repository.
Previously this was hard-coded to `radicle`, now it's just an extension
scheme along with a statically known string. This means we have to
explicitly support new formats which should be fine.
…upport (#450)

No leniency because it's hard to decide what to do, allowing connections
is probably not it, so better fail with the correct error message.
…450)

This wasn't supposed to happen but a requirement to get `serde` support
back.
It's actually used and useful.
align usage of stdin and stdout to factor it elsewhere.
avoid using actual credentials helper where none is required
…module. (#450)

Also allow to pass arbitrary bytes (more or less) as context by not
forcing it all into a string. Values can now be everything, which
helps with passing paths or other values.
Conflicts:
	cargo-smart-release/Cargo.toml
	experiments/diffing/Cargo.toml
	experiments/object-access/Cargo.toml
	experiments/traversal/Cargo.toml
	git-attributes/Cargo.toml
	git-config/Cargo.toml
	git-odb/Cargo.toml
	git-pack/Cargo.toml
	git-ref/Cargo.toml
	git-repository/Cargo.toml
	git-url/Cargo.toml
	gitoxide-core/Cargo.toml
Needs integration with Repo configuration, which includes prompt
options actually.
When true, default false, inject the git installation configuration file
if present at the cost of one `git config` invocation.

Note that we rely on the underlying `git-config` crate to not load
duplicate files.

We also currently lie about the scope which is actually unclear - have
seen 'unknown' or normal scopes like `system`.
#450)

This is important as it may contain additional credential helper
configuration that we definitely need to find the credentials that
git usually finds.
@Byron
Sebastian Thiel (Byron) merged commit 5c05198 into main Sep 5, 2022
@Byron
Sebastian Thiel (Byron) deleted the filter-refs-by-spec branch September 5, 2022 11:00
This was referenced Sep 5, 2022
Eliah Kagan (EliahKagan) added a commit to EliahKagan/gitoxide that referenced this pull request May 17, 2026
`gix-command` was already using the `command_name` operand to set `$0`
inside `-c` invocations, but the value passed was a fixed literal: `--`
historically (since GitoxideLabs#496) and most recently `"sh"`. Both have the same
shortcoming: they decouple the value the shell uses to identify itself
in diagnostic messages from the shell that is actually running.

The fixed literal `"sh"` improves over `"--"` only when the shell really
is `sh`. When a caller selects a different shell via
`Prepare::with_shell_program(...)`, the shell will still announce itself
as `sh` in any error output. For example, with bash configured and an
invalid command, errors would read `sh: line 1: ...` even though bash
is what produced them.

Derive the value from the shell program itself instead, using the
basename of the path that is passed to `Command::new` for the shell.
Concretely:

  - With the default shell on Unix (`/bin/sh`), `$0` is `sh`. Same as
    before in practice, but no longer hardcoded.
  - With the default shell on Windows (`sh.exe` from Git for Windows),
    `$0` is `sh.exe`, matching what's actually being launched rather
    than a literal that drops the `.exe`.
  - With a customized `shell_program`, `$0` reflects the chosen shell
    (e.g. `bash`, `zsh`), so its diagnostics correctly identify it.

The basename avoids the MSYS2 path-translation surprise that the full
shell path can trigger on Windows, where Cygwin/MSYS2 programs rewrite
absolute Windows paths handed to them via `lpCommandLine`. Filenames
without path separators do not trigger that rewrite.

The construction-test assertions in `gix-command/tests/command.rs` and
`gix-credentials/tests/program/from_custom_definition.rs` are updated
to derive the expected `command_name` slot from the same source (a new
`SH_BASENAME` `LazyLock` alongside `SH`), so the assertions remain
correct on Windows where the basename is `sh.exe` rather than `sh`. The
one test that configures a custom shell
(`single_and_multiple_arguments_as_part_of_command_with_given_shell`)
now expects the basename of its `/somepath/to/bash` argument, i.e.
`bash`, which is what the corrected code will produce there. The
previously-added `spawn::with_shell::dollar_zero_*` tests, which spawn
the shell and read `$0` directly, now pass under this implementation.

Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
Eliah Kagan (EliahKagan) added a commit to EliahKagan/gitoxide that referenced this pull request May 17, 2026
`gix-command` already used the `command_name` operand to set `$0` inside
`-c` invocations, but the value passed was a fixed literal: `--`
historically (since GitoxideLabs#496) and most recently `"sh"`. Both have the same
shortcoming: they decouple the value the shell uses to identify itself
in diagnostic messages from the shell that is actually running.

The fixed literal `"sh"` improves over `"--"` only when the shell really
is `sh`. When a caller selects a different shell via
`Prepare::with_shell_program(...)`, the shell will still announce itself
as `sh` in any error output. For example, with bash configured and an
invalid command, errors would read `sh: line 1: ...` even though bash
is what produced them.

Derive the value from the shell program itself, using the basename of
the path that is passed to `Command::new` for the shell. Concretely:

  - With the default shell on Unix (`/bin/sh`), `$0` is `sh`. Same as
    before in practice, but no longer hardcoded.
  - With the default shell on Windows (`sh.exe` from Git for Windows),
    `$0` is `sh.exe`, matching what's actually being launched rather
    than a literal that drops the `.exe`.
  - With a customized `shell_program`, `$0` reflects the chosen shell
    (e.g. `bash`, `zsh`), so its diagnostics correctly identify it.

The basename avoids the MSYS2 path-translation surprise that the full
shell path can trigger on Windows, where Cygwin/MSYS2 programs rewrite
absolute Windows paths handed to them via `lpCommandLine`. Filenames
without path separators do not trigger that rewrite.

The tests added in the preceding commit now pass under this
implementation, including the construction-test assertions whose
`SH_BASENAME`-derived expected value matches `sh.exe` on Windows and
the bash-specific assertion whose expected value is `"bash"`.

Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
Eliah Kagan (EliahKagan) added a commit to EliahKagan/gitoxide that referenced this pull request May 17, 2026
`gix-command` already used the `command_name` operand to set `$0` inside
`-c` invocations, but the value passed was a fixed literal: `--`
historically (since GitoxideLabs#496) and most recently `"sh"`. Both have the same
shortcoming: they decouple the value the shell uses to identify itself
in diagnostic messages from the shell that is actually running.

The fixed literal `"sh"` improves over `"--"` only when the shell really
is `sh`. When a caller selects a different shell via
`Prepare::with_shell_program(...)`, the shell will still announce itself
as `sh` in any error output. For example, with bash configured and an
invalid command, errors would read `sh: line 1: ...` even though bash
is what produced them.

Derive the value from the shell program itself, using the basename of
the path that is passed to `Command::new` for the shell. Concretely:

  - With the default shell on Unix (`/bin/sh`), `$0` is `sh`. Same as
    before in practice, but no longer hardcoded.
  - With the default shell on Windows (`sh.exe` from Git for Windows),
    `$0` is `sh.exe`, matching what's actually being launched rather
    than a literal that drops the `.exe`.
  - With a customized `shell_program`, `$0` reflects the chosen shell
    (e.g. `bash`, `zsh`), so its diagnostics correctly identify it.

The basename avoids the MSYS2 path-translation surprise that the full
shell path can trigger on Windows, where Cygwin/MSYS2 programs rewrite
absolute Windows paths handed to them via `lpCommandLine`. Filenames
without path separators do not trigger that rewrite.

The tests added in the preceding commit now pass under this
implementation, including the construction-test assertions whose
`SH_BASENAME`-derived expected value matches `sh.exe` on Windows and
the bash-specific assertion whose expected value is `"bash"`.

Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant