diff --git a/contrib/repro-push-reparent-drops-work.sh b/contrib/repro-push-reparent-drops-work.sh new file mode 100755 index 00000000..c14b3de6 --- /dev/null +++ b/contrib/repro-push-reparent-drops-work.sh @@ -0,0 +1,127 @@ +#!/usr/bin/env bash +# +# Reproduction: a push carrying more than one local revision into a branch head +# that has moved silently drops the other side's work. +# +# LORE=/path/to/lore LORE_SERVER=lore://127.0.0.1:41337 ./repro-push-reparent-drops-work.sh +# +# Needs a reachable loreserver with no auth and a lore CLI. Creates its own +# repository on that server and three working copies under a temp directory. +# +# What it does: +# A and B both clone the same head. +# A commits one file and pushes. -> the head moves +# B, still on the old head, commits TWO files and +# pushes once with --fast-forward-merge. -> both land +# A third clone then shows what the server really holds. +# +# Expected (correct): the third clone has A's file and both of B's. +# Observed (before the fix): A's file is gone, while A's commit is still in +# the history, so nothing looks wrong to anyone reading it. +# +# Why: in lore-revision/src/branch/push.rs the client bends a revision's +# parent pointer onto whatever the previous push returned, keeping its tree: +# +# if !current_latest.is_zero() && state.parent_self() != current_latest { +# state.set_parent_self(current_latest); +# } +# +# That is right when the server merely renumbered the revision we just sent +# (same content under a new signature). After a server-side fast-forward merge +# `current_latest` is a different revision carrying somebody else's work, and +# re-pointing at it makes B's stale tree look like a direct descendant. The +# server then takes the ordinary fast-forward path — no merge, no three-way +# diff, no conflict check — and the branch becomes B's tree. +# +# The guard `!current_latest.is_zero()` is why a single-revision push is safe: +# the block cannot fire for the first revision of a push, only from the second. + +set -euo pipefail + +LORE=${LORE:-lore} +: "${LORE_SERVER:?set LORE_SERVER, e.g. lore://127.0.0.1:41337}" +# merge - integrate with --fast-forward-merge (the shape that ships today) +# rebase - integrate with --rebase (linear history) +MODE=${MODE:-merge} +case "$MODE" in merge|rebase) ;; *) echo "MODE must be merge or rebase"; exit 2 ;; esac + +WORK=$(mktemp -d) +trap 'rm -rf "$WORK"' EXIT +REPO="${LORE_SERVER%/}/reparent-repro-$$-$(date +%s)" + +lore_in() { local wc=$1; shift; "$LORE" --repository "$wc" --no-pager "$@" + local wc=$1 file=$2 content=$3 msg=$4 + printf '%s\n' "$content" > "$wc/$file" + lore_in "$wc" stage "$wc/$file" >/dev/null + lore_in "$wc" commit "$msg" >/dev/null +} + +echo "repository: $REPO" +echo + +# ── seed ──────────────────────────────────────────────────────────────────── +"$LORE" --no-pager repository create "$REPO" --repository "$WORK/seed" /dev/null +add_commit "$WORK/seed" keep.txt seed "seed" +lore_in "$WORK/seed" push >/dev/null + +# ── two clones off the same head ──────────────────────────────────────────── +"$LORE" --no-pager clone "$REPO" "$WORK/a" /dev/null +"$LORE" --no-pager clone "$REPO" "$WORK/b" /dev/null + +# ── A publishes, moving the head ──────────────────────────────────────────── +add_commit "$WORK/a" from-a.txt "A only" "A publishes" +lore_in "$WORK/a" push >/dev/null +echo "A pushed from-a.txt" + +# ── B, still on the old head, commits TWICE and pushes once ───────────────── +add_commit "$WORK/b" from-b1.txt "B one" "B one" +add_commit "$WORK/b" from-b2.txt "B two" "B two" +if [ "$MODE" = rebase ]; then + if ! lore_in "$WORK/b" push --rebase; then + echo + echo "RESULT: REFUSED — the server did not integrate the push." + echo " On a server that predates --rebase this is the expected answer:" + echo " it ignores the unknown field, sees no integration opt-in and" + echo " soft-rejects, rather than silently merging instead." + "$LORE" --no-pager clone "$REPO" "$WORK/c" /dev/null + [ -f "$WORK/c/from-a.txt" ] && echo " A's file is untouched on the server." || echo " UNEXPECTED: A's file is gone anyway." + exit 3 + fi + echo "B pushed two revisions with --rebase" +else + lore_in "$WORK/b" push --fast-forward-merge >/dev/null + echo "B pushed two revisions with --fast-forward-merge" +fi +echo + +# ── what does the server actually hold? ───────────────────────────────────── +"$LORE" --no-pager clone "$REPO" "$WORK/c" /dev/null +echo "files in a fresh clone:" +find "$WORK/c" -maxdepth 1 -mindepth 1 -exec basename {} \; | sort | sed 's/^/ /' +echo +echo "history (A's commit is still in it either way):" +lore_in "$WORK/c" history 6 | grep -E '^(Revision|Signature| )' | sed 's/^/ /' +echo + +rc=0 +for f in from-b1.txt from-b2.txt; do + [ -f "$WORK/c/$f" ] || { echo "UNEXPECTED: B's own $f is missing too"; rc=1; } +done +if [ "$MODE" = rebase ]; then + if lore_in "$WORK/c" history 6 | grep -q '^Merge '; then + echo "RESULT: NOT LINEAR — a rebase must not leave a merge revision" + rc=1 + else + echo "(history is linear: no merge revision)" + fi +fi + +if [ -f "$WORK/c/from-a.txt" ]; then + echo "RESULT: OK — A's file survived B's push" +else + echo "RESULT: LOST — A's file is gone from the branch, with no error reported" + rc=1 +fi +exit $rc diff --git a/lore-capi/lore.h b/lore-capi/lore.h index cee485c6..a33a60c4 100644 --- a/lore-capi/lore.h +++ b/lore-capi/lore.h @@ -4291,6 +4291,9 @@ typedef struct lore_branch_push_args_t { struct lore_string_t branch; // Allow the server to fast-forward merge if the target branch head has moved uint8_t fast_forward_merge; + // Allow the server to rebase onto the target branch head if it has moved, + // keeping the branch linear. Mutually exclusive with `fast_forward_merge`. + uint8_t rebase; } lore_branch_push_args_t; // Arguments for retrieving branch metadata (one key or all). diff --git a/lore-client/src/cli/commands/branch.rs b/lore-client/src/cli/commands/branch.rs index 267a95db..85456908 100644 --- a/lore-client/src/cli/commands/branch.rs +++ b/lore-client/src/cli/commands/branch.rs @@ -88,6 +88,12 @@ pub struct BranchPushArgs { /// Allow the server to fast-forward merge if the target branch head has moved #[clap(long)] fast_forward_merge: bool, + + /// Allow the server to rebase onto the target branch head if it has moved, + /// keeping the branch linear instead of recording a merge. A server that + /// does not support this refuses the push rather than merging instead. + #[clap(long, conflicts_with = "fast_forward_merge")] + rebase: bool, } #[derive(Args)] @@ -818,6 +824,7 @@ pub fn handle_branch_push(globals: LoreGlobalArgs, args: &BranchPushArgs) -> u8 let push_args = LoreBranchPushArgs { branch: args.name.clone().into(), fast_forward_merge: args.fast_forward_merge.into(), + rebase: args.rebase.into(), }; let debug = progress_debug(); diff --git a/lore-proto/proto/lore/revision/v1/revision.proto b/lore-proto/proto/lore/revision/v1/revision.proto index 689b6e78..58f7d7df 100644 --- a/lore-proto/proto/lore/revision/v1/revision.proto +++ b/lore-proto/proto/lore/revision/v1/revision.proto @@ -130,6 +130,20 @@ message BranchPushRequest { // the server attempts a fast-forward merge. Mutually superseded by // `force`. bool fast_forward_merge = 4; + // When true and the new tip does not descend from the current tip, + // the server rebases it: the same three-way diff `fast_forward_merge` + // performs, but the result records only the current tip as its parent, + // so the branch stays linear and the pushed revision is not retained + // as a second parent. Mutually superseded by `force`; setting it + // together with `fast_forward_merge` is INVALID_ARGUMENT, because the + // two ask for different histories and silently picking one would hide + // which was applied. + // + // A server that predates this field ignores it and sees a push with + // no integration opt-in, so it soft-rejects rather than merging. That + // is deliberate: a client asking to rebase must never be answered + // with a merge it did not ask for. + bool rebase = 5; } // Response describing the resulting branch tip after a push. diff --git a/lore-proto/src/grpc/lore.revision.v1.rs b/lore-proto/src/grpc/lore.revision.v1.rs index 9eafeb6f..72f7dbfc 100644 --- a/lore-proto/src/grpc/lore.revision.v1.rs +++ b/lore-proto/src/grpc/lore.revision.v1.rs @@ -204,6 +204,21 @@ pub struct BranchPushRequest { /// `force`. #[prost(bool, tag = "4")] pub fast_forward_merge: bool, + /// When true and the new tip does not descend from the current tip, + /// the server rebases it: the same three-way diff `fast_forward_merge` + /// performs, but the result records only the current tip as its parent, + /// so the branch stays linear and the pushed revision is not retained + /// as a second parent. Mutually superseded by `force`; setting it + /// together with `fast_forward_merge` is INVALID_ARGUMENT, because the + /// two ask for different histories and silently picking one would hide + /// which was applied. + /// + /// A server that predates this field ignores it and sees a push with + /// no integration opt-in, so it soft-rejects rather than merging. That + /// is deliberate: a client asking to rebase must never be answered + /// with a merge it did not ask for. + #[prost(bool, tag = "5")] + pub rebase: bool, } impl ::prost::Name for BranchPushRequest { const NAME: &'static str = "BranchPushRequest"; diff --git a/lore-proto/tests/v1_revision.rs b/lore-proto/tests/v1_revision.rs index e195eab3..7dc08961 100644 --- a/lore-proto/tests/v1_revision.rs +++ b/lore-proto/tests/v1_revision.rs @@ -76,6 +76,7 @@ fn v1_revision_field_shapes() { revision_signature: _, force: _, fast_forward_merge: _, + rebase: _, } = BranchPushRequest::default(); let BranchPushResponse { revision_signature: _, diff --git a/lore-revision/src/branch/merge.rs b/lore-revision/src/branch/merge.rs index 704822db..ee295fa6 100644 --- a/lore-revision/src/branch/merge.rs +++ b/lore-revision/src/branch/merge.rs @@ -4225,7 +4225,7 @@ async fn merge_into_link( .forward::("pushing fragments")?; let response = revision_protocol - .branch_push(target_branch, signature, false, false) + .branch_push(target_branch, signature, false, false, false) .await .forward::("pushing branch")?; @@ -4616,7 +4616,7 @@ pub async fn merge_into( .send(); let response = revision_protocol - .branch_push(branch, signature, false, false) + .branch_push(branch, signature, false, false, false) .await .forward::("pushing branch")?; diff --git a/lore-revision/src/branch/push.rs b/lore-revision/src/branch/push.rs index 4ed08d9f..21cac440 100644 --- a/lore-revision/src/branch/push.rs +++ b/lore-revision/src/branch/push.rs @@ -288,6 +288,12 @@ pub struct PushOptions { pub branch: Option, /// Allow the server to fast-forward merge if the target branch head has moved pub fast_forward_merge: bool, + /// Allow the server to rebase onto the branch head if it has moved. Same + /// three-way diff as `fast_forward_merge`, but the result keeps only the + /// head as its parent, so the branch stays linear. Setting both is an + /// error: they ask for different histories, and quietly preferring one + /// would leave the caller unable to tell which it got. + pub rebase: bool, } impl EventError for PushError { @@ -480,6 +486,16 @@ pub async fn push( token: &RepositoryWriteToken, options: PushOptions, ) -> Result<(), PushError> { + // Rejected here rather than resolved by precedence: the two ask the server + // for different histories, and picking one silently would leave the caller + // unable to tell which it got. The CLI refuses the combination too, but + // embedders do not go through it. + if options.fast_forward_merge && options.rebase { + return Err(PushError::internal( + "fast_forward_merge and rebase are mutually exclusive", + )); + } + let _stats_report = PushStatsReport::start(); let branch; @@ -906,12 +922,13 @@ async fn collect_fragments_and_push( return Ok(()); } - // If the branch diverged, early out (unless fast-forward merge is enabled, - // in which case let the server attempt to resolve the divergence) + // If the branch diverged, early out (unless the caller asked the server to + // resolve the divergence — by merging or by rebasing) let force = execution_context().globals().force(); if !current_branch_remote_history.is_empty() && !force && !options.fast_forward_merge + && !options.rebase && !repository.is_link() { lore_debug!( @@ -1021,8 +1038,23 @@ async fn collect_fragments_and_push( let mut current_latest = Hash::default(); let mut fast_forward_merged = false; - for current_revision in full_local_history.iter().rev() { - let mut current_revision = *current_revision; + // The revision pushed on the previous iteration, as (the signature it has + // in local history, the signature it ended up with on the server). Set + // only when the server accepted that revision as it stood and merely + // renumbered it, so the two names denote the same content and the same + // lineage. That is the one situation in which the next revision's parent + // pointer may be moved. + // + // Deliberately NOT `current_latest`: that also holds a revision the head + // moved to for entirely different reasons — a server-side fast-forward + // merge, which is a new revision carrying somebody else's work. Pointing + // the next revision at that one keeps this revision's tree while claiming + // to descend from theirs, so the server sees an ordinary fast-forward and + // the push silently drops everything the other side added. Nothing + // reports it: the push succeeds and their commit is still in the history. + let mut renumbered_previous: Option<(Hash, Hash)> = None; + for original_revision in full_local_history.iter().rev() { + let mut current_revision = *original_revision; let state = State::deserialize(repository.clone(), current_revision) .await @@ -1030,22 +1062,35 @@ async fn collect_fragments_and_push( push_revision_links(&repository, token, &options, &state, branch).await?; - if !current_latest.is_zero() && state.parent_self() != current_latest { - // Rebase on new latest revision - // TODO(mjansson): This only handles revision number rewrite for now, implement proper - // automatic rebase if the push resulted in a clean rebase - // ... - + if let Some((local, stored)) = renumbered_previous + && local != stored + && state.parent_self() == local + { + // The parent this revision descends from was stored under a + // different signature (the server rewrites the revision number + // and re-serializes). Same content, same lineage, new name — so + // follow it. Any other mismatch is left alone: pushing the + // revision as it stands lets the server three-way merge it + // against a base it holds, which is what keeps both sides. + // + // TODO(mjansson): this still only follows a revision the server + // renamed — a real automatic rebase, applying this revision's + // delta onto a head that moved for other reasons, is not + // implemented. The result today is one server merge revision + // per pushed revision rather than a linear history. Note that + // a rebase cannot be done on the server alone: the merge is + // what keeps the pushed revision reachable there, and the next + // revision in the same push needs it as its diff base. event::LoreEvent::BranchPushRevisionUpdateBegin( LoreBranchPushRevisionUpdateBeginEventData { revision: state.revision(), old_parent: state.parent_self(), - new_parent: current_latest, + new_parent: stored, }, ) .send(); - state.set_parent_self(current_latest); + state.set_parent_self(stored); current_revision = state .serialize(repository.clone(), token) .await @@ -1088,7 +1133,13 @@ async fn collect_fragments_and_push( if !dry_run && remote_latest != current_revision { let push_result = revision_protocol - .branch_push(branch, current_revision, force, options.fast_forward_merge) + .branch_push( + branch, + current_revision, + force, + options.fast_forward_merge, + options.rebase, + ) .await; // If the server returns NotFound, the branch was deleted on the server. @@ -1131,11 +1182,13 @@ async fn collect_fragments_and_push( current_revision, force, options.fast_forward_merge, + options.rebase, ) .await, + options.rebase, )? } - result => forward_branch_push(result)?, + result => forward_branch_push(result, options.rebase)?, }; if response.fast_forward_merged { // Server performed a fast-forward merge — push succeeded with a new revision. @@ -1157,6 +1210,12 @@ async fn collect_fragments_and_push( remote_latest = response.revision; current_latest = response.revision; + // The head moved to a merge the server built, not to a + // renumbered copy of what we sent. The revision we pushed is + // on the server unchanged (as the merge's second parent), so + // the next revision's parent is already resolvable there and + // must keep pointing at it. + renumbered_previous = None; event::LoreEvent::BranchPushRevisionPushEnd( LoreBranchPushRevisionPushEndEventData { @@ -1201,8 +1260,13 @@ async fn collect_fragments_and_push( remote_latest = response.revision; current_latest = response.revision; + // The server took this revision as it stood — it only rewrote the + // revision number, which changes the signature. Record both names + // so the next revision can follow its parent to the stored one. + renumbered_previous = Some((*original_revision, response.revision)); } else { current_latest = current_revision; + renumbered_previous = Some((*original_revision, current_revision)); } let current_number = State::deserialize(repository.clone(), current_latest) @@ -1263,7 +1327,10 @@ async fn collect_fragments_and_push( /// does not hold, and the address is the peer's answer rather than anything the attempt decides, /// so both report it the same way. #[track_caller] -fn forward_branch_push(result: Result) -> Result { +fn forward_branch_push( + result: Result, + rebase_requested: bool, +) -> Result { match result { Err(ProtocolError::AddressNotFound(missing)) => { let address = Address::from(&missing.address[..]); @@ -1271,6 +1338,18 @@ fn forward_branch_push(result: Result) -> Result Err(err).forward::(concat!( + "pushing branch to remote: the server did not perform the rebase. ", + "If it does not support rebasing on push, nothing was pushed and ", + "nothing was merged — synchronize, or allow a fast-forward merge ", + "to let the server merge instead", + )), result => result.forward::("pushing branch to remote"), } } diff --git a/lore-revision/src/metadata.rs b/lore-revision/src/metadata.rs index e0a11acd..55ae0ab7 100644 --- a/lore-revision/src/metadata.rs +++ b/lore-revision/src/metadata.rs @@ -127,6 +127,11 @@ pub const REVERTED_FROM: &str = "reverted-from"; pub const CHANGE_REQUEST: &str = "change-request"; /// Indicates the revision was created by a fast-forward merge ([`MetadataType::Numeric`]) pub const FAST_FORWARD_MERGE: &str = "fast-forward-merge"; +/// Indicates the revision was rebased onto the branch head when it was pushed +/// ([`MetadataType::Numeric`]). Set instead of [`FAST_FORWARD_MERGE`], never +/// alongside it: the two describe the same integration resolved into different +/// histories, and a reader has to be able to tell them apart. +pub const REBASED_ON_PUSH: &str = "rebased-on-push"; /// Keys describing the operation that creates a revision rather than the work /// it records, written by that operation. diff --git a/lore-revision/src/revision/restore.rs b/lore-revision/src/revision/restore.rs index 91d0600a..7ca00a01 100644 --- a/lore-revision/src/revision/restore.rs +++ b/lore-revision/src/revision/restore.rs @@ -563,7 +563,7 @@ pub async fn restore( .send(); let response = revision_protocol - .branch_push(current_branch, signature, false, false) + .branch_push(current_branch, signature, false, false, false) .await .forward::("pushing branch head pointer")?; diff --git a/lore-server/src/cache/revision.rs b/lore-server/src/cache/revision.rs index 97bd0590..5ca4ea36 100644 --- a/lore-server/src/cache/revision.rs +++ b/lore-server/src/cache/revision.rs @@ -1352,7 +1352,7 @@ mod tests { serialized, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, STEP_ONE_HUNDRED, BOTH, ) @@ -1385,7 +1385,7 @@ mod tests { serialized, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, STEP_ONE_HUNDRED, BOTH, ) diff --git a/lore-server/src/grpc/forwarded_revision/v1/branch_delete.rs b/lore-server/src/grpc/forwarded_revision/v1/branch_delete.rs index 78404512..4dfaa3ee 100644 --- a/lore-server/src/grpc/forwarded_revision/v1/branch_delete.rs +++ b/lore-server/src/grpc/forwarded_revision/v1/branch_delete.rs @@ -111,7 +111,7 @@ mod test { state_hash, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/forwarded_revision/v1/branch_get.rs b/lore-server/src/grpc/forwarded_revision/v1/branch_get.rs index 300fc744..fc9d76ac 100644 --- a/lore-server/src/grpc/forwarded_revision/v1/branch_get.rs +++ b/lore-server/src/grpc/forwarded_revision/v1/branch_get.rs @@ -88,7 +88,7 @@ mod test { state_hash, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/handlers/branch_delete.rs b/lore-server/src/grpc/handlers/branch_delete.rs index cc9aeca9..4e7aa0f2 100644 --- a/lore-server/src/grpc/handlers/branch_delete.rs +++ b/lore-server/src/grpc/handlers/branch_delete.rs @@ -170,7 +170,7 @@ mod test { state_hash, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/handlers/branch_diff.rs b/lore-server/src/grpc/handlers/branch_diff.rs index a518bf06..ec2dc0cc 100644 --- a/lore-server/src/grpc/handlers/branch_diff.rs +++ b/lore-server/src/grpc/handlers/branch_diff.rs @@ -268,7 +268,7 @@ mod test { state_hash, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/handlers/branch_push.rs b/lore-server/src/grpc/handlers/branch_push.rs index 0e3e9bf7..fe2d936c 100644 --- a/lore-server/src/grpc/handlers/branch_push.rs +++ b/lore-server/src/grpc/handlers/branch_push.rs @@ -159,7 +159,13 @@ pub async fn handler( revision, bypass_protection, force, - fast_forward_merge, + // The deprecated request has no rebase field, so this path + // can only ever merge. + if fast_forward_merge { + Integration::Merge + } else { + Integration::Refuse + }, history_step_size, acceleration, ) @@ -281,6 +287,26 @@ pub struct PushResult { pub revision_number: u64, } +/// What the server may do with a push whose parent is no longer the branch +/// head. +/// +/// One value rather than a pair of flags because the two ways of integrating +/// are mutually exclusive — they produce different histories, and a request +/// asking for both is refused at the wire boundary. Expressing that here makes +/// the invalid combination unrepresentable rather than something every caller +/// has to be trusted not to construct. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Integration { + /// Refuse the push and let the client resolve the divergence itself. + Refuse, + /// Merge onto the head, recording the pushed revision as the second + /// parent, which keeps it reachable on the branch. + Merge, + /// Rebase onto the head, recording only the head as the parent, which + /// keeps the branch linear. + Rebase, +} + #[allow(clippy::too_many_arguments)] #[instrument(level = "debug", skip_all, fields(branch))] pub async fn push( @@ -289,7 +315,7 @@ pub async fn push( latest: Hash, bypass_protection: bool, force: bool, - fast_forward_merge: bool, + integration: Integration, history_step_size: u64, acceleration: crate::grpc::server::RevisionListAcceleration, ) -> Result { @@ -365,7 +391,7 @@ pub async fn push( return Err(Status::not_found("Branch not found")); } - if !fast_forward_merge { + if integration == Integration::Refuse { return Ok(PushResult { success: false, fast_forward_merged: false, @@ -374,14 +400,17 @@ pub async fn push( }); } - // Fast-forward merge: the incoming revision's parent_self no longer matches - // the branch head. Attempt to create a new merge revision with - // parent_self=current_head and parent_other=incoming_revision. - return try_fast_forward_merge( + // The incoming revision's parent_self no longer matches the branch + // head. Integrate it with a three-way diff against its own parent: + // with parent_self=current_head, and parent_other=incoming_revision + // unless the caller asked for a rebase, which keeps the branch + // linear by recording only the head as the parent. + return try_integrate_onto_head( repository.clone(), branch, state.clone(), current_head, + integration == Integration::Rebase, history_step_size, acceleration, ) @@ -463,24 +492,32 @@ pub async fn push( }) } -/// Attempts a server-side fast-forward merge when the target branch head has moved -/// since the client created the merge revision. +/// Integrates a push whose parent is no longer the branch head, by applying the +/// incoming revision's changes onto the head. /// -/// Creates a new merge revision with: -/// - `parent_self` = current branch head (target branch) -/// - `parent_other` = the incoming merge revision +/// Uses a three-way diff between the incoming revision's own parent, the +/// incoming revision, and the current head, and applies the non-conflicting +/// changes to the head's state. If conflicts are detected, returns failure so +/// the client can resolve locally. /// -/// Uses a three-way diff between the original merge base, the incoming revision, -/// and the current head. If conflicts are detected, returns failure so the client -/// can resolve locally. +/// The result records: +/// - `parent_self` = current branch head (target branch), always +/// - `parent_other` = the incoming revision, unless `rebase` +/// +/// That single difference is what separates the two shapes. Merging keeps the +/// pushed revision reachable as a second parent, which is also what lets the +/// next revision of the same push use it as a diff base. Rebasing keeps the +/// branch linear and drops it, so the pushed revision survives only as long as +/// the store holds it — long enough for the rest of this push, not beyond. /// /// Retries via CAS loop if the branch head moves again during processing. #[instrument(level = "debug", skip_all)] -async fn try_fast_forward_merge( +async fn try_integrate_onto_head( repository: Arc, branch: BranchId, incoming_state: Arc, mut current_head: Hash, + rebase: bool, history_step_size: u64, acceleration: crate::grpc::server::RevisionListAcceleration, ) -> Result { @@ -577,9 +614,13 @@ async fn try_fast_forward_merge( )) })?; - // Set parents: self=current head (target branch), other=incoming merge revision + // Set parents: self=current head (target branch), other=incoming + // revision — the latter only when merging. A rebase records no second + // parent, which is what keeps the branch linear. state_current.set_parent_self(current_head); - state_current.set_parent_other(incoming_revision); + if !rebase { + state_current.set_parent_other(incoming_revision); + } // Compute revision number from both parents let parent_state = State::deserialize(repository.clone(), current_head) @@ -589,13 +630,23 @@ async fn try_fast_forward_merge( Status::internal(format!("Failed to load current head state: {err}")) })?; + // A rebased revision has one parent, so its number follows the head + // alone — the same arithmetic the ordinary push path uses when there + // is no second parent. let revision_number = next_revision_number( parent_state.revision_number(), - incoming_state.revision_number(), + if rebase { + 0 + } else { + incoming_state.revision_number() + }, ); state_current.set_revision_number(revision_number); - // Copy metadata from the incoming revision and set merged-by to "server" + // Copy metadata from the incoming revision. A merge is stamped as + // merged-by/fast-forward-merge; a rebase carries no merger — nothing + // was merged — and is marked as rebased instead, so a reader can tell + // the two apart rather than seeing a merge that has no second parent. let incoming_metadata_hash = incoming_state.metadata_hash(); if !incoming_metadata_hash.is_zero() { let mut metadata = lore_revision::metadata::Metadata::deserialize( @@ -611,20 +662,30 @@ async fn try_fast_forward_merge( metadata .set_branch(branch) .warn_map_err(|_| Status::internal("Failed to set branch in metadata"))?; - // Preserve the existing merged-by field if set, otherwise fall back to "server" - if metadata - .get_string(lore_revision::metadata::MERGED_BY) - .is_err() - { + if rebase { metadata - .set_string(lore_revision::metadata::MERGED_BY, "server") - .warn_map_err(|_| Status::internal("Failed to set merged-by in metadata"))?; + .set_u64(lore_revision::metadata::REBASED_ON_PUSH, 1) + .warn_map_err(|_| { + Status::internal("Failed to set rebased-on-push in metadata") + })?; + } else { + // Preserve the existing merged-by field if set, otherwise fall back to "server" + if metadata + .get_string(lore_revision::metadata::MERGED_BY) + .is_err() + { + metadata + .set_string(lore_revision::metadata::MERGED_BY, "server") + .warn_map_err(|_| { + Status::internal("Failed to set merged-by in metadata") + })?; + } + metadata + .set_u64(lore_revision::metadata::FAST_FORWARD_MERGE, 1) + .warn_map_err(|_| { + Status::internal("Failed to set fast-forward-merge in metadata") + })?; } - metadata - .set_u64(lore_revision::metadata::FAST_FORWARD_MERGE, 1) - .warn_map_err(|_| { - Status::internal("Failed to set fast-forward-merge in metadata") - })?; let metadata_hash = metadata .serialize(repository.clone()) @@ -1065,7 +1126,7 @@ mod tests { state.revision(), true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, RevisionListAcceleration::default(), ) @@ -1109,7 +1170,7 @@ mod tests { state.revision(), true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, RevisionListAcceleration::default(), ) @@ -1265,7 +1326,7 @@ mod tests { nonexistent_revision, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, RevisionListAcceleration::default(), ) @@ -1316,7 +1377,7 @@ mod tests { state.revision(), true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, RevisionListAcceleration::default(), ) @@ -1379,7 +1440,7 @@ mod tests { state.revision(), true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, RevisionListAcceleration::default(), ) @@ -1443,7 +1504,7 @@ mod tests { merge.revision(), true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, RevisionListAcceleration::default(), ) @@ -1515,7 +1576,7 @@ mod tests { merge.revision(), true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/handlers/branch_revision_list.rs b/lore-server/src/grpc/handlers/branch_revision_list.rs index 29cce27f..4bb4025b 100644 --- a/lore-server/src/grpc/handlers/branch_revision_list.rs +++ b/lore-server/src/grpc/handlers/branch_revision_list.rs @@ -151,7 +151,7 @@ mod tests { first_hash, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -173,7 +173,7 @@ mod tests { second_hash, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -195,7 +195,7 @@ mod tests { third_hash, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -217,7 +217,7 @@ mod tests { fourth_hash, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -454,7 +454,7 @@ mod tests { first_hash, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -475,7 +475,7 @@ mod tests { second_hash, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -496,7 +496,7 @@ mod tests { third_hash, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/handlers/revision_describe.rs b/lore-server/src/grpc/handlers/revision_describe.rs index 96e215b8..c0223ead 100644 --- a/lore-server/src/grpc/handlers/revision_describe.rs +++ b/lore-server/src/grpc/handlers/revision_describe.rs @@ -151,7 +151,7 @@ mod tests { first_hash, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -173,7 +173,7 @@ mod tests { second_hash, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -195,7 +195,7 @@ mod tests { third_hash, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -266,7 +266,7 @@ mod tests { merge_hash, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/handlers/revision_tree.rs b/lore-server/src/grpc/handlers/revision_tree.rs index 9d553d8e..33f9cc96 100644 --- a/lore-server/src/grpc/handlers/revision_tree.rs +++ b/lore-server/src/grpc/handlers/revision_tree.rs @@ -174,7 +174,7 @@ mod tests { revision_hash, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -248,7 +248,7 @@ mod tests { revision_hash, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -339,7 +339,7 @@ mod tests { revision_hash, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -455,7 +455,7 @@ mod tests { revision_hash, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/revision/v1/branch_delete.rs b/lore-server/src/grpc/revision/v1/branch_delete.rs index 28a278cd..444d582d 100644 --- a/lore-server/src/grpc/revision/v1/branch_delete.rs +++ b/lore-server/src/grpc/revision/v1/branch_delete.rs @@ -267,7 +267,7 @@ mod test { state_hash, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/revision/v1/branch_get.rs b/lore-server/src/grpc/revision/v1/branch_get.rs index 3ee342c6..b9832734 100644 --- a/lore-server/src/grpc/revision/v1/branch_get.rs +++ b/lore-server/src/grpc/revision/v1/branch_get.rs @@ -229,7 +229,7 @@ mod test { state_hash, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/revision/v1/branch_list.rs b/lore-server/src/grpc/revision/v1/branch_list.rs index 02d0dca0..08ef1c28 100644 --- a/lore-server/src/grpc/revision/v1/branch_list.rs +++ b/lore-server/src/grpc/revision/v1/branch_list.rs @@ -374,7 +374,7 @@ pub mod test { state_hash, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/revision/v1/branch_metadata_get.rs b/lore-server/src/grpc/revision/v1/branch_metadata_get.rs index 854c93b8..81f80bff 100644 --- a/lore-server/src/grpc/revision/v1/branch_metadata_get.rs +++ b/lore-server/src/grpc/revision/v1/branch_metadata_get.rs @@ -201,7 +201,7 @@ mod test { state_hash, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/revision/v1/branch_metadata_set.rs b/lore-server/src/grpc/revision/v1/branch_metadata_set.rs index 1f60bbad..e0c6be4a 100644 --- a/lore-server/src/grpc/revision/v1/branch_metadata_set.rs +++ b/lore-server/src/grpc/revision/v1/branch_metadata_set.rs @@ -520,7 +520,7 @@ mod test { state_hash, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, lore_revision::branch::DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/revision/v1/branch_push.rs b/lore-server/src/grpc/revision/v1/branch_push.rs index bb89718b..79d30dd1 100644 --- a/lore-server/src/grpc/revision/v1/branch_push.rs +++ b/lore-server/src/grpc/revision/v1/branch_push.rs @@ -31,6 +31,7 @@ use crate::grpc::extract_correlation_id; use crate::grpc::get_authorization; use crate::grpc::get_repository; use crate::grpc::get_user_id; +use crate::grpc::handlers::branch_push::Integration; use crate::grpc::handlers::branch_push::PushResult; use crate::grpc::handlers::branch_push::dispatch_response_message; use crate::grpc::handlers::branch_push::extract_client_ip; @@ -88,6 +89,7 @@ pub async fn handler( let revision = Hash::from(req.revision_signature); let force = req.force; let fast_forward_merge = req.fast_forward_merge; + let rebase = req.rebase; if revision.is_zero() { info!("Invalid branch push request, revision_signature is zero"); @@ -96,12 +98,30 @@ pub async fn handler( )); } + // The wire carries two independent bits; the server takes one decision. + // Both set is refused rather than resolved by precedence: they ask for + // different histories, and applying one anyway would leave the client + // unable to tell which it got. Past this point the invalid combination + // cannot be expressed. + let integration = match (fast_forward_merge, rebase) { + (true, true) => { + info!("Invalid branch push request, fast_forward_merge and rebase both set"); + return Err(Status::invalid_argument( + "fast_forward_merge and rebase are mutually exclusive", + )); + } + (true, false) => Integration::Merge, + (false, true) => Integration::Rebase, + (false, false) => Integration::Refuse, + }; + debug!( {REVISION} = %revision, bypass_protection, {BRANCH_ID} = %branch_id, force, fast_forward_merge, + rebase, "Handling branch push request", ); @@ -146,7 +166,7 @@ pub async fn handler( revision, bypass_protection, force, - fast_forward_merge, + integration, history_step_size, acceleration, ) @@ -420,12 +440,31 @@ mod test { revision: Hash, force: bool, fast_forward_merge: bool, + ) -> Request { + make_request_with( + repository, + branch, + revision, + force, + fast_forward_merge, + false, + ) + } + + fn make_request_with( + repository: RepositoryId, + branch: BranchId, + revision: Hash, + force: bool, + fast_forward_merge: bool, + rebase: bool, ) -> Request { let mut request = Request::new(BranchPushRequest { id: branch.into(), revision_signature: revision.into(), force, fast_forward_merge, + rebase, }); request.metadata_mut().insert_bin( REPOSITORY_ID_KEY, @@ -494,6 +533,45 @@ mod test { .await; } + /// The two ask for different histories, so the request is refused rather + /// than resolved by precedence — a client must never be left guessing + /// which of the two it got. + #[tokio::test] + async fn push_with_both_merge_and_rebase_returns_invalid_argument() { + let repository = random::(); + let (immutable_store, mutable_store, execution) = + test_store_create().await.expect("Failed to create stores"); + + let notification_sender = Arc::new(MockNotificationSender::new()); + let instrument_provider = TestInstrumentProvider {}; + + Box::pin(LORE_CONTEXT.scope(execution.clone(), async move { + let repository_context = Arc::new(RepositoryContext::new_server_context( + immutable_store.clone(), + mutable_store.clone(), + repository, + )); + let main = create_root_branch(&repository_context, "main").await; + let revision = build_revision(&repository_context, Hash::default(), 1).await; + + let hook_dispatcher = HookDispatcher::empty(); + let err = handler( + make_request_with(repository, main, revision, false, true, true), + immutable_store.clone(), + mutable_store.clone(), + notification_sender.clone(), + &hook_dispatcher, + DEFAULT_HISTORY_STEP_SIZE, + crate::grpc::server::RevisionListAcceleration::default(), + &instrument_provider, + ) + .await + .expect_err("asking for a merge and a rebase at once should fail"); + assert_eq!(err.code(), tonic::Code::InvalidArgument); + })) + .await; + } + #[tokio::test] async fn push_zero_revision_returns_invalid_argument() { let repository = random::(); diff --git a/lore-server/src/grpc/revision/v1/revision_list.rs b/lore-server/src/grpc/revision/v1/revision_list.rs index 015c7e81..a613f492 100644 --- a/lore-server/src/grpc/revision/v1/revision_list.rs +++ b/lore-server/src/grpc/revision/v1/revision_list.rs @@ -1166,7 +1166,7 @@ mod test { serialized, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -1239,7 +1239,7 @@ mod test { serialized, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/thinclient/v1/revision_diff.rs b/lore-server/src/grpc/thinclient/v1/revision_diff.rs index a447505f..66b78b37 100644 --- a/lore-server/src/grpc/thinclient/v1/revision_diff.rs +++ b/lore-server/src/grpc/thinclient/v1/revision_diff.rs @@ -859,7 +859,7 @@ mod test { serialized, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/thinclient/v1/revision_info.rs b/lore-server/src/grpc/thinclient/v1/revision_info.rs index 16380a94..5bee75fb 100644 --- a/lore-server/src/grpc/thinclient/v1/revision_info.rs +++ b/lore-server/src/grpc/thinclient/v1/revision_info.rs @@ -360,7 +360,7 @@ mod test { serialized, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -551,7 +551,7 @@ mod test { serialized, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -644,7 +644,7 @@ mod test { serialized, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-server/src/grpc/thinclient/v1/revision_tree.rs b/lore-server/src/grpc/thinclient/v1/revision_tree.rs index e34c91df..ed60a796 100644 --- a/lore-server/src/grpc/thinclient/v1/revision_tree.rs +++ b/lore-server/src/grpc/thinclient/v1/revision_tree.rs @@ -290,7 +290,7 @@ mod test { serialized, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -378,7 +378,7 @@ mod test { serialized, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -481,7 +481,7 @@ mod test { serialized, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) @@ -1490,7 +1490,7 @@ mod test { serialized, true, true, - false, + crate::grpc::handlers::branch_push::Integration::Refuse, DEFAULT_HISTORY_STEP_SIZE, crate::grpc::server::RevisionListAcceleration::default(), ) diff --git a/lore-transport/src/grpc/mod.rs b/lore-transport/src/grpc/mod.rs index 566971d2..8a50bebd 100644 --- a/lore-transport/src/grpc/mod.rs +++ b/lore-transport/src/grpc/mod.rs @@ -1444,6 +1444,7 @@ impl Revision for GRPCRevision { latest: Hash, force: bool, fast_forward_merge: bool, + rebase: bool, ) -> Result { with_reconnect( &self.connection, @@ -1451,7 +1452,7 @@ impl Revision for GRPCRevision { self.client .read() .await - .branch_push(branch, latest, force, fast_forward_merge) + .branch_push(branch, latest, force, fast_forward_merge, rebase) .await }, |reconnect_id| self.reconnect(reconnect_id), diff --git a/lore-transport/src/grpc/revision_client.rs b/lore-transport/src/grpc/revision_client.rs index 786e957c..5a1b9e78 100644 --- a/lore-transport/src/grpc/revision_client.rs +++ b/lore-transport/src/grpc/revision_client.rs @@ -227,6 +227,7 @@ impl RevisionService { revision: Hash, force: bool, fast_forward_merge: bool, + rebase: bool, ) -> Result { lore_debug!("Pushing branch: {} at {}", branch, revision); let _ = RequestScopedCounter::new(self.request_inflight.clone()); @@ -238,6 +239,7 @@ impl RevisionService { revision_signature: revision.into(), force, fast_forward_merge, + rebase, }; let mut client = self.client.clone(); diff --git a/lore-transport/src/traits.rs b/lore-transport/src/traits.rs index 37cbd504..1d5c9448 100644 --- a/lore-transport/src/traits.rs +++ b/lore-transport/src/traits.rs @@ -274,12 +274,16 @@ pub trait Revision: Send + Sync { /// Push a new LATEST pointer for branch. Returns the (new) current LATEST pointer for the branch, /// if this is different from the given LATEST pointer the operation failed due to the /// LATEST pointer having moved. + /// `rebase` integrates by rebasing onto the head instead of merging onto + /// it, and is mutually exclusive with `fast_forward_merge` — callers are + /// expected to have rejected the combination before reaching here. async fn branch_push( &self, branch: BranchId, latest: Hash, force: bool, fast_forward_merge: bool, + rebase: bool, ) -> Result; /// List all branches diff --git a/lore/src/branch.rs b/lore/src/branch.rs index a9a407d6..ca3f51de 100644 --- a/lore/src/branch.rs +++ b/lore/src/branch.rs @@ -860,6 +860,9 @@ pub struct LoreBranchPushArgs { pub branch: LoreString, /// Allow the server to fast-forward merge if the target branch head has moved pub fast_forward_merge: u8, + /// Allow the server to rebase onto the target branch head if it has moved, + /// keeping the branch linear. Mutually exclusive with `fast_forward_merge`. + pub rebase: u8, } /// Pushes the current or specified branch and its revisions to the remote. @@ -929,6 +932,7 @@ async fn push_impl( let options = PushOptions { branch: args.branch.into(), fast_forward_merge: args.fast_forward_merge != 0, + rebase: args.rebase != 0, }; // Push is never local