From patchwork Thu Apr 18 13:14:05 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phillip Wood X-Patchwork-Id: 13634712 Received: from mail-wr1-f41.google.com (mail-wr1-f41.google.com [209.85.221.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E7009146005 for ; Thu, 18 Apr 2024 13:14:44 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713446086; cv=none; b=PPrT7bdaCh6K0xWe3C5w0vEyhjUpyNeyAeN8Tp4exPEZ5Le7eJ3t+O72RUNWSBkh0ZaWWPhtbjG/7Wvig3JuODKTcJ8QE1JHTFZIzrXxzIiBCjfHuCuKNUd5TzY3y3+xLTvnqnbppP8Xf7ojYjwUpCd9aeso/paZd594IGOupbM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713446086; c=relaxed/simple; bh=rkbYOeKhdwY7rqFxx80XwEwnMsa6eLA2NetHCN1poQk=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=jZapLC8vUsGKqk+3jC5130ftxlz7UW4y/+lUQHHfzGdhhoFjK5+ncz2jznd+d3ctsVRwvCXAktjgnOmE5znJa/3OOvo8yF0NH7ClFYGgwrhVTj7t2Q5mLd+vX0AUpmXQ+voSxOXVeMVC7ZRRRcQRV8ioCznkkLe8iImB3YbBW6I= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=GcGGndu6; arc=none smtp.client-ip=209.85.221.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="GcGGndu6" Received: by mail-wr1-f41.google.com with SMTP id ffacd0b85a97d-349c4505058so583461f8f.3 for ; Thu, 18 Apr 2024 06:14:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1713446083; x=1714050883; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:reply-to:references :in-reply-to:message-id:date:subject:cc:to:from:from:to:cc:subject :date:message-id:reply-to; bh=kCOFTHk9OLWOT5my2MTw8V0w0oC6hE+y+576LTdSmOo=; b=GcGGndu6gigVuwZQGQ6wWLAvTusxthSsQ0+VliAmdn5D/MQcUylIcuhMm79mQ6v7Vb Rxn2zoVOa/suuor5fDV1ZyaYJUsyjZaZxe+XVZTJBzWfPsehXszCWOYjAaRTuUJziFzv 1uzRBDh1+HUmHevUXb5ZnKq1nQ6nJhOWD+nW68A1q1i3iDgErvdlUE2BaO4uZJni1svm al8mFHnY1rBnCqpiro8/ZriYEA9k4x9wTyZVoRQvy8xpS3KzD86AVIKuA3y+n0fup/A1 kac/aeON1hKmpAS+Dq8GvxGA6nGQ269RXA195HCVhznFVMUuSOBPXtqDobOdDpiENJIF aksw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713446083; x=1714050883; h=content-transfer-encoding:mime-version:reply-to:references :in-reply-to:message-id:date:subject:cc:to:from:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=kCOFTHk9OLWOT5my2MTw8V0w0oC6hE+y+576LTdSmOo=; b=lv8X9yo7EP1oIVZtigAWAo+Dtqf5fEiROf5+/M+o0q6JdnkNdPYD7aOMLg3dAm4WI2 02ExWnHkuBnrl9RuwQK4bZj7g0DRAGBdmvyAKF/D62bcOICyfqXCHVhKN9jShxkxmydd JfZfYho3RgeCdmkxsa0IZXvdYTAJgC4kJMuWddD86dITOUfYnQeuiApDyP1D1iKUAoas FYnN4Bma3RmijZxyHHxM5lmQoiF29N3tvZnl/2jlDAXtT690kUXkdSFj2JSptWzBvKip gLI/A69O9KPHAsojU6+yf9PqyBjIi9u77/3jBqqAy2fGFArciZQ5Z8wnblqICA4+EiDF Gr3w== X-Gm-Message-State: AOJu0Ywa4wSyjIkrV84HITDTmdLFghoPvwFwuVcFC/X4rtgRRfT7oA6O 5jeuN45NbbEbvUk/loXI93TmJvFFIKC84xatq1pua4nBFv9aW9cVp3Tmng== X-Google-Smtp-Source: AGHT+IEnuomCPp0d/8iJqxp8T3NntnVdtTu4kdbzf01RUx/HNgKJtM6DNBQAECABU2otHuFhd75nvQ== X-Received: by 2002:adf:e9c3:0:b0:343:f662:18f8 with SMTP id l3-20020adfe9c3000000b00343f66218f8mr1772122wrn.63.1713446083036; Thu, 18 Apr 2024 06:14:43 -0700 (PDT) Received: from localhost.localdomain ([2a0a:ef40:68c:c401:12ba:addc:3daa:a3e]) by smtp.gmail.com with ESMTPSA id r14-20020a05600c458e00b00417e5b71188sm2682748wmo.34.2024.04.18.06.14.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 18 Apr 2024 06:14:42 -0700 (PDT) From: Phillip Wood To: git@vger.kernel.org Cc: David Bimmler , Johannes Schindelin , Phillip Wood Subject: [PATCH 1/5] sequencer: always free "struct replay_opts" Date: Thu, 18 Apr 2024 14:14:05 +0100 Message-ID: <9d96300b1c8ce3ab88affa571b2e29cd5b52160d.1713445918.git.phillip.wood@dunelm.org.uk> X-Mailer: git-send-email 2.44.0.661.ge68cfcc6c2f In-Reply-To: References: Reply-To: Phillip Wood Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Phillip Wood sequencer_post_commit_cleanup() initializes an instance of "struct replay_opts" but does not call replay_opts_release(). Currently this does not leak memory because the code paths called don't allocate any of the struct members. That will change in the next commit so add call to replay_opts_release() to prevent a memory leak in "git commit" that breaks all of the leak free tests. Signed-off-by: Phillip Wood --- sequencer.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/sequencer.c b/sequencer.c index f49a871ac06..e4146b4cdfa 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2823,12 +2823,14 @@ void sequencer_post_commit_cleanup(struct repository *r, int verbose) NULL, REF_NO_DEREF); if (!need_cleanup) - return; + goto out; if (!have_finished_the_last_pick()) - return; + goto out; sequencer_remove_state(&opts); +out: + replay_opts_release(&opts); } static void todo_list_write_total_nr(struct todo_list *todo_list) From patchwork Thu Apr 18 13:14:06 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phillip Wood X-Patchwork-Id: 13634714 Received: from mail-wm1-f43.google.com (mail-wm1-f43.google.com [209.85.128.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 72C331487F0 for ; Thu, 18 Apr 2024 13:14:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.43 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713446088; cv=none; b=JObYJ0C9LHf4xoZM9zWt1cEr3b5b+EG6QLitAKOIr5STIoej4rEtOJ22KvhoYajGZnu7gEUV0XoBWWgIXpgaAVmmIkHBBpELbVAP08KBgbd30GxdMHq8SChQBpic+xGQZucbnC+o/VmPp9nxN1v5IWL3fSJpV9eel5gxDPQ8KB4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713446088; c=relaxed/simple; bh=ZupEspvFSPdBw/p5dB8kVHRIAA14bnuwtAkakFFU5QA=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=VeNohjtgi+Rz0vGfKP2NMU4A0NPD/pjC4+m1ynzTYI2FjGHQh1rbmXsrESLctsfeN4loGxx14atX8MaA6xxh7Rj9g6wtowRWkhopa4ZDV8wMqYuILbM+E4hU7uqwMQpeQggvhrBI7Hn7tb4jj8KN+2zCKGzvxC5xvJUkZYplK2Q= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=G5mxC883; arc=none smtp.client-ip=209.85.128.43 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="G5mxC883" Received: by mail-wm1-f43.google.com with SMTP id 5b1f17b1804b1-418c979ddf3so6770005e9.0 for ; Thu, 18 Apr 2024 06:14:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1713446084; x=1714050884; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:reply-to:references :in-reply-to:message-id:date:subject:cc:to:from:from:to:cc:subject :date:message-id:reply-to; bh=EAVZcYnHNHgcvHTQdiwfcnCFvp3HBvn67/bgcx1K/QQ=; b=G5mxC883pnj7NKwSfyv7VKp9DacENSGrLlFR0w6lQDmRWnm6Zc3bb3TVOsFVY9yBzl 0FhkM+lsgtdwXlZ1Ew3zQuYuAhHpgKtb+9skDfHqncwHi9r3v79RDyk2lwIGALWOaX+y CFvy3B53jdNROeM2mmZlXls8O6rezxtI0UBrrHzdPoFfooRQ4t2wFPPkNd9MSe6ZYOLy ZIOIwL6dQrDYzNkR8l4pAGSrEYdthqJrq3hBjQVlArtnAkjF10xD3i6IVBYfax1S423N x5N9HoEF+tK4SOkZOkan90MtO+0KpL+TPghfE9S5Q/fL258USnzKtVzDgxAgUaYJ/rYh Jw0g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713446084; x=1714050884; h=content-transfer-encoding:mime-version:reply-to:references :in-reply-to:message-id:date:subject:cc:to:from:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=EAVZcYnHNHgcvHTQdiwfcnCFvp3HBvn67/bgcx1K/QQ=; b=bDuk6hDcemx4VVBuMEk4ZKsr9Fumz534itfEV6pJt45hnUT9RtxARqVkiBEIIDpbr+ /oYXlouxZB3Hh2wIWv+/pOEmkIY0zfmUVLSPG/wC57dsyXMURBDoOLVncQoukt1y8Kxg 1rAdNty8ApJfhBm/GNTbeuuJu0/Q0GPwK2XlSdyjx5VtGtWoYi8GRnA1gkhLcXZiv0J6 CydETCWYFOaBnUfKvtPi/Ig7GbJzmhPbGSWVGugfy8lXdPH335OdTtS/VKD0LNUr/fbM VzfLgzAFAWto5FFz332E9ouGr1limxj3bEAJq/q52m+zD5pwrCYjL4mPT4CWhq9n0d5U 9Bnw== X-Gm-Message-State: AOJu0YyekZ3WD3NYU+erTfhHIiTICFB0xjkJHCoJRHa87oMMY6dDeYQK MTSk4tYfQOUvJn/qLUn32WIype8cry4kNRRQrN14Oeq5lbHJQX5vnIUmNA== X-Google-Smtp-Source: AGHT+IGWinfRs196eB/J+Rg0XKFrp8ZwVlxhoft+2DmmrEhDibboLEwtmxW3EYTz2KAVhTp0QxW1Yw== X-Received: by 2002:a05:600c:4587:b0:418:7451:a385 with SMTP id r7-20020a05600c458700b004187451a385mr1810111wmo.40.1713446083609; Thu, 18 Apr 2024 06:14:43 -0700 (PDT) Received: from localhost.localdomain ([2a0a:ef40:68c:c401:12ba:addc:3daa:a3e]) by smtp.gmail.com with ESMTPSA id r14-20020a05600c458e00b00417e5b71188sm2682748wmo.34.2024.04.18.06.14.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 18 Apr 2024 06:14:43 -0700 (PDT) From: Phillip Wood To: git@vger.kernel.org Cc: David Bimmler , Johannes Schindelin , Phillip Wood Subject: [PATCH 2/5] sequencer: start removing private fields from public API Date: Thu, 18 Apr 2024 14:14:06 +0100 Message-ID: <2beedb4547013629a507d646d582ca6b3f420c2c.1713445918.git.phillip.wood@dunelm.org.uk> X-Mailer: git-send-email 2.44.0.661.ge68cfcc6c2f In-Reply-To: References: Reply-To: Phillip Wood Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Phillip Wood "struct replay_opts" has a number of fields that are for internal use. While they are marked as private having them in a public struct is a distraction for callers and means that every time the internal details are changed we have to recompile all the files that include sequencer.h even though the public API is unchanged. This commit starts the process of removing the private fields by adding an opaque pointer to a "struct replay_ctx" to "struct replay_opts" and moving the "reflog_message" member to the new private struct. The sequencer currently updates the state files on disc each time it processes a command in the todo list. This is an artifact of the scripted implementation and makes the code hard to reason about as it is not possible to get a complete view of the state in memory. In the future we will add new members to "struct replay_ctx" to remedy this and avoid writing state to disc unless the sequencer stops for user interaction. Signed-off-by: Phillip Wood --- sequencer.c | 36 ++++++++++++++++++++++++++++++------ sequencer.h | 6 +++++- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/sequencer.c b/sequencer.c index e4146b4cdfa..6ddf6a8aa1e 100644 --- a/sequencer.c +++ b/sequencer.c @@ -207,6 +207,24 @@ static GIT_PATH_FUNC(rebase_path_no_reschedule_failed_exec, "rebase-merge/no-res static GIT_PATH_FUNC(rebase_path_drop_redundant_commits, "rebase-merge/drop_redundant_commits") static GIT_PATH_FUNC(rebase_path_keep_redundant_commits, "rebase-merge/keep_redundant_commits") +/* + * A 'struct replay_ctx' represents the private state of the sequencer. + */ +struct replay_ctx { + /* + * Stores the reflog message that will be used when creating a + * commit. Points to a static buffer and should not be free()'d. + */ + const char *reflog_message; +}; + +struct replay_ctx* replay_ctx_new(void) +{ + struct replay_ctx *ctx = xcalloc(1, sizeof(*ctx)); + + return ctx; +} + /** * A 'struct update_refs_record' represents a value in the update-refs * list. We use a string_list to map refs to these (before, after) pairs. @@ -377,6 +395,7 @@ void replay_opts_release(struct replay_opts *opts) if (opts->revs) release_revisions(opts->revs); free(opts->revs); + free(opts->ctx); } int sequencer_remove_state(struct replay_opts *opts) @@ -1054,6 +1073,7 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts, unsigned int flags) { + struct replay_ctx *ctx = opts->ctx; struct child_process cmd = CHILD_PROCESS_INIT; if ((flags & CLEANUP_MSG) && (flags & VERBATIM_MSG)) @@ -1071,7 +1091,7 @@ static int run_git_commit(const char *defmsg, gpg_opt, gpg_opt); } - strvec_pushf(&cmd.env, GIT_REFLOG_ACTION "=%s", opts->reflog_message); + strvec_pushf(&cmd.env, GIT_REFLOG_ACTION "=%s", ctx->reflog_message); if (opts->committer_date_is_author_date) strvec_pushf(&cmd.env, "GIT_COMMITTER_DATE=%s", @@ -1457,6 +1477,7 @@ static int try_to_commit(struct repository *r, struct replay_opts *opts, unsigned int flags, struct object_id *oid) { + struct replay_ctx *ctx = opts->ctx; struct object_id tree; struct commit *current_head = NULL; struct commit_list *parents = NULL; @@ -1618,7 +1639,7 @@ static int try_to_commit(struct repository *r, goto out; } - if (update_head_with_reflog(current_head, oid, opts->reflog_message, + if (update_head_with_reflog(current_head, oid, ctx->reflog_message, msg, &err)) { res = error("%s", err.buf); goto out; @@ -4725,11 +4746,12 @@ static int pick_one_commit(struct repository *r, struct replay_opts *opts, int *check_todo, int* reschedule) { + struct replay_ctx *ctx = opts->ctx; int res; struct todo_item *item = todo_list->items + todo_list->current; const char *arg = todo_item_get_arg(todo_list, item); if (is_rebase_i(opts)) - opts->reflog_message = reflog_message( + ctx->reflog_message = reflog_message( opts, command_to_string(item->command), NULL); res = do_pick_commit(r, item, opts, is_final_fixup(todo_list), @@ -4786,9 +4808,10 @@ static int pick_commits(struct repository *r, struct todo_list *todo_list, struct replay_opts *opts) { + struct replay_ctx *ctx = opts->ctx; int res = 0, reschedule = 0; - opts->reflog_message = sequencer_reflog_action(opts); + ctx->reflog_message = sequencer_reflog_action(opts); if (opts->allow_ff) assert(!(opts->signoff || opts->no_commit || opts->record_origin || should_edit(opts) || @@ -5205,6 +5228,7 @@ static int commit_staged_changes(struct repository *r, int sequencer_continue(struct repository *r, struct replay_opts *opts) { + struct replay_ctx *ctx = opts->ctx; struct todo_list todo_list = TODO_LIST_INIT; int res; @@ -5224,7 +5248,7 @@ int sequencer_continue(struct repository *r, struct replay_opts *opts) unlink(rebase_path_dropped()); } - opts->reflog_message = reflog_message(opts, "continue", NULL); + ctx->reflog_message = reflog_message(opts, "continue", NULL); if (commit_staged_changes(r, opts, &todo_list)) { res = -1; goto release_todo_list; @@ -5276,7 +5300,7 @@ static int single_pick(struct repository *r, TODO_PICK : TODO_REVERT; item.commit = cmit; - opts->reflog_message = sequencer_reflog_action(opts); + opts->ctx->reflog_message = sequencer_reflog_action(opts); return do_pick_commit(r, &item, opts, 0, &check_todo); } diff --git a/sequencer.h b/sequencer.h index dcef7bb99c0..069f06400d2 100644 --- a/sequencer.h +++ b/sequencer.h @@ -29,6 +29,9 @@ enum commit_msg_cleanup_mode { COMMIT_MSG_CLEANUP_ALL }; +struct replay_ctx; +struct replay_ctx* replay_ctx_new(void); + struct replay_opts { enum replay_action action; @@ -78,13 +81,14 @@ struct replay_opts { struct rev_info *revs; /* Private use */ - const char *reflog_message; + struct replay_ctx *ctx; }; #define REPLAY_OPTS_INIT { \ .edit = -1, \ .action = -1, \ .current_fixups = STRBUF_INIT, \ .xopts = STRVEC_INIT, \ + .ctx = replay_ctx_new(), \ } /* From patchwork Thu Apr 18 13:14:07 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phillip Wood X-Patchwork-Id: 13634715 Received: from mail-wm1-f54.google.com (mail-wm1-f54.google.com [209.85.128.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1C9A115AD8F for ; Thu, 18 Apr 2024 13:14:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.54 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713446089; cv=none; b=iSPW1VG4pBGYzhR+xL2VdDh1QFuxQXgLwiCXaxCv0BtGofvQvbcXTh6mTLGxlXWrgogCTHjBkfisbXnZAENM4KmqoPJvsWdAkIlbDE8/s7OQXA92cW0HMcC2KuhV7F2f9Pw42MbwgxB6YlrD2OS6+PVCs4CIqLraqgKMYPjhVtY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713446089; c=relaxed/simple; bh=oBOUTC7ojT9s7Hxkdo0O1M3WiOe6msX6M4gzzMurSPI=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=lur7TNwibtPMyLaOgFvQbqp5wTgirOuR65+R5WGhTJldVUno05TD6jSoAHUP/8F/ThVbw1WYfthFrV+FDAGI02JKERpAFglMH8A3qqDYY4xXduDag2bEFxirfkY5fBN/AOT+5MRrnS+5u+Ge1IzVZywT8aZ4pRZAywVPW+f5+5Q= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=hzSFX7Fk; arc=none smtp.client-ip=209.85.128.54 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="hzSFX7Fk" Received: by mail-wm1-f54.google.com with SMTP id 5b1f17b1804b1-418e06c0ef4so7213105e9.0 for ; Thu, 18 Apr 2024 06:14:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1713446085; x=1714050885; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:reply-to:references :in-reply-to:message-id:date:subject:cc:to:from:from:to:cc:subject :date:message-id:reply-to; bh=JR+k+GZgqEhcKG3byGCxmQbqHTcKkrHPcXc8jGWGxoE=; b=hzSFX7FkBeud1iZ1yaztG6VIDQJPw9EuURv5gV4BLTgvLeQVEuFl6KGhT82l+lEV+0 9x2rub5PkIoD7Lh1KRVeRDChiaboly67q2qk55B2NC/+x3YlCqj/w5o/k270IHevECQd 6+Oadx4pLbjkLmv+lOqoDypWAv+dMBg5XpN67dLJgIF+ZoX/2B/YjQQGWFlGurPheRv5 4jhloK/0iIR0A4QALrKPPSpPX6MhA5D02t+3uUssRQwPEgnF6+wiA3C5gP704/0V0rI0 ldYUHjR7MD397r7vz77LjTxFip3SQAt1yqAO5lBGnpdee7WYUb/npN3oB7yQuGAJO8Ll wdXQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713446085; x=1714050885; h=content-transfer-encoding:mime-version:reply-to:references :in-reply-to:message-id:date:subject:cc:to:from:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=JR+k+GZgqEhcKG3byGCxmQbqHTcKkrHPcXc8jGWGxoE=; b=Z4IrVOo8WLYHWwpINaq0XFP02bE+xug7gHeTYhLGX4moXh9hHTWjAdoB6HqA74J8iu n/TbReuQ4YZeGOlMu9fXoprpprVqQmsHFMI3ee5pQdia08YSuE06lGroTLY/swdVCYAP n1ER6yB5BD73+Tt+NuyISiKTK52CdAZDVbBcztXZy+qZ4ZvZ+Yxcey4Z6L2kUjIaxBEs q3HF9W4+a6WcWOXkZ41Edi23sZoBHBX7DxNlS9Mt4v4AW+8yEr/jHMkcLLkCGyzc1jRV FfnY3Wl2GAjQxco5YeFPKIq5OEsgaNeCNXjels66xNTVqNRI+Ikj99uXdfBP1AGTvm/z QIaQ== X-Gm-Message-State: AOJu0YxRzyomf2jjXBYKVCik58mzi76bmiJg/4RP4g745Rg6wbhuxFz3 U3u1/qztT3ooiTSg0t47DU11qsJ57as7W4fFBTD4Yzfcn5mrSY+0L8uL+g== X-Google-Smtp-Source: AGHT+IEVZMoh9SJaTc4ZXDXh40MIvs5O33crpF9H9f3gl5bGrklC6ZSpnKCEUVMirIW8WzEa6Ucjcg== X-Received: by 2002:a05:600c:4f0e:b0:418:37e1:3f73 with SMTP id l14-20020a05600c4f0e00b0041837e13f73mr2670674wmq.2.1713446085328; Thu, 18 Apr 2024 06:14:45 -0700 (PDT) Received: from localhost.localdomain ([2a0a:ef40:68c:c401:12ba:addc:3daa:a3e]) by smtp.gmail.com with ESMTPSA id r14-20020a05600c458e00b00417e5b71188sm2682748wmo.34.2024.04.18.06.14.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 18 Apr 2024 06:14:44 -0700 (PDT) From: Phillip Wood To: git@vger.kernel.org Cc: David Bimmler , Johannes Schindelin , Phillip Wood Subject: [PATCH 3/5] sequencer: move current fixups to private context Date: Thu, 18 Apr 2024 14:14:07 +0100 Message-ID: <2e4a2168733cb226a6fd660cb0d0bd700a1e7d86.1713445918.git.phillip.wood@dunelm.org.uk> X-Mailer: git-send-email 2.44.0.661.ge68cfcc6c2f In-Reply-To: References: Reply-To: Phillip Wood Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Phillip Wood The list of current fixups is an implementation detail of the sequencer and so it should not be stored in the public options struct. Signed-off-by: Phillip Wood --- sequencer.c | 89 ++++++++++++++++++++++++++++++++++------------------- sequencer.h | 5 --- 2 files changed, 57 insertions(+), 37 deletions(-) diff --git a/sequencer.c b/sequencer.c index 6ddf6a8aa1e..edc49c94cce 100644 --- a/sequencer.c +++ b/sequencer.c @@ -211,17 +211,29 @@ static GIT_PATH_FUNC(rebase_path_keep_redundant_commits, "rebase-merge/keep_redu * A 'struct replay_ctx' represents the private state of the sequencer. */ struct replay_ctx { + /* + * The list of completed fixup and squash commands in the + * current chain. + */ + struct strbuf current_fixups; /* * Stores the reflog message that will be used when creating a * commit. Points to a static buffer and should not be free()'d. */ const char *reflog_message; + /* + * The number of completed fixup and squash commands in the + * current chain. + */ + int current_fixup_count; }; struct replay_ctx* replay_ctx_new(void) { struct replay_ctx *ctx = xcalloc(1, sizeof(*ctx)); + strbuf_init(&ctx->current_fixups, 0); + return ctx; } @@ -384,17 +396,24 @@ static const char *gpg_sign_opt_quoted(struct replay_opts *opts) return buf.buf; } +static void replay_ctx_release(struct replay_ctx *ctx) +{ + strbuf_release(&ctx->current_fixups); +} + void replay_opts_release(struct replay_opts *opts) { + struct replay_ctx *ctx = opts->ctx; + free(opts->gpg_sign); free(opts->reflog_action); free(opts->default_strategy); free(opts->strategy); strvec_clear (&opts->xopts); - strbuf_release(&opts->current_fixups); if (opts->revs) release_revisions(opts->revs); free(opts->revs); + replay_ctx_release(ctx); free(opts->ctx); } @@ -1876,10 +1895,10 @@ static void add_commented_lines(struct strbuf *buf, const void *str, size_t len) } /* Does the current fixup chain contain a squash command? */ -static int seen_squash(struct replay_opts *opts) +static int seen_squash(struct replay_ctx *ctx) { - return starts_with(opts->current_fixups.buf, "squash") || - strstr(opts->current_fixups.buf, "\nsquash"); + return starts_with(ctx->current_fixups.buf, "squash") || + strstr(ctx->current_fixups.buf, "\nsquash"); } static void update_comment_bufs(struct strbuf *buf1, struct strbuf *buf2, int n) @@ -1955,6 +1974,7 @@ static int append_squash_message(struct strbuf *buf, const char *body, enum todo_command command, struct replay_opts *opts, unsigned flag) { + struct replay_ctx *ctx = opts->ctx; const char *fixup_msg; size_t commented_len = 0, fixup_off; /* @@ -1963,21 +1983,21 @@ static int append_squash_message(struct strbuf *buf, const char *body, * squashing commit messages. */ if (starts_with(body, "amend!") || - ((command == TODO_SQUASH || seen_squash(opts)) && + ((command == TODO_SQUASH || seen_squash(ctx)) && (starts_with(body, "squash!") || starts_with(body, "fixup!")))) commented_len = commit_subject_length(body); strbuf_addf(buf, "\n%c ", comment_line_char); strbuf_addf(buf, _(nth_commit_msg_fmt), - ++opts->current_fixup_count + 1); + ++ctx->current_fixup_count + 1); strbuf_addstr(buf, "\n\n"); strbuf_add_commented_lines(buf, body, commented_len, comment_line_char); /* buf->buf may be reallocated so store an offset into the buffer */ fixup_off = buf->len; strbuf_addstr(buf, body + commented_len); /* fixup -C after squash behaves like squash */ - if (is_fixup_flag(command, flag) && !seen_squash(opts)) { + if (is_fixup_flag(command, flag) && !seen_squash(ctx)) { /* * We're replacing the commit message so we need to * append the Signed-off-by: trailer if the user @@ -2011,12 +2031,13 @@ static int update_squash_messages(struct repository *r, struct replay_opts *opts, unsigned flag) { + struct replay_ctx *ctx = opts->ctx; struct strbuf buf = STRBUF_INIT; int res = 0; const char *message, *body; const char *encoding = get_commit_output_encoding(); - if (opts->current_fixup_count > 0) { + if (ctx->current_fixup_count > 0) { struct strbuf header = STRBUF_INIT; char *eol; @@ -2029,10 +2050,10 @@ static int update_squash_messages(struct repository *r, strbuf_addf(&header, "%c ", comment_line_char); strbuf_addf(&header, _(combined_commit_msg_fmt), - opts->current_fixup_count + 2); + ctx->current_fixup_count + 2); strbuf_splice(&buf, 0, eol - buf.buf, header.buf, header.len); strbuf_release(&header); - if (is_fixup_flag(command, flag) && !seen_squash(opts)) + if (is_fixup_flag(command, flag) && !seen_squash(ctx)) update_squash_message_for_fixup(&buf); } else { struct object_id head; @@ -2079,7 +2100,7 @@ static int update_squash_messages(struct repository *r, } else if (command == TODO_FIXUP) { strbuf_addf(&buf, "\n%c ", comment_line_char); strbuf_addf(&buf, _(skip_nth_commit_msg_fmt), - ++opts->current_fixup_count + 1); + ++ctx->current_fixup_count + 1); strbuf_addstr(&buf, "\n\n"); strbuf_add_commented_lines(&buf, body, strlen(body), comment_line_char); @@ -2093,12 +2114,12 @@ static int update_squash_messages(struct repository *r, strbuf_release(&buf); if (!res) { - strbuf_addf(&opts->current_fixups, "%s%s %s", - opts->current_fixups.len ? "\n" : "", + strbuf_addf(&ctx->current_fixups, "%s%s %s", + ctx->current_fixups.len ? "\n" : "", command_to_string(command), oid_to_hex(&commit->object.oid)); - res = write_message(opts->current_fixups.buf, - opts->current_fixups.len, + res = write_message(ctx->current_fixups.buf, + ctx->current_fixups.len, rebase_path_current_fixups(), 0); } @@ -2176,6 +2197,7 @@ static int do_pick_commit(struct repository *r, struct replay_opts *opts, int final_fixup, int *check_todo) { + struct replay_ctx *ctx = opts->ctx; unsigned int flags = should_edit(opts) ? EDIT_MSG : 0; const char *msg_file = should_edit(opts) ? NULL : git_path_merge_msg(r); struct object_id head; @@ -2456,8 +2478,8 @@ static int do_pick_commit(struct repository *r, unlink(rebase_path_fixup_msg()); unlink(rebase_path_squash_msg()); unlink(rebase_path_current_fixups()); - strbuf_reset(&opts->current_fixups); - opts->current_fixup_count = 0; + strbuf_reset(&ctx->current_fixups); + ctx->current_fixup_count = 0; } leave: @@ -3019,6 +3041,8 @@ static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf) static int read_populate_opts(struct replay_opts *opts) { + struct replay_ctx *ctx = opts->ctx; + if (is_rebase_i(opts)) { struct strbuf buf = STRBUF_INIT; int ret = 0; @@ -3078,13 +3102,13 @@ static int read_populate_opts(struct replay_opts *opts) read_strategy_opts(opts, &buf); strbuf_reset(&buf); - if (read_oneliner(&opts->current_fixups, + if (read_oneliner(&ctx->current_fixups, rebase_path_current_fixups(), READ_ONELINER_SKIP_IF_EMPTY)) { - const char *p = opts->current_fixups.buf; - opts->current_fixup_count = 1; + const char *p = ctx->current_fixups.buf; + ctx->current_fixup_count = 1; while ((p = strchr(p, '\n'))) { - opts->current_fixup_count++; + ctx->current_fixup_count++; p++; } } @@ -5066,6 +5090,7 @@ static int commit_staged_changes(struct repository *r, struct replay_opts *opts, struct todo_list *todo_list) { + struct replay_ctx *ctx = opts->ctx; unsigned int flags = ALLOW_EMPTY | EDIT_MSG; unsigned int final_fixup = 0, is_clean; @@ -5102,7 +5127,7 @@ static int commit_staged_changes(struct repository *r, * the commit message and if there was a squash, let the user * edit it. */ - if (!is_clean || !opts->current_fixup_count) + if (!is_clean || !ctx->current_fixup_count) ; /* this is not the final fixup */ else if (!oideq(&head, &to_amend) || !file_exists(rebase_path_stopped_sha())) { @@ -5111,20 +5136,20 @@ static int commit_staged_changes(struct repository *r, unlink(rebase_path_fixup_msg()); unlink(rebase_path_squash_msg()); unlink(rebase_path_current_fixups()); - strbuf_reset(&opts->current_fixups); - opts->current_fixup_count = 0; + strbuf_reset(&ctx->current_fixups); + ctx->current_fixup_count = 0; } } else { /* we are in a fixup/squash chain */ - const char *p = opts->current_fixups.buf; - int len = opts->current_fixups.len; + const char *p = ctx->current_fixups.buf; + int len = ctx->current_fixups.len; - opts->current_fixup_count--; + ctx->current_fixup_count--; if (!len) BUG("Incorrect current_fixups:\n%s", p); while (len && p[len - 1] != '\n') len--; - strbuf_setlen(&opts->current_fixups, len); + strbuf_setlen(&ctx->current_fixups, len); if (write_message(p, len, rebase_path_current_fixups(), 0) < 0) return error(_("could not write file: '%s'"), @@ -5141,7 +5166,7 @@ static int commit_staged_changes(struct repository *r, * actually need to re-commit with a cleaned up commit * message. */ - if (opts->current_fixup_count > 0 && + if (ctx->current_fixup_count > 0 && !is_fixup(peek_command(todo_list, 0))) { final_fixup = 1; /* @@ -5214,14 +5239,14 @@ static int commit_staged_changes(struct repository *r, unlink(rebase_path_fixup_msg()); unlink(rebase_path_squash_msg()); } - if (opts->current_fixup_count > 0) { + if (ctx->current_fixup_count > 0) { /* * Whether final fixup or not, we just cleaned up the commit * message... */ unlink(rebase_path_current_fixups()); - strbuf_reset(&opts->current_fixups); - opts->current_fixup_count = 0; + strbuf_reset(&ctx->current_fixups); + ctx->current_fixup_count = 0; } return 0; } diff --git a/sequencer.h b/sequencer.h index 069f06400d2..c15b9a64020 100644 --- a/sequencer.h +++ b/sequencer.h @@ -69,10 +69,6 @@ struct replay_opts { /* Reflog */ char *reflog_action; - /* Used by fixup/squash */ - struct strbuf current_fixups; - int current_fixup_count; - /* placeholder commit for -i --root */ struct object_id squash_onto; int have_squash_onto; @@ -86,7 +82,6 @@ struct replay_opts { #define REPLAY_OPTS_INIT { \ .edit = -1, \ .action = -1, \ - .current_fixups = STRBUF_INIT, \ .xopts = STRVEC_INIT, \ .ctx = replay_ctx_new(), \ } From patchwork Thu Apr 18 13:14:08 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phillip Wood X-Patchwork-Id: 13634716 Received: from mail-wm1-f48.google.com (mail-wm1-f48.google.com [209.85.128.48]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7FD8F15D5BE for ; Thu, 18 Apr 2024 13:14:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.48 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713446089; cv=none; b=gCfUvgx1rXzCiz00ZNhI0cKR7wh+WFvKZ5oC1w+e6yOMfwBIidakXziSFJSQkNRauIhKojdrOneR5pYXDa9YItqxm1zVlqR06fO/HpXaXrS0UAvkvdOq8cibKvxGlD99uRs9axnGJ2btGJdUmCuSBbFL6AzXUqQput08HbHO7Hw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713446089; c=relaxed/simple; bh=55U2Hjj8HfkmHHjtt7utIolZ0dKAQqp7OiLgq4W7wN8=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=XpAJTmlUkm/PzJxs2lXSXvFKSKkiZ5gchZanMndTNoI1Evo7sYqLIplY1NG+VUOi3s6+xrVHIZmbw0Q1rAQRR2/gmS8qfT/yY5OJzM6FeHcFKaCSn1evhQl//qBB6gw+CHlhYn7YKyGlTAxpwjWu4o596UF1YluyFK6l3/ElS+8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=JQW4TETm; arc=none smtp.client-ip=209.85.128.48 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="JQW4TETm" Received: by mail-wm1-f48.google.com with SMTP id 5b1f17b1804b1-418c2bf2f90so5083915e9.1 for ; Thu, 18 Apr 2024 06:14:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1713446086; x=1714050886; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:reply-to:references :in-reply-to:message-id:date:subject:cc:to:from:from:to:cc:subject :date:message-id:reply-to; bh=1AzTNEc8LT6OXfXlafBYZVhSfw0BtXBFIBWbBfJS+zg=; b=JQW4TETmL1prwWsAlNy+JndzzDyL2EyJahQWjwlDTri1SzwVDwmrDvcbU9f7DiFtjn 4QshFb0Eq/DzwSm5Ioeb0kQYZreFFGEOZp5vtaZHqZekkdidWTQK4prwpl0PGIs0YcEU dQM1FyvnQ1rhMjtszpcEWx6nU0/YkIP1qh1gzhFb8xTK/IPhxB22biMZL4cjXu85Z3cs 2O6+9k3AOmryIc7AXQD6MF8J+WmoQfCxiWDOOZLV7wUoivIBc1Q0lJulhdO5zVbJ/qgW pz+Gv+BofjcrG2QA6oIjoY4/I3W+IAKYl6oR4iJeOPxPA+db5DQUdqBgmDPuJzPbhY++ rPhg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713446086; x=1714050886; h=content-transfer-encoding:mime-version:reply-to:references :in-reply-to:message-id:date:subject:cc:to:from:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=1AzTNEc8LT6OXfXlafBYZVhSfw0BtXBFIBWbBfJS+zg=; b=FmkpJxqnkg6Cdr/d/X6wJ7/Mxbycogl4NNg1WeHUb2XpKtXS88sNwu4awOa5XVQLEI +T6zds1gr8LvGdldu9KWLkUdBb13Yu1dR7/9FsFfhniUPMAzXQrJARuJ/Dj2oYxGxulB SuZDnpLgL5PBZl9wvq/O33RjwVwaI5gnAA0eCIwEzyk8/Ks1E0OBRf1wOFoEJHWbd3Xo dnUxqPAS6ZA/A4VApsvryagL4lZOe+RnoQVlx/+IDJoU11zKNrCZCHI0BWackWM6dVjh d2BF2gPeEI5J6ZlonYOOp3s2CktXltbQZImizxw02yHgovlnDJgYC/pxJKlPop/OIavl ZwmQ== X-Gm-Message-State: AOJu0YxFPbwZKkAKoNTCHd53beLeIKZYFl9vyYYRDwSHLtE2IJaD1NMn xLeiQxOg8x7sbWWBkoTaM+9PhwiZTPxBU9lTcNk6pOTdOWFVaTvsSXtBpQ== X-Google-Smtp-Source: AGHT+IFfvjKDsGj7ebLFX/9FCI3B9x2ZDpwW6+X+h8rGxDcR8eqHM2xKfcfgXqQf7icVwVm7j9xjpg== X-Received: by 2002:a05:600c:4447:b0:418:2ab6:7123 with SMTP id v7-20020a05600c444700b004182ab67123mr1917149wmn.10.1713446085909; Thu, 18 Apr 2024 06:14:45 -0700 (PDT) Received: from localhost.localdomain ([2a0a:ef40:68c:c401:12ba:addc:3daa:a3e]) by smtp.gmail.com with ESMTPSA id r14-20020a05600c458e00b00417e5b71188sm2682748wmo.34.2024.04.18.06.14.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 18 Apr 2024 06:14:45 -0700 (PDT) From: Phillip Wood To: git@vger.kernel.org Cc: David Bimmler , Johannes Schindelin , Phillip Wood Subject: [PATCH 4/5] sequencer: store commit message in private context Date: Thu, 18 Apr 2024 14:14:08 +0100 Message-ID: X-Mailer: git-send-email 2.44.0.661.ge68cfcc6c2f In-Reply-To: References: Reply-To: Phillip Wood Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Phillip Wood Add an strbuf to "struct replay_ctx" to hold the current commit message. This does not change the behavior but it will allow us to fix a bug with "git rebase --signoff" in the next commit. A future patch series will use the changes here to avoid writing the commit message to disc unless there are conflicts or the commit is being reworded. The changes in do_pick_commit() are a mechanical replacement of "msgbuf" with "ctx->message". In do_merge() the code to write commit message to disc is factored out of the conditional now that both branches store the message in the same buffer. Signed-off-by: Phillip Wood --- sequencer.c | 96 ++++++++++++++++++++++++++++------------------------- 1 file changed, 50 insertions(+), 46 deletions(-) diff --git a/sequencer.c b/sequencer.c index edc49c94cce..31e4d3fdec0 100644 --- a/sequencer.c +++ b/sequencer.c @@ -211,6 +211,11 @@ static GIT_PATH_FUNC(rebase_path_keep_redundant_commits, "rebase-merge/keep_redu * A 'struct replay_ctx' represents the private state of the sequencer. */ struct replay_ctx { + /* + * The commit message that will be used except at the end of a + * chain of fixup and squash commands. + */ + struct strbuf message; /* * The list of completed fixup and squash commands in the * current chain. @@ -226,13 +231,18 @@ struct replay_ctx { * current chain. */ int current_fixup_count; + /* + * Whether message contains a commit message. + */ + unsigned have_message :1; }; struct replay_ctx* replay_ctx_new(void) { struct replay_ctx *ctx = xcalloc(1, sizeof(*ctx)); strbuf_init(&ctx->current_fixups, 0); + strbuf_init(&ctx->message, 0); return ctx; } @@ -399,6 +409,7 @@ static const char *gpg_sign_opt_quoted(struct replay_opts *opts) static void replay_ctx_release(struct replay_ctx *ctx) { strbuf_release(&ctx->current_fixups); + strbuf_release(&ctx->message); } void replay_opts_release(struct replay_opts *opts) @@ -2205,7 +2216,6 @@ static int do_pick_commit(struct repository *r, const char *base_label, *next_label; char *author = NULL; struct commit_message msg = { NULL, NULL, NULL, NULL }; - struct strbuf msgbuf = STRBUF_INIT; int res, unborn = 0, reword = 0, allow, drop_commit; enum todo_command command = item->command; struct commit *commit = item->commit; @@ -2304,7 +2314,7 @@ static int do_pick_commit(struct repository *r, next = parent; next_label = msg.parent_label; if (opts->commit_use_reference) { - strbuf_addstr(&msgbuf, + strbuf_addstr(&ctx->message, "# *** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***"); } else if (skip_prefix(msg.subject, "Revert \"", &orig_subject) && /* @@ -2313,21 +2323,21 @@ static int do_pick_commit(struct repository *r, * thus requiring excessive complexity to deal with. */ !starts_with(orig_subject, "Revert \"")) { - strbuf_addstr(&msgbuf, "Reapply \""); - strbuf_addstr(&msgbuf, orig_subject); + strbuf_addstr(&ctx->message, "Reapply \""); + strbuf_addstr(&ctx->message, orig_subject); } else { - strbuf_addstr(&msgbuf, "Revert \""); - strbuf_addstr(&msgbuf, msg.subject); - strbuf_addstr(&msgbuf, "\""); + strbuf_addstr(&ctx->message, "Revert \""); + strbuf_addstr(&ctx->message, msg.subject); + strbuf_addstr(&ctx->message, "\""); } - strbuf_addstr(&msgbuf, "\n\nThis reverts commit "); - refer_to_commit(opts, &msgbuf, commit); + strbuf_addstr(&ctx->message, "\n\nThis reverts commit "); + refer_to_commit(opts, &ctx->message, commit); if (commit->parents && commit->parents->next) { - strbuf_addstr(&msgbuf, ", reversing\nchanges made to "); - refer_to_commit(opts, &msgbuf, parent); + strbuf_addstr(&ctx->message, ", reversing\nchanges made to "); + refer_to_commit(opts, &ctx->message, parent); } - strbuf_addstr(&msgbuf, ".\n"); + strbuf_addstr(&ctx->message, ".\n"); } else { const char *p; @@ -2336,21 +2346,22 @@ static int do_pick_commit(struct repository *r, next = commit; next_label = msg.label; - /* Append the commit log message to msgbuf. */ + /* Append the commit log message to ctx->message. */ if (find_commit_subject(msg.message, &p)) - strbuf_addstr(&msgbuf, p); + strbuf_addstr(&ctx->message, p); if (opts->record_origin) { - strbuf_complete_line(&msgbuf); - if (!has_conforming_footer(&msgbuf, NULL, 0)) - strbuf_addch(&msgbuf, '\n'); - strbuf_addstr(&msgbuf, cherry_picked_prefix); - strbuf_addstr(&msgbuf, oid_to_hex(&commit->object.oid)); - strbuf_addstr(&msgbuf, ")\n"); + strbuf_complete_line(&ctx->message); + if (!has_conforming_footer(&ctx->message, NULL, 0)) + strbuf_addch(&ctx->message, '\n'); + strbuf_addstr(&ctx->message, cherry_picked_prefix); + strbuf_addstr(&ctx->message, oid_to_hex(&commit->object.oid)); + strbuf_addstr(&ctx->message, ")\n"); } if (!is_fixup(command)) author = get_author(msg.message); } + ctx->have_message = 1; if (command == TODO_REWORD) reword = 1; @@ -2381,7 +2392,7 @@ static int do_pick_commit(struct repository *r, } if (opts->signoff && !is_fixup(command)) - append_signoff(&msgbuf, 0, 0); + append_signoff(&ctx->message, 0, 0); if (is_rebase_i(opts) && write_author_script(msg.message) < 0) res = -1; @@ -2390,17 +2401,17 @@ static int do_pick_commit(struct repository *r, !strcmp(opts->strategy, "ort") || command == TODO_REVERT) { res = do_recursive_merge(r, base, next, base_label, next_label, - &head, &msgbuf, opts); + &head, &ctx->message, opts); if (res < 0) goto leave; - res |= write_message(msgbuf.buf, msgbuf.len, + res |= write_message(ctx->message.buf, ctx->message.len, git_path_merge_msg(r), 0); } else { struct commit_list *common = NULL; struct commit_list *remotes = NULL; - res = write_message(msgbuf.buf, msgbuf.len, + res = write_message(ctx->message.buf, ctx->message.len, git_path_merge_msg(r), 0); commit_list_insert(base, &common); @@ -2485,7 +2496,6 @@ static int do_pick_commit(struct repository *r, leave: free_message(commit, &msg); free(author); - strbuf_release(&msgbuf); update_abort_safety_file(); return res; @@ -3952,6 +3962,7 @@ static int do_merge(struct repository *r, const char *arg, int arg_len, int flags, int *check_todo, struct replay_opts *opts) { + struct replay_ctx *ctx = opts->ctx; int run_commit_flags = 0; struct strbuf ref_name = STRBUF_INIT; struct commit *head_commit, *merge_commit, *i; @@ -4080,40 +4091,31 @@ static int do_merge(struct repository *r, write_author_script(message); find_commit_subject(message, &body); len = strlen(body); - ret = write_message(body, len, git_path_merge_msg(r), 0); + strbuf_add(&ctx->message, body, len); repo_unuse_commit_buffer(r, commit, message); - if (ret) { - error_errno(_("could not write '%s'"), - git_path_merge_msg(r)); - goto leave_merge; - } } else { struct strbuf buf = STRBUF_INIT; - int len; strbuf_addf(&buf, "author %s", git_author_info(0)); write_author_script(buf.buf); - strbuf_reset(&buf); + strbuf_release(&buf); if (oneline_offset < arg_len) { - p = arg + oneline_offset; - len = arg_len - oneline_offset; + strbuf_add(&ctx->message, arg + oneline_offset, + arg_len - oneline_offset); } else { - strbuf_addf(&buf, "Merge %s '%.*s'", + strbuf_addf(&ctx->message, "Merge %s '%.*s'", to_merge->next ? "branches" : "branch", merge_arg_len, arg); - p = buf.buf; - len = buf.len; - } - - ret = write_message(p, len, git_path_merge_msg(r), 0); - strbuf_release(&buf); - if (ret) { - error_errno(_("could not write '%s'"), - git_path_merge_msg(r)); - goto leave_merge; } } + ctx->have_message = 1; + if (write_message(ctx->message.buf, ctx->message.len, + git_path_merge_msg(r), 0)) { + ret = error_errno(_("could not write '%s'"), + git_path_merge_msg(r)); + goto leave_merge; + } if (strategy || to_merge->next) { /* Octopus merge */ @@ -4885,6 +4887,8 @@ static int pick_commits(struct repository *r, return stopped_at_head(r); } } + strbuf_reset(&ctx->message); + ctx->have_message = 0; if (item->command <= TODO_SQUASH) { res = pick_one_commit(r, todo_list, opts, &check_todo, &reschedule); From patchwork Thu Apr 18 13:14:09 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phillip Wood X-Patchwork-Id: 13634717 Received: from mail-wm1-f41.google.com (mail-wm1-f41.google.com [209.85.128.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2E5F2146005 for ; Thu, 18 Apr 2024 13:14:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713446093; cv=none; b=tQ3HWtLfL+bUWZ0rxkmdYG5vrFyuN6DdazBAqdmwpjEU/rOT6duuAv/021/pMUFe2nJZIHl/n491IkwntyOpbCjrhY4q1JgmuXdxNRzN/9ox+StbDBNJ6wUqPCSzLmFtAFq/ngW3rvT6LGWgjlxYNBvqT4/IRjm7Ci66zd4BlTs= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713446093; c=relaxed/simple; bh=ttj4l1X72q09vwRr4GBizah96tWp7Ua6fl6M2SVz//0=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=p7Kc7X4qGeUoIrfY8YpgoAP73gt2R6zs+tXKXYY/Tq/OrAx5/0XuJoVQ+YpTi0p4TQMCiyEJOON4aa7uE6J4dLebPiIpkQjIfIuWrbLYk6kWI2t2pW3rmCHVzstY4yxL9hqJCeWHrhy7MlOm9OHTh0sBo/SFsZZLHoe0zn/dRk8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=dcfu3Kpz; arc=none smtp.client-ip=209.85.128.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="dcfu3Kpz" Received: by mail-wm1-f41.google.com with SMTP id 5b1f17b1804b1-418c2bf2f90so5083965e9.1 for ; Thu, 18 Apr 2024 06:14:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1713446086; x=1714050886; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:reply-to:references :in-reply-to:message-id:date:subject:cc:to:from:from:to:cc:subject :date:message-id:reply-to; bh=DSVQvKsZHXk+fZ/19krXJCX2iuhfxUSQ53OG1rer458=; b=dcfu3KpzhEQeRwar8VRqFwmYbIF1AfTv1FN3CnXMaz6tuJ2KyLnIFhsSNH0MBu2DuS +8J66m+VbuYxP5K+SDWsS8cLlwfbLGMgox5rxNQlRQ/gKEyDHykHO4u2+9SORxJnpi05 YXoK/YIBRudVRptRJguOnPCXYw2CNr57fUL3Tjv0k6eOOkZQAYWASjHOu9tsQSO1JsE6 vKMIA6typLrhnJ9WLP7JXtqz+9rUS4t2+HA845//04gNaoSrfWkDfEhFy0s2xkAY/i8d XkLoFvTr1bunq1f52UBy/BfI9WLgEZ/aRmfAzqIr10wrEpeiMnFoPbXtmWZLUT9RQw+P Zeww== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713446086; x=1714050886; h=content-transfer-encoding:mime-version:reply-to:references :in-reply-to:message-id:date:subject:cc:to:from:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=DSVQvKsZHXk+fZ/19krXJCX2iuhfxUSQ53OG1rer458=; b=aO6rrzkT5P7Q+NdEqiTsAhwiDc/gjXt8NyCA4Ykjggg82ahwWbWDwlrve5e1oEOHiT HjfGM6B0zMTJjGZaKKpbvn3Kiq//bM0ph/N55e4tBO3eBfZl5cZm3QFD8DkHVoOUEgbF Hpn7ZVZqTD29+KEGjk4EdJSqLB7yUdDcWA0nf/Jmq7yc9SykRnPYuaj/R1oO8UDMRrPv qNvs6h8A4zyKQvPdUdBGJnjwwhci14meF+pU3vqcbk9nlqZNgJW2Udnq1iiC0Uo8UnID RF1pWtujGHOlbEoSYoPR+FORlQLeS8z388hajq7yL2YdVQZJ1UfydD9GcxLffZJwGm9V zuDw== X-Gm-Message-State: AOJu0YyZvXBOE4MC0D+Ks7RJqekoIm+Y1+HZR6otF8geQlUjX+QLQe87 T7gZfGBg7GhJ5M88O0fbiMVfZjQNj95uIUkHlBWxcEmCRsrKn9HKB7Tnuw== X-Google-Smtp-Source: AGHT+IFEPRPeNcSqcrXuwZZmtUrrxCpOsQDB06vFoMy8lipXkVGEUaGtGBeTGgvxqPPEKp5f/Es1Fg== X-Received: by 2002:a05:600c:4ec8:b0:418:bdcd:e59b with SMTP id g8-20020a05600c4ec800b00418bdcde59bmr1815217wmq.7.1713446086484; Thu, 18 Apr 2024 06:14:46 -0700 (PDT) Received: from localhost.localdomain ([2a0a:ef40:68c:c401:12ba:addc:3daa:a3e]) by smtp.gmail.com with ESMTPSA id r14-20020a05600c458e00b00417e5b71188sm2682748wmo.34.2024.04.18.06.14.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 18 Apr 2024 06:14:46 -0700 (PDT) From: Phillip Wood To: git@vger.kernel.org Cc: David Bimmler , Johannes Schindelin , Phillip Wood Subject: [PATCH 5/5] rebase -m: fix --signoff with conflicts Date: Thu, 18 Apr 2024 14:14:09 +0100 Message-ID: <4c8f8843780f3ac23262f1e45a5000d183adca6b.1713445918.git.phillip.wood@dunelm.org.uk> X-Mailer: git-send-email 2.44.0.661.ge68cfcc6c2f In-Reply-To: References: Reply-To: Phillip Wood Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Phillip Wood When rebasing with "--signoff" the commit created by "rebase --continue" after resolving conflicts or editing a commit fails to add the "Signed-off-by:" trailer. This happens because the message from the original commit is reused instead of the one that would have been used if the sequencer had not stopped for the user interaction. The correct message is stored in ctx->message and so with a couple of exceptions this is written to rebase_path_message() when stopping for user interaction instead. The exceptions are (i) "fixup" and "squash" commands where the file is written by error_failed_squash() and (ii) "edit" commands that are fast-forwarded where the original message is still reused. The latter is safe because "--signoff" will never fast-forward. Note this introduces a change in behavior as the message file now contains conflict comments. This is safe because commit_staged_changes() passes an explicit cleanup flag when not editing the message and when the message is being edited it will be cleaned up automatically. This means user now sees the same message comments in editor with "rebase --continue" as they would if they ran "git commit" themselves before continuing the rebase. It also matches the behavior of "git cherry-pick", "git merge" etc. which all list the files with merge conflicts. The tests are extended to check that all commits made after continuing a rebase have a "Signed-off-by:" trailer. Sadly there are a couple of leaks in apply.c which I've not been able to track down that mean this test file is no-longer leak free when testing "git rebase --apply --signoff" with conflicts. Reported-by: David Bimmler Signed-off-by: Phillip Wood --- sequencer.c | 23 +++++++--- t/t3428-rebase-signoff.sh | 96 ++++++++++++++++++++++++++++++++------- t/t3434-rebase-i18n.sh | 2 +- 3 files changed, 97 insertions(+), 24 deletions(-) diff --git a/sequencer.c b/sequencer.c index 31e4d3fdec0..e29a01944be 100644 --- a/sequencer.c +++ b/sequencer.c @@ -3636,13 +3636,24 @@ static int error_with_patch(struct repository *r, struct replay_opts *opts, int exit_code, int to_amend) { - if (commit) { - if (make_patch(r, commit, opts)) + struct replay_ctx *ctx = opts->ctx; + + /* + * Write the commit message to be used by "git rebase + * --continue". If a "fixup" or "squash" command has conflicts + * then we will have already written rebase_path_message() in + * error_failed_squash(). If an "edit" command was + * fast-forwarded then we don't have a message in ctx->message + * and rely on make_patch() to write rebase_path_message() + * instead. + */ + if (ctx->have_message && !file_exists(rebase_path_message()) && + write_message(ctx->message.buf, ctx->message.len, + rebase_path_message(), 0)) + return error(_("could not write commit message file")); + + if (commit && make_patch(r, commit, opts)) return -1; - } else if (copy_file(rebase_path_message(), - git_path_merge_msg(r), 0666)) - return error(_("unable to copy '%s' to '%s'"), - git_path_merge_msg(r), rebase_path_message()); if (to_amend) { if (intend_to_amend()) diff --git a/t/t3428-rebase-signoff.sh b/t/t3428-rebase-signoff.sh index 1bebd1ce74a..6f57aed9fac 100755 --- a/t/t3428-rebase-signoff.sh +++ b/t/t3428-rebase-signoff.sh @@ -5,12 +5,17 @@ test_description='git rebase --signoff This test runs git rebase --signoff and make sure that it works. ' -TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh +. "$TEST_DIRECTORY"/lib-rebase.sh test_expect_success 'setup' ' git commit --allow-empty -m "Initial empty commit" && test_commit first file a && + test_commit second file && + git checkout -b conflict-branch first && + test_commit file-2 file-2 && + test_commit conflict file && + test_commit third file && ident="$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" && @@ -28,6 +33,22 @@ test_expect_success 'setup' ' Signed-off-by: $ident EOF + # Expected commit message after conflict resolution for rebase --signoff + cat >expected-signed-conflict <<-EOF && + third + + Signed-off-by: $ident + + conflict + + Signed-off-by: $ident + + file-2 + + Signed-off-by: $ident + + EOF + # Expected commit message after rebase without --signoff (or with --no-signoff) cat >expected-unsigned <<-EOF && first @@ -39,8 +60,12 @@ test_expect_success 'setup' ' # We configure an alias to do the rebase --signoff so that # on the next subtest we can show that --no-signoff overrides the alias test_expect_success 'rebase --apply --signoff adds a sign-off line' ' - git rbs --apply HEAD^ && - test_commit_message HEAD expected-signed + test_must_fail git rbs --apply second third && + git checkout --theirs file && + git add file && + git rebase --continue && + git log --format=%B -n3 >actual && + test_cmp expected-signed-conflict actual ' test_expect_success 'rebase --no-signoff does not add a sign-off line' ' @@ -51,28 +76,65 @@ test_expect_success 'rebase --no-signoff does not add a sign-off line' ' test_expect_success 'rebase --exec --signoff adds a sign-off line' ' test_when_finished "rm exec" && - git commit --amend -m "first" && - git rebase --exec "touch exec" --signoff HEAD^ && + git rebase --exec "touch exec" --signoff first^ first && test_path_is_file exec && test_commit_message HEAD expected-signed ' test_expect_success 'rebase --root --signoff adds a sign-off line' ' - git commit --amend -m "first" && + git checkout first && git rebase --root --keep-empty --signoff && test_commit_message HEAD^ expected-initial-signed && test_commit_message HEAD expected-signed ' -test_expect_success 'rebase -i --signoff fails' ' - git commit --amend -m "first" && - git rebase -i --signoff HEAD^ && - test_commit_message HEAD expected-signed -' - -test_expect_success 'rebase -m --signoff fails' ' - git commit --amend -m "first" && - git rebase -m --signoff HEAD^ && - test_commit_message HEAD expected-signed -' +test_expect_success 'rebase -m --signoff adds a sign-off line' ' + test_must_fail git rebase -m --signoff second third && + git checkout --theirs file && + git add file && + GIT_EDITOR="sed -n /Conflicts:/,/^\\\$/p >actual" \ + git rebase --continue && + cat >expect <<-\EOF && + # Conflicts: + # file + + EOF + test_cmp expect actual && + git log --format=%B -n3 >actual && + test_cmp expected-signed-conflict actual +' + +test_expect_success 'rebase -i --signoff adds a sign-off line when editing commit' ' + ( + set_fake_editor && + FAKE_LINES="edit 1 edit 3 edit 2" \ + git rebase -i --signoff first third + ) && + echo a >a && + git add a && + test_must_fail git rebase --continue && + git checkout --ours file && + echo b >a && + git add a file && + git rebase --continue && + echo c >a && + git add a && + git log --format=%B -n3 >actual && + cat >expect <<-EOF && + conflict + + Signed-off-by: $ident + + third + + Signed-off-by: $ident + + file-2 + + Signed-off-by: $ident + + EOF + test_cmp expect actual +' + test_done diff --git a/t/t3434-rebase-i18n.sh b/t/t3434-rebase-i18n.sh index e6fef696bbc..a4e482d2cd0 100755 --- a/t/t3434-rebase-i18n.sh +++ b/t/t3434-rebase-i18n.sh @@ -71,7 +71,7 @@ test_rebase_continue_update_encode () { git config i18n.commitencoding $new && test_must_fail git rebase -m main && test -f .git/rebase-merge/message && - git stripspace <.git/rebase-merge/message >two.t && + git stripspace -s <.git/rebase-merge/message >two.t && git add two.t && git rebase --continue && compare_msg $msgfile $old $new &&