From patchwork Tue Feb 6 05:34:41 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13546686 Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com [66.111.4.25]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 38FF278B78 for ; Tue, 6 Feb 2024 05:34:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=66.111.4.25 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1707197687; cv=none; b=FTREpd1JmMdDhqIjMIY3J+zHte7jVxrxXw+lObKD7wHwZoMew2l0wvFyrv8I8ZYdyEPsO5QEtfQPVmgk6Yx1JH7l8E+MO5A8JMQzawnC1RgDaV4ELVADkUc+laF8hWaoViSQvIoQlJPJww/f1j7+RDQPhSjzOpkJlTKE2VysDVg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1707197687; c=relaxed/simple; bh=5bqDkpdnZgXgYrKO5+h3Ys66pMW0rflvUzo/JYUeV2c=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=LWkilqLj6EepcGsRgAjkUBl80PIMUxU0N1BJMJzkrwHVHCjo/Ak7pI/yWfwaRrwo/n6HiD9PqkOBAvn/kKkXdvNn8bxpbiLULo3Qhrgi1zEOGB4sSYbyjNRg/NItcJqtDyXF+mBIMu8l7I9QBxudWeGP9VmSoIiInf5rkFBe+cc= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=SrFhq2Hv; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=wpCh2Th6; arc=none smtp.client-ip=66.111.4.25 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="SrFhq2Hv"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="wpCh2Th6" Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 585B55C0081; Tue, 6 Feb 2024 00:34:44 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Tue, 06 Feb 2024 00:34:44 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm3; t=1707197684; x=1707284084; bh=kMImrr5Vgw Ymv9IhDvW1sC7q5IhBCigeuVfq5sns2X4=; b=SrFhq2HvxzBDWP1PtswIIdIhqA CZwUutolJH7RuOPukfH0a0yTmJXk3ib5h3LnAZpNl7tqoPw9j6uu0LonNJPD+32W nsi2yKRr6lekpZQFFS1oTx+ZM4qg2V6RHIozabm8zUJMiVmVt1WHgLAt3QK+vqDr rcG8zphXcU58MA0NiQbp0j4h/DFMVUz3svLnxLMSQGtN+/n6osJ1FDfZMJfPJouE A1XNKz6wFdoYxqoImKXng5jdxqcVRq7cOdyBLqI9rpmSXzA+Vf5bUb0Oh84MAegS q45+J0RcRPokIWYThl420cZ3RbFGAabbgmcaqQDTPQ3dpMgLloPWpYyKFx9w== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1707197684; x=1707284084; bh=kMImrr5VgwYmv9IhDvW1sC7q5IhB CigeuVfq5sns2X4=; b=wpCh2Th6z67/nRX66T8G30ZKJ7juEG7R68cUYSv9OGc8 4GCeWY6tAkJvhYWK3xBVrOsnEw6qhCwsnjHQVfKaV0igUzUKGHTDGdiAYzThgzsZ P+RE8zI6tV71Kj0L7V1xgLk7hXqsXf+uqL/H77SRfL21/XvjYOT4ziKaysLtsFSM b2pwycViIlUhvb4d+VstUvduS5M8Rx/wveisMYd8o7kfolBk/qnoJ+xTaQC/wYay m1fTONvmTy/hRXUfV6OqK933K23pyW8SbNJTVBakJEafcwM+WmYx1f0ALsBGRLrO 5CjvcFF5LqpKqMRnfIq8Gh4c6NuBDwwSs2a7fv1CvA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrfedvvddgkeejucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvfevuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhr ihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvg hrnhepueektdevtdffveeljeetgfehheeigeekleduvdeffeeghefgledttdehjeelffet necuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepphhsse hpkhhsrdhimh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 6 Feb 2024 00:34:43 -0500 (EST) Received: by vm-mail (OpenSMTPD) with ESMTPSA id d309408a (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Tue, 6 Feb 2024 05:31:13 +0000 (UTC) Date: Tue, 6 Feb 2024 06:34:41 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Cc: moti sd , =?iso-8859-1?q?Rub=E9n?= Justo , Junio C Hamano Subject: [PATCH v2] builtin/stash: report failure to write to index Message-ID: References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: The git-stash(1) command needs to write to the index for many of its operations. When the index is locked by a concurrent writer it will thus fail to operate, which is expected. What is not expected though is that we do not print any error message at all in this case. The user can thus easily miss the fact that the command didn't do what they expected it to do and would be left wondering why that is. Fix this bug and report failures to write to the index. Add tests for the subcommands which hit the respective code paths. While at it, unify error messages when writing to the index fails. The chosen error message is already used in "builtin/stash.c". Reported-by: moti sd Signed-off-by: Patrick Steinhardt --- Range-diff against v1: 1: f114688eac < -: ---------- drop dot 2: b13a5a10ac ! 1: cb098cf88c builtin/stash: report failure to write to index @@ Commit message Fix this bug and report failures to write to the index. Add tests for the subcommands which hit the respective code paths. - Note that the chosen error message ("Cannot write to the index") does - not match our guidelines as it starts with a capitalized letter. This is - intentional though and matches the style of all the other messages used - in git-stash(1). + While at it, unify error messages when writing to the index fails. The + chosen error message is already used in "builtin/stash.c". Reported-by: moti sd Signed-off-by: Patrick Steinhardt ## builtin/stash.c ## +@@ builtin/stash.c: static void unstage_changes_unless_new(struct object_id *orig_tree) + repo_hold_locked_index(the_repository, &lock, LOCK_DIE_ON_ERROR); + if (write_locked_index(&the_index, &lock, + COMMIT_LOCK | SKIP_IF_UNCHANGED)) +- die(_("Unable to write index.")); ++ die(_("could not write index")); + } + + static int do_apply_stash(const char *prefix, struct stash_info *info, @@ builtin/stash.c: static int do_apply_stash(const char *prefix, struct stash_info *info, repo_read_index_preload(the_repository, NULL, 0); if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0, NULL, NULL, NULL)) - return -1; -+ return error(_("Cannot write to the index")); ++ return error(_("could not write index")); if (write_index_as_tree(&c_tree, &the_index, get_index_file(), 0, NULL)) @@ builtin/stash.c: static int do_create_stash(const struct pathspec *ps, struct st if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0, NULL, NULL, NULL) < 0) { - ret = -1; -+ ret = error(_("Cannot write to the index")); ++ ret = error(_("could not write index")); goto done; } @@ builtin/stash.c: static int do_push_stash(const struct pathspec *ps, const char if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0, NULL, NULL, NULL)) { - ret = -1; -+ ret = error(_("Cannot write to the index")); ++ ret = error(_("could not write index")); goto done; } @@ t/t3903-stash.sh: test_expect_success 'restore untracked files even when we hit + touch .git/index.lock && + + cat >expect <<-EOF && -+ error: Cannot write to the index ++ error: could not write index + EOF + test_must_fail git stash create 2>err && + test_cmp expect err @@ t/t3903-stash.sh: test_expect_success 'restore untracked files even when we hit + touch .git/index.lock && + + cat >expect <<-EOF && -+ error: Cannot write to the index ++ error: could not write index + EOF + test_must_fail git stash push 2>err && + test_cmp expect err @@ t/t3903-stash.sh: test_expect_success 'restore untracked files even when we hit + touch .git/index.lock && + + cat >expect <<-EOF && -+ error: Cannot write to the index ++ error: could not write index + EOF + test_must_fail git stash apply 2>err && + test_cmp expect err builtin/stash.c | 8 ++++---- t/t3903-stash.sh | 52 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/builtin/stash.c b/builtin/stash.c index 26489b76c0..d65cd20ee6 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -520,7 +520,7 @@ static void unstage_changes_unless_new(struct object_id *orig_tree) repo_hold_locked_index(the_repository, &lock, LOCK_DIE_ON_ERROR); if (write_locked_index(&the_index, &lock, COMMIT_LOCK | SKIP_IF_UNCHANGED)) - die(_("Unable to write index.")); + die(_("could not write index")); } static int do_apply_stash(const char *prefix, struct stash_info *info, @@ -537,7 +537,7 @@ static int do_apply_stash(const char *prefix, struct stash_info *info, repo_read_index_preload(the_repository, NULL, 0); if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0, NULL, NULL, NULL)) - return -1; + return error(_("could not write index")); if (write_index_as_tree(&c_tree, &the_index, get_index_file(), 0, NULL)) @@ -1364,7 +1364,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b repo_read_index_preload(the_repository, NULL, 0); if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0, NULL, NULL, NULL) < 0) { - ret = -1; + ret = error(_("could not write index")); goto done; } @@ -1555,7 +1555,7 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q if (repo_refresh_and_write_index(the_repository, REFRESH_QUIET, 0, 0, NULL, NULL, NULL)) { - ret = -1; + ret = error(_("could not write index")); goto done; } diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh index 3319240515..00db82fb24 100755 --- a/t/t3903-stash.sh +++ b/t/t3903-stash.sh @@ -1516,4 +1516,56 @@ test_expect_success 'restore untracked files even when we hit conflicts' ' ) ' +test_expect_success 'stash create reports a locked index' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit A A.file && + echo change >A.file && + touch .git/index.lock && + + cat >expect <<-EOF && + error: could not write index + EOF + test_must_fail git stash create 2>err && + test_cmp expect err + ) +' + +test_expect_success 'stash push reports a locked index' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit A A.file && + echo change >A.file && + touch .git/index.lock && + + cat >expect <<-EOF && + error: could not write index + EOF + test_must_fail git stash push 2>err && + test_cmp expect err + ) +' + +test_expect_success 'stash apply reports a locked index' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit A A.file && + echo change >A.file && + git stash push && + touch .git/index.lock && + + cat >expect <<-EOF && + error: could not write index + EOF + test_must_fail git stash apply 2>err && + test_cmp expect err + ) +' + test_done