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

git login shell #1752

Merged
merged 5 commits into from
Jan 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ jobs:

strategy:
matrix:
target: [ wasm32-unknown-unknown, wasm32-wasi ]
target: [ wasm32-unknown-unknown, wasm32-wasip1 ]

env:
TARGET: ${{ matrix.target }}
Expand Down
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -394,3 +394,4 @@ stable_sort_primitive = "allow" # x1
no_effect_underscore_binding = "allow" # x1
empty_docs = "allow"
too_long_first_doc_paragraph = "allow"
large_stack_arrays = "allow"
2 changes: 1 addition & 1 deletion gitoxide-core/src/repository/attributes/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ pub(crate) mod function {
let entry = cache.at_entry(
path,
Some(is_dir_to_mode(
workdir.map_or(false, |wd| wd.join(gix::path::from_bstr(path)).is_dir())
workdir.is_some_and(|wd| wd.join(gix::path::from_bstr(path)).is_dir())
|| pattern.signature.contains(gix::pathspec::MagicSignature::MUST_BE_DIR),
)),
)?;
Expand Down
4 changes: 2 additions & 2 deletions gitoxide-core/src/repository/clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,10 @@ pub(crate) mod function {
let pathspec_includes_entry = match pathspec.as_mut() {
None => entry
.pathspec_match
.map_or(false, |m| m != gix::dir::entry::PathspecMatch::Excluded),
.is_some_and(|m| m != gix::dir::entry::PathspecMatch::Excluded),
Some(pathspec) => pathspec
.pattern_matching_relative_path(entry.rela_path.as_bstr(), entry.disk_kind.map(|k| k.is_dir()))
.map_or(false, |m| !m.is_excluded()),
.is_some_and(|m| !m.is_excluded()),
};
pruned_entries += usize::from(!pathspec_includes_entry);
if !pathspec_includes_entry && debug {
Expand Down
9 changes: 5 additions & 4 deletions gitoxide-core/src/repository/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub fn list(
}

let meta = section.meta();
if last_meta.map_or(true, |last| last != meta) {
if last_meta != Some(meta) {
write_meta(meta, &mut out)?;
}
last_meta = Some(meta);
Expand All @@ -41,9 +41,10 @@ pub fn list(
for event in matter {
event.write_to(&mut out)?;
}
if it.peek().map_or(false, |(next_section, _)| {
next_section.header().name() != section.header().name()
}) {
if it
.peek()
.is_some_and(|(next_section, _)| next_section.header().name() != section.header().name())
{
writeln!(&mut out)?;
}
}
Expand Down
2 changes: 1 addition & 1 deletion gitoxide-core/src/repository/exclude.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ pub fn query(
let entry = cache.at_entry(
path,
Some(is_dir_to_mode(
workdir.map_or(false, |wd| wd.join(gix::path::from_bstr(path)).is_dir())
workdir.is_some_and(|wd| wd.join(gix::path::from_bstr(path)).is_dir())
|| pattern.signature.contains(gix::pathspec::MagicSignature::MUST_BE_DIR),
)),
)?;
Expand Down
2 changes: 1 addition & 1 deletion gitoxide-core/src/repository/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ pub(crate) mod function {
for fix in &map.fixes {
match fix {
Fix::MappingWithPartialDestinationRemoved { name, spec } => {
if prev_spec.map_or(true, |prev_spec| prev_spec != spec) {
if prev_spec.is_some_and(|prev_spec| prev_spec != spec) {
prev_spec = spec.into();
spec.to_ref().write_to(&mut err)?;
writeln!(err)?;
Expand Down
2 changes: 1 addition & 1 deletion gitoxide-core/src/repository/index/entries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ pub(crate) mod function {
sms_by_path
.iter()
.find_map(|(path, sm)| (path == entry_path).then_some(sm))
.filter(|sm| sm.git_dir_try_old_form().map_or(false, |dot_git| dot_git.exists()))
.filter(|sm| sm.git_dir_try_old_form().is_ok_and(|dot_git| dot_git.exists()))
})
{
let sm_path = gix::path::to_unix_separators_on_windows(sm.path()?);
Expand Down
2 changes: 1 addition & 1 deletion gitoxide-core/src/repository/remote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ mod refs_impl {
for fix in &map.fixes {
match fix {
Fix::MappingWithPartialDestinationRemoved { name, spec } => {
if prev_spec.map_or(true, |prev_spec| prev_spec != spec) {
if prev_spec.is_some_and(|prev_spec| prev_spec != spec) {
prev_spec = spec.into();
spec.to_ref().write_to(&mut err)?;
writeln!(err)?;
Expand Down
2 changes: 1 addition & 1 deletion gitoxide-core/src/repository/revision/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ pub(crate) mod function {
}
}
progress.inc();
if limit.map_or(false, |limit| limit == progress.step()) {
if limit.is_some_and(|limit| limit == progress.step()) {
break;
}
}
Expand Down
2 changes: 1 addition & 1 deletion gix-dir/src/walk/classify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ pub fn path(

let is_dir = if symlinks_to_directories_are_ignored_like_directories
&& ctx.excludes.is_some()
&& kind.map_or(false, |ft| ft == entry::Kind::Symlink)
&& kind == Some(entry::Kind::Symlink)
{
path.metadata().ok().map(|md| is_dir_to_mode(md.is_dir()))
} else {
Expand Down
2 changes: 1 addition & 1 deletion gix-filter/examples/arrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {

match sub_command.as_str() {
"process" => {
let disallow_delay = next_arg.as_deref().map_or(false, |arg| arg == "disallow-delay");
let disallow_delay = next_arg.as_deref() == Some("disallow-delay");
let mut srv = gix_filter::driver::process::Server::handshake(
stdin(),
stdout(),
Expand Down
2 changes: 1 addition & 1 deletion gix-filter/src/driver/process/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ impl Status {

/// Returns true if this is an `abort` status.
pub fn is_abort(&self) -> bool {
self.message().map_or(false, |m| m == "abort")
self.message() == Some("abort")
}

/// Return true if the status is explicitly set to indicated delayed output processing
Expand Down
4 changes: 2 additions & 2 deletions gix-pack/src/multi_index/chunk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ pub mod index_names {
ascii_path.is_ascii(),
"must use ascii bytes for correct size computation"
);
count += (ascii_path.as_bytes().len() + 1/* null byte */) as u64;
count += (ascii_path.len() + 1/* null byte */) as u64;
}

let needed_alignment = CHUNK_ALIGNMENT - (count % CHUNK_ALIGNMENT);
Expand All @@ -89,7 +89,7 @@ pub mod index_names {
let path = path.as_ref().to_str().expect("UTF-8 path");
out.write_all(path.as_bytes())?;
out.write_all(&[0])?;
written_bytes += path.as_bytes().len() as u64 + 1;
written_bytes += path.len() as u64 + 1;
}

let needed_alignment = CHUNK_ALIGNMENT - (written_bytes % CHUNK_ALIGNMENT);
Expand Down
8 changes: 4 additions & 4 deletions gix-packetline-blocking/src/line/async_io.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions gix-packetline/src/line/async_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use futures_io::AsyncWrite;

use crate::{encode, BandRef, Channel, ErrorRef, PacketLineRef, TextRef};

impl<'a> BandRef<'a> {
impl BandRef<'_> {
/// Serialize this instance to `out`, returning the amount of bytes written.
///
/// The data written to `out` can be decoded with [`Borrowed::decode_band()]`.
Expand All @@ -18,14 +18,14 @@ impl<'a> BandRef<'a> {
}
}

impl<'a> TextRef<'a> {
impl TextRef<'_> {
/// Serialize this instance to `out`, appending a newline if there is none, returning the amount of bytes written.
pub async fn write_to(&self, out: impl AsyncWrite + Unpin) -> io::Result<usize> {
encode::text_to_write(self.0, out).await
}
}

impl<'a> ErrorRef<'a> {
impl ErrorRef<'_> {
/// Serialize this line as error to `out`.
///
/// This includes a marker to allow decoding it outside of a side-band channel, returning the amount of bytes written.
Expand All @@ -34,7 +34,7 @@ impl<'a> ErrorRef<'a> {
}
}

impl<'a> PacketLineRef<'a> {
impl PacketLineRef<'_> {
/// Serialize this instance to `out` in git `packetline` format, returning the amount of bytes written to `out`.
pub async fn write_to(&self, out: impl AsyncWrite + Unpin) -> io::Result<usize> {
match self {
Expand Down
17 changes: 17 additions & 0 deletions gix-path/src/env/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,23 @@ pub fn installation_config_prefix() -> Option<&'static Path> {
installation_config().map(git::config_to_base_path)
}

/// Return the shell that Git would prefer as login shell, the shell to execute Git commands from.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I know, only Git for Windows has such a preference, in that its Git Bash environment is intended to be a good environment for running git commands from. Even there, the preference is mild, in that the Git for Windows installer defaults to making the git command available in the user's PATH even outside Git Bash. To the best of my knowledge, no such preference exists on the part of the upstream Git project.

This doesn't necessarily mean that a login_shell function is not useful. But it is not clear from this description what it should be used for. (Relatedly, some of the existing and planned uses seem to be things it should not be used for.)

///
/// On Windows, this is the `bash.exe` bundled with it, and on Unix it's the shell specified by `SHELL`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The antecedent of "it" may be ambiguous for readers who are less familiar with Git for Windows. If the function is kept, I suggest adjusting this to:

Suggested change
/// On Windows, this is the `bash.exe` bundled with it, and on Unix it's the shell specified by `SHELL`,
/// On Windows, this is the `bash.exe` bundled with Git, and on Unix it's the shell specified by `SHELL`,

/// or `None` if it is truly unspecified.
pub fn login_shell() -> Option<&'static Path> {
static PATH: Lazy<Option<PathBuf>> = Lazy::new(|| {
if cfg!(windows) {
installation_config_prefix()
.and_then(|p| p.parent())
.map(|p| p.join("usr").join("bin").join("bash.exe"))
Byron marked this conversation as resolved.
Show resolved Hide resolved
} else {
std::env::var_os("SHELL").map(PathBuf::from)
Byron marked this conversation as resolved.
Show resolved Hide resolved
}
});
PATH.as_deref()
}

/// Return the name of the Git executable to invoke it.
/// If it's in the `PATH`, it will always be a short name.
///
Expand Down
80 changes: 0 additions & 80 deletions gix-path/tests/path.rs

This file was deleted.

File renamed without changes.
71 changes: 71 additions & 0 deletions gix-path/tests/path/env.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
#[test]
fn exe_invocation() {
let actual = gix_path::env::exe_invocation();
assert!(
!actual.as_os_str().is_empty(),
"it finds something as long as git is installed somewhere on the system (or a default location)"
);
}

#[test]
fn login_shell() {
// On CI, the $SHELL variable isn't necessarily set. Maybe other ways to get the login shell should be used then.
if !gix_testtools::is_ci::cached() {
assert!(gix_path::env::login_shell()
.expect("There should always be the notion of a shell used by git")
Byron marked this conversation as resolved.
Show resolved Hide resolved
.exists());
}
}

#[test]
fn installation_config() {
assert_ne!(
gix_path::env::installation_config().map(|p| p.components().count()),
gix_path::env::installation_config_prefix().map(|p| p.components().count()),
"the prefix is a bit shorter than the installation config path itself"
);
}

#[test]
fn system_prefix() {
assert_ne!(
gix_path::env::system_prefix(),
None,
"git should be present when running tests"
);
}

#[test]
fn home_dir() {
assert_ne!(
gix_path::env::home_dir(),
None,
"we find a home on every system these tests execute"
);
}

mod xdg_config {
use std::ffi::OsStr;

#[test]
fn prefers_xdg_config_bases() {
let actual = gix_path::env::xdg_config("test", &mut |n| {
(n == OsStr::new("XDG_CONFIG_HOME")).then(|| "marker".into())
})
.expect("set");
#[cfg(unix)]
assert_eq!(actual.to_str(), Some("marker/git/test"));
#[cfg(windows)]
assert_eq!(actual.to_str(), Some("marker\\git\\test"));
}

#[test]
fn falls_back_to_home() {
let actual = gix_path::env::xdg_config("test", &mut |n| (n == OsStr::new("HOME")).then(|| "marker".into()))
.expect("set");
#[cfg(unix)]
assert_eq!(actual.to_str(), Some("marker/.config/git/test"));
#[cfg(windows)]
assert_eq!(actual.to_str(), Some("marker\\.config\\git\\test"));
}
}
Loading
Loading