Skip to content

Commit 2800015

Browse files
codexByron
andcommitted
Add but-core git-config utility to quickly read-change-write a Git configuration file.
Use it where possible. Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
1 parent a263db9 commit 2800015

23 files changed

Lines changed: 316 additions & 202 deletions

File tree

AGENTS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,5 @@
33
When asked about general development workflow, project structure, building, testing, or contributing to GitButler, please read `@.github/copilot-instructions.md` for context.
44

55
When working on the `but` CLI (Rust command-line tool in `crates/but`), please read `@crates/but/agents.md` for context on API usage patterns, output formatting, and testing conventions.
6+
7+
When tests need to observe repository changes written to disk, prefer `repo.reload()` over reopening the repository.

Cargo.lock

Lines changed: 0 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/but-api/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@ legacy = [
7575
but-serde.workspace = true
7676
but-path.workspace = true
7777
but-api-macros.workspace = true
78-
but-oxidize.workspace = true
7978
but-error.workspace = true
8079
but-core.workspace = true
8180
but-graph.workspace = true

crates/but-api/src/legacy/git.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use anyhow::{Context as _, Result};
33
use bstr::ByteSlice;
44
use but_api_macros::but_api;
55
use but_core::git_config::{
6-
open_user_global_config_for_editing, remove_config_value, set_config_value, write_config,
6+
edit_config, open_global_config_for_reading, remove_config_value, set_config_value,
77
};
88
use gitbutler_reference::RemoteRefname;
99
use gitbutler_repo_actions::RepoActionsExt as _;
@@ -80,25 +80,27 @@ pub fn delete_all_data() -> Result<()> {
8080
#[but_api]
8181
#[instrument(err(Debug))]
8282
pub fn git_set_global_config(key: String, value: String) -> Result<String> {
83-
let (mut config, path) = open_user_global_config_for_editing()?;
84-
set_config_value(&mut config, &key, &value)?;
85-
write_config(&path, &config)?;
83+
_ = edit_config(None, gix::config::Source::User, |config| {
84+
set_config_value(config, &key, &value)?;
85+
Ok(())
86+
})?;
8687
Ok(value)
8788
}
8889

8990
#[but_api]
9091
#[instrument(err(Debug))]
9192
pub fn git_remove_global_config(key: String) -> Result<()> {
92-
let (mut config, path) = open_user_global_config_for_editing()?;
93-
remove_config_value(&mut config, &key)?;
94-
write_config(&path, &config)?;
93+
_ = edit_config(None, gix::config::Source::User, |config| {
94+
remove_config_value(config, &key)?;
95+
Ok(())
96+
})?;
9597
Ok(())
9698
}
9799

98100
#[but_api]
99101
#[instrument(err(Debug))]
100102
pub fn git_get_global_config(key: String) -> Result<Option<String>> {
101-
let (config, _) = open_user_global_config_for_editing()?;
103+
let config = open_global_config_for_reading()?;
102104
Ok(get_config_string(&config, &key))
103105
}
104106

crates/but-core/src/git_config.rs

Lines changed: 98 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,54 +1,120 @@
11
//! Utilities for mutating Git configuration entries by dotted key.
22
3-
use std::path::{Path, PathBuf};
3+
use std::path::PathBuf;
44

5-
use anyhow::{Context as _, Result};
5+
use anyhow::{Context as _, Result, bail};
66
use bstr::ByteSlice as _;
77
use gix::config::AsKey as _;
88

9-
/// Open the user-global Git config for editing, creating it first if needed.
10-
pub fn open_user_global_config_for_editing() -> Result<(gix::config::File<'static>, PathBuf)> {
11-
let path = gix::config::Source::User
9+
fn config_path(repo: Option<&gix::Repository>, source: gix::config::Source) -> Result<PathBuf> {
10+
let path = source
1211
.storage_location(&mut |name| std::env::var_os(name))
13-
.context("failed to determine global git config location")?
14-
.into_owned();
12+
.with_context(|| format!("failed to determine {source:?} git config location"))?;
13+
let path = if path.is_relative() {
14+
let repo = repo.with_context(|| {
15+
format!("determining the {source:?} git config location requires a repository")
16+
})?;
17+
if source == gix::config::Source::Local {
18+
repo.common_dir().join(path.as_ref())
19+
} else {
20+
repo.git_dir().join(path.as_ref())
21+
}
22+
} else {
23+
path.into_owned()
24+
};
25+
Ok(path)
26+
}
27+
28+
/// Open the Git config for `source` for editing, creating it first if needed.
29+
/// `repo` is used to resolve repo-local paths, depending on `source`.
30+
/// Return `(config, lock)`.
31+
/// Write it back with [`write_locked_config()`].
32+
pub fn open_config_for_editing(
33+
repo: Option<&gix::Repository>,
34+
source: gix::config::Source,
35+
) -> Result<(gix::config::File<'static>, gix::lock::File)> {
36+
let path = config_path(repo, source)?;
37+
std::fs::create_dir_all(path.parent().context("git config path has no parent")?)?;
38+
let lock = gix::lock::File::acquire_to_update_resource(
39+
&path,
40+
gix::lock::acquire::Fail::Immediately,
41+
None,
42+
)?;
1543
if !path.exists() {
16-
std::fs::create_dir_all(
17-
path.parent()
18-
.context("global git config path has no parent")?,
19-
)?;
2044
std::fs::File::create(&path)?;
2145
}
22-
let config = gix::config::File::from_path_no_includes(path.clone(), gix::config::Source::User)
23-
.with_context(|| format!("failed to open global git config at {}", path.display()))?;
24-
Ok((config, path))
46+
let config = gix::config::File::from_path_no_includes(path.clone(), source)
47+
.with_context(|| format!("failed to open {source:?} git config at {}", path.display()))?;
48+
Ok((config, lock))
49+
}
50+
51+
/// Open the user-global Git config for reading without acquiring a write lock.
52+
///
53+
/// If the config file doesn't exist yet, an empty in-memory config is returned.
54+
pub fn open_global_config_for_reading() -> Result<gix::config::File<'static>> {
55+
let path = config_path(None, gix::config::Source::User)?;
56+
if !path.exists() {
57+
return Ok(gix::config::File::new(gix::config::file::Metadata::from(
58+
gix::config::Source::User,
59+
)));
60+
}
61+
gix::config::File::from_path_no_includes(path.clone(), gix::config::Source::User)
62+
.with_context(|| format!("failed to open User git config at {}", path.display()))
2563
}
2664

27-
/// Serialize a Git `config` file back to disk at `path`.
28-
pub fn write_config(path: &Path, config: &gix::config::File<'_>) -> Result<()> {
29-
let mut file = std::io::BufWriter::new(
30-
std::fs::OpenOptions::new()
31-
.write(true)
32-
.truncate(true)
33-
.create(true)
34-
.open(path)
35-
.with_context(|| {
36-
format!(
37-
"failed to open git config for writing at {}",
38-
path.display()
39-
)
40-
})?,
41-
);
65+
/// Serialize a Git `config` file back to disk at `lock`.
66+
pub fn write_locked_config(
67+
config: &gix::config::File<'_>,
68+
mut lock: gix::lock::File,
69+
) -> Result<()> {
70+
let path = lock.resource_path();
4271
config
43-
.write_to(&mut file)
72+
.write_to(&mut lock)
4473
.with_context(|| format!("failed to serialize git config at {}", path.display()))?;
45-
std::io::Write::flush(&mut file)
74+
std::io::Write::flush(&mut lock)
4675
.with_context(|| format!("failed to flush git config at {}", path.display()))?;
76+
lock.commit()
77+
.map_err(|err| err.error)
78+
.with_context(|| format!("failed to commit git config at {}", path.display()))?;
4779
Ok(())
4880
}
4981

82+
/// Open the Git config for `source` using `repo` when needed, let `edit` mutate it, and
83+
/// write it back if the edited configuration differs from its original state.
84+
/// Return `true` if the file changed and was written, `false` otherwise.
85+
pub fn edit_config(
86+
repo: Option<&gix::Repository>,
87+
source: gix::config::Source,
88+
edit: impl FnOnce(&mut gix::config::File<'static>) -> Result<()>,
89+
) -> Result<bool> {
90+
let (mut config, lock) = open_config_for_editing(repo, source)?;
91+
let previous_contents = config.to_bstring();
92+
edit(&mut config)?;
93+
let changed = config.to_bstring() != previous_contents;
94+
if changed {
95+
write_locked_config(&config, lock)?;
96+
}
97+
Ok(changed)
98+
}
99+
100+
/// Open the Git config for `source` relative to `repo`, let `edit` mutate it, and write it back
101+
/// if the edited configuration differs from its original state.
102+
pub fn edit_repo_config(
103+
repo: &gix::Repository,
104+
source: gix::config::Source,
105+
edit: impl FnOnce(&mut gix::config::File<'static>) -> Result<()>,
106+
) -> Result<bool> {
107+
if matches!(
108+
source,
109+
gix::config::Source::System | gix::config::Source::GitInstallation
110+
) {
111+
bail!("editing {source:?} config through a repository is not supported");
112+
}
113+
edit_config(Some(repo), source, edit)
114+
}
115+
50116
/// Set the entry in `config` identified by the dotted `key` (like `section.value` or `section.subsection.value`) to `value`.
51-
/// This will create sections as needed.
117+
/// This will create sections as needed, and remove all previous values under the same section with the same name.
52118
pub fn set_config_value(
53119
config: &mut gix::config::File<'static>,
54120
key: &str,

crates/but-core/src/repo_ext.rs

Lines changed: 29 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -74,12 +74,20 @@ pub trait RepositoryExt: Sized {
7474
/// Return all signatures that would be needed to perform a commit as configured in Git: `(author, committer)`.
7575
fn commit_signatures(&self) -> anyhow::Result<(gix::actor::Signature, gix::actor::Signature)>;
7676

77-
/// Return the configuration freshly loaded from `.git/config` so that it can be changed in memory,
78-
/// and possibly written back with [`Self::write_local_common_config()`].
79-
fn local_common_config_for_editing(&self) -> anyhow::Result<gix::config::File<'static>>;
80-
/// Write the given `local_config` to `.git/config` of the common repository.
81-
/// Note that we never write linked worktree-local configuration.
82-
fn write_local_common_config(&self, local_config: &gix::config::File) -> anyhow::Result<()>;
77+
/// Return the configuration freshly loaded from `.git/config` together with an acquired lock
78+
/// for that file so it can be changed in memory and safely written back without another writer
79+
/// racing the read-modify-write cycle.
80+
fn local_common_config_for_editing(
81+
&self,
82+
) -> anyhow::Result<(gix::config::File<'static>, gix::lock::File)>;
83+
/// Write the given `local_config` to the file at `lock` of the while consuming
84+
/// the lock previously acquired with [`Self::local_common_config_for_editing()`].
85+
/// Note that only local configuraiton is written, so it's safe to use it with `repo.config_snapshot_mut()`.
86+
fn write_locked_config(
87+
&self,
88+
local_config: &gix::config::File,
89+
lock: gix::lock::File,
90+
) -> anyhow::Result<()>;
8391
/// Cherry-pick the changes in the tree of `to_rebase_commit_id` onto `new_base_commit_id`.
8492
/// This method deals with the presence of conflicting commits to select the correct trees
8593
/// for the cheery-pick merge.
@@ -184,37 +192,28 @@ impl RepositoryExt for gix::Repository {
184192
Ok((author.into(), committer))
185193
}
186194

187-
fn local_common_config_for_editing(&self) -> anyhow::Result<gix::config::File<'static>> {
195+
fn local_common_config_for_editing(
196+
&self,
197+
) -> anyhow::Result<(gix::config::File<'static>, gix::lock::File)> {
188198
let local_config_path = self.common_dir().join("config");
199+
let lock = gix::lock::File::acquire_to_update_resource(
200+
&local_config_path,
201+
gix::lock::acquire::Fail::Immediately,
202+
None,
203+
)?;
189204
let config = gix::config::File::from_path_no_includes(
190205
local_config_path.clone(),
191206
gix::config::Source::Local,
192207
)?;
193-
Ok(config)
208+
Ok((config, lock))
194209
}
195210

196-
fn write_local_common_config(&self, local_config: &gix::config::File) -> anyhow::Result<()> {
197-
use std::io::Write;
198-
// Note: we don't use a lock file here to not risk changing the mode, and it's what Git does.
199-
// But we lock the file so there is no raciness.
200-
let local_config_path = self.common_dir().join("config");
201-
let _lock = gix::lock::Marker::acquire_to_hold_resource(
202-
&local_config_path,
203-
gix::lock::acquire::Fail::Immediately,
204-
None,
205-
)?;
206-
let mut config_file = std::io::BufWriter::new(
207-
std::fs::File::options()
208-
.write(true)
209-
.truncate(true)
210-
.create(false)
211-
.open(local_config_path)?,
212-
);
213-
local_config.write_to_filter(&mut config_file, |section| {
214-
section.meta().source.kind() == gix::config::source::Kind::Repository
215-
})?;
216-
config_file.flush()?;
217-
Ok(())
211+
fn write_locked_config(
212+
&self,
213+
local_config: &gix::config::File,
214+
lock: gix::lock::File,
215+
) -> anyhow::Result<()> {
216+
crate::git_config::write_locked_config(local_config, lock)
218217
}
219218

220219
fn cherry_pick_commits_to_tree(

0 commit comments

Comments
 (0)