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

Implement FR #2338 for current branches / automatically advancing branches #3129

Merged
merged 3 commits into from
Apr 20, 2024
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
128 changes: 127 additions & 1 deletion cli/src/cli_util.rs
emesterhazy marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ use jj_lib::matchers::Matcher;
use jj_lib::merge::MergedTreeValue;
use jj_lib::merged_tree::MergedTree;
use jj_lib::object_id::ObjectId;
use jj_lib::op_store::{OpStoreError, OperationId, WorkspaceId};
use jj_lib::op_store::{OpStoreError, OperationId, RefTarget, WorkspaceId};
use jj_lib::op_walk::OpsetEvaluationError;
use jj_lib::operation::Operation;
use jj_lib::repo::{
Expand Down Expand Up @@ -393,6 +393,77 @@ impl ReadonlyUserRepo {
}
}

/// A branch that should be advanced to satisfy the "advance-branches" feature.
/// This is a helper for `WorkspaceCommandTransaction`. It provides a type-safe
/// way to separate the work of checking whether a branch can be advanced and
/// actually advancing it. Advancing the branch never fails, but can't be done
/// until the new `CommitId` is available. Splitting the work in this way also
/// allows us to identify eligible branches without actually moving them and
/// return config errors to the user early.
pub struct AdvanceableBranch {
name: String,
old_commit_id: CommitId,
}

/// Helper for parsing and evaluating settings for the advance-branches feature.
/// Settings are configured in the jj config.toml as lists of [`StringPattern`]s
/// for enabled and disabled branches. Example:
/// ```toml
/// [experimental-advance-branches]
/// # Enable the feature for all branches except "main".
/// enabled-branches = ["glob:*"]
/// disabled-branches = ["main"]
/// ```
struct AdvanceBranchesSettings {
enabled_branches: Vec<StringPattern>,
disabled_branches: Vec<StringPattern>,
}

impl AdvanceBranchesSettings {
fn from_config(config: &config::Config) -> Result<Self, CommandError> {
let get_setting = |setting_key| {
let setting = format!("experimental-advance-branches.{setting_key}");
match config.get::<Vec<String>>(&setting).optional()? {
Some(patterns) => patterns
.into_iter()
.map(|s| {
StringPattern::parse(&s).map_err(|e| {
config_error_with_message(
format!("Error parsing '{s}' for {setting}"),
e,
)
})
})
.collect(),
None => Ok(Vec::new()),
}
};
Ok(Self {
enabled_branches: get_setting("enabled-branches")?,
disabled_branches: get_setting("disabled-branches")?,
})
}

/// Returns true if the advance-branches feature is enabled for
/// `branch_name`.
fn branch_is_eligible(&self, branch_name: &str) -> bool {
if self
.disabled_branches
.iter()
.any(|d| d.matches(branch_name))
{
return false;
}
self.enabled_branches.iter().any(|e| e.matches(branch_name))
}

/// Returns true if the config includes at least one "enabled-branches"
/// pattern.
fn feature_enabled(&self) -> bool {
!self.enabled_branches.is_empty()
}
}

/// Provides utilities for writing a command that works on a [`Workspace`]
/// (which most commands do).
pub struct WorkspaceCommandHelper {
Expand Down Expand Up @@ -1375,6 +1446,44 @@ Then run `jj squash` to move the resolution into the conflicted commit."#,

Ok(())
}

/// Identifies branches which are eligible to be moved automatically during
/// `jj commit` and `jj new`. Whether a branch is eligible is determined by
/// its target and the user and repo config for "advance-branches".
///
/// Returns a Vec of branches in `repo` that point to any of the `from`
/// commits and that are eligible to advance. The `from` commits are
/// typically the parents of the target commit of `jj commit` or `jj new`.
///
/// Branches are not moved until
/// `WorkspaceCommandTransaction::advance_branches()` is called with the
/// `AdvanceableBranch`s returned by this function.
///
/// Returns an empty `std::Vec` if no branches are eligible to advance.
pub fn get_advanceable_branches<'a>(
&self,
from: impl IntoIterator<Item = &'a CommitId>,
) -> Result<Vec<AdvanceableBranch>, CommandError> {
let ab_settings = AdvanceBranchesSettings::from_config(self.settings.config())?;
if !ab_settings.feature_enabled() {
// Return early if we know that there's no work to do.
return Ok(Vec::new());
}

let mut advanceable_branches = Vec::new();
for from_commit in from {
for (name, _) in self.repo().view().local_branches_for_commit(from_commit) {
if ab_settings.branch_is_eligible(name) {
advanceable_branches.push(AdvanceableBranch {
name: name.to_owned(),
old_commit_id: from_commit.clone(),
});
}
}
}

Ok(advanceable_branches)
}
}

/// A [`Transaction`] tied to a particular workspace.
Expand Down Expand Up @@ -1461,6 +1570,23 @@ impl WorkspaceCommandTransaction<'_> {
pub fn into_inner(self) -> Transaction {
self.tx
}

/// Moves each branch in `branches` from an old commit it's associated with
/// (configured by `get_advanceable_branches`) to the `move_to` commit. If
/// the branch is conflicted before the update, it will remain conflicted
/// after the update, but the conflict will involve the `move_to` commit
/// instead of the old commit.
pub fn advance_branches(&mut self, branches: Vec<AdvanceableBranch>, move_to: &CommitId) {
for branch in branches {
// This removes the old commit ID from the branch's RefTarget and
// replaces it with the `move_to` ID.
self.mut_repo().merge_local_branch(
&branch.name,
&RefTarget::normal(branch.old_commit_id),
&RefTarget::normal(move_to.clone()),
);
}
}
}

fn find_workspace_dir(cwd: &Path) -> &Path {
Expand Down
5 changes: 5 additions & 0 deletions cli/src/commands/commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ pub(crate) fn cmd_commit(
let matcher = workspace_command
.parse_file_patterns(&args.paths)?
.to_matcher();
let advanceable_branches = workspace_command.get_advanceable_branches(commit.parent_ids())?;
let diff_selector =
workspace_command.diff_selector(ui, args.tool.as_deref(), args.interactive)?;
let mut tx = workspace_command.start_transaction();
Expand Down Expand Up @@ -121,6 +122,10 @@ new working-copy commit.
commit.tree_id().clone(),
)
.write()?;

// Does nothing if there's no branches to advance.
tx.advance_branches(advanceable_branches, new_commit.id());

for workspace_id in workspace_ids {
tx.mut_repo().edit(workspace_id, &new_wc_commit).unwrap();
}
Expand Down
18 changes: 18 additions & 0 deletions cli/src/commands/new.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,18 @@ Please use `jj new 'all:x|y'` instead of `jj new --allow-large-revsets x y`.",
.into_iter()
.collect_vec();
let target_ids = target_commits.iter().ids().cloned().collect_vec();

let should_advance_branches =
target_commits.len() == 1 && !args.insert_before && !args.insert_after;
let (advance_branches_target, advanceable_branches) = if should_advance_branches {
let ab_target = target_ids[0].clone();
let ab_branches =
workspace_command.get_advanceable_branches(target_commits[0].parent_ids())?;
(Some(ab_target), ab_branches)
} else {
(None, Vec::new())
};

let mut tx = workspace_command.start_transaction();
let mut num_rebased;
let new_commit;
Expand Down Expand Up @@ -197,6 +209,12 @@ Please use `jj new 'all:x|y'` instead of `jj new --allow-large-revsets x y`.",
if num_rebased > 0 {
writeln!(ui.status(), "Rebased {num_rebased} descendant commits")?;
}

// Does nothing if there's no branches to advance.
if let Some(target) = advance_branches_target {
tx.advance_branches(advanceable_branches, &target);
}

tx.finish(ui, "new empty commit")?;
Ok(())
}
20 changes: 20 additions & 0 deletions cli/src/config-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,26 @@
}
}
},
"experimental-advance-branches": {
"type": "object",
"description": "Settings controlling the 'advance-branches' feature which moves branches forward when new commits are created.",
"properties": {
"enabled-branches": {
"type": "array",
"description": "Patterns used to identify branches which may be advanced.",
"items": {
"type": "string"
}
},
"disabled-branches": {
"type": "array",
"description": "Patterns used to identify branches which are not advanced. Takes precedence over 'enabled-branches'.",
"items": {
"type": "string"
}
}
}
},
"signing": {
"type": "object",
"description": "Settings for verifying and creating cryptographic commit signatures",
Expand Down
1 change: 1 addition & 0 deletions cli/tests/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ fn test_no_forgotten_test_files() {
}

mod test_abandon_command;
mod test_advance_branches;
mod test_alias;
mod test_branch_command;
mod test_builtin_aliases;
Expand Down
Loading