git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"David Turner" <novalis@novalis.org>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v2 0/2] commit: fix duplication regression in permission error output
Date: Tue, 12 Oct 2021 16:30:47 +0200	[thread overview]
Message-ID: <cover-v2-0.2-00000000000-20211012T142950Z-avarab@gmail.com> (raw)
In-Reply-To: <patch-2.2-56b20f6024b-20211009T133354Z-avarab@gmail.com>

A v2 of a series that fixes duplicate permission output whe we write
trees. Before:

    $ git commit -m"add another one"
    error: insufficient permission for adding an object to repository database .git/objects
    error: insufficient permission for adding an object to repository database .git/objects
    error: Error building trees

After we only emit that "insufficient permission" line once.

The only change since v1 is to make 1/2 and 2/2 flip a
"test_expect_failure" to a "test_expect_success".

Ævar Arnfjörð Bjarmason (2):
  unwritable tests: assert exact error output
  commit: fix duplication regression in permission error output

 cache-tree.c          |  5 +++--
 cache.h               |  1 +
 object-file.c         | 20 ++++++++++--------
 object-store.h        | 10 +++++++--
 t/t0004-unwritable.sh | 47 +++++++++++++++++++++++++++++++++++++++----
 5 files changed, 67 insertions(+), 16 deletions(-)

Range-diff against v1:
1:  a5ef8ea47f4 ! 1:  74bc5568c88 unwritable tests: assert exact error output
    @@ Commit message
         of these error messages twice, let's assert what the output from "git
         commit" and friends is now in the case of permission errors.
     
    +    As noted in [1] using test_expect_failure to mark up a TODO test has
    +    some unexpected edge cases, e.g. we don't want to break --run=3 by
    +    skipping the "test_lazy_prereq" here. This pattern allows us to test
    +    just the test_cmp (and the "cat", which shouldn't fail) with the added
    +    "test_expect_failure", we'll flip that to a "test_expect_success" in
    +    the next commit.
    +
    +    1. https://lore.kernel.org/git/87tuhmk19c.fsf@evledraar.gmail.com/T/#u
    +
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## t/t0004-unwritable.sh ##
    @@ t/t0004-unwritable.sh: test_expect_success setup '
      	test_when_finished "chmod 775 .git/objects .git/objects/??" &&
      	chmod a-w .git/objects .git/objects/?? &&
     -	test_must_fail git write-tree
    ++	test_must_fail git write-tree 2>out.write-tree
    ++'
     +
    ++test_lazy_prereq WRITE_TREE_OUT 'test -e "$TRASH_DIRECTORY"/out.write-tree'
    ++test_expect_success WRITE_TREE_OUT 'write-tree output on unwritable repository' '
     +	cat >expect <<-\EOF &&
     +	error: insufficient permission for adding an object to repository database .git/objects
     +	fatal: git-write-tree: error building trees
     +	EOF
    -+	test_must_fail git write-tree 2>actual &&
    -+	test_cmp expect actual
    ++	test_cmp expect out.write-tree
      '
      
    - test_expect_success POSIXPERM,SANITY 'commit should notice unwritable repository' '
    + test_expect_success POSIXPERM,SANITY,!SANITIZE_LEAK 'commit should notice unwritable repository' '
      	test_when_finished "chmod 775 .git/objects .git/objects/??" &&
      	chmod a-w .git/objects .git/objects/?? &&
     -	test_must_fail git commit -m second
    ++	test_must_fail git commit -m second 2>out.commit
    ++'
     +
    ++test_lazy_prereq COMMIT_OUT 'test -e "$TRASH_DIRECTORY"/out.commit'
    ++test_expect_failure COMMIT_OUT 'commit output on unwritable repository' '
     +	cat >expect <<-\EOF &&
     +	error: insufficient permission for adding an object to repository database .git/objects
    -+	error: insufficient permission for adding an object to repository database .git/objects
     +	error: Error building trees
     +	EOF
    -+	test_must_fail git commit -m second 2>actual &&
    -+	test_cmp expect actual
    ++	test_cmp expect out.commit
      '
      
      test_expect_success POSIXPERM,SANITY 'update-index should notice unwritable repository' '
    @@ t/t0004-unwritable.sh: test_expect_success setup '
      	echo 6O >file &&
      	chmod a-w .git/objects .git/objects/?? &&
     -	test_must_fail git update-index file
    ++	test_must_fail git update-index file 2>out.update-index
    ++'
     +
    ++test_lazy_prereq UPDATE_INDEX_OUT 'test -e "$TRASH_DIRECTORY"/out.update-index'
    ++test_expect_success UPDATE_INDEX_OUT 'update-index output on unwritable repository' '
     +	cat >expect <<-\EOF &&
     +	error: insufficient permission for adding an object to repository database .git/objects
     +	error: file: failed to insert into database
     +	fatal: Unable to process path file
     +	EOF
    -+	test_must_fail git update-index file 2>actual &&
    -+	test_cmp expect actual
    ++	test_cmp expect out.update-index
      '
      
      test_expect_success POSIXPERM,SANITY 'add should notice unwritable repository' '
    @@ t/t0004-unwritable.sh: test_expect_success setup '
      	echo b >file &&
      	chmod a-w .git/objects .git/objects/?? &&
     -	test_must_fail git add file
    ++	test_must_fail git add file 2>out.add
    ++'
     +
    ++test_lazy_prereq ADD_OUT 'test -e "$TRASH_DIRECTORY"/out.add'
    ++test_expect_success ADD_OUT 'add output on unwritable repository' '
     +	cat >expect <<-\EOF &&
     +	error: insufficient permission for adding an object to repository database .git/objects
     +	error: file: failed to insert into database
     +	error: unable to index file '\''file'\''
     +	fatal: updating files failed
     +	EOF
    -+	test_must_fail git add file 2>actual &&
    -+	test_cmp expect actual
    ++	test_cmp expect out.add
      '
      
      test_done
2:  56b20f6024b ! 2:  e6cd47355d5 commit: fix duplication regression in permission error output
    @@ object-store.h: int hash_object_file(const struct git_hash_algo *algo, const voi
      			       const char *type, struct object_id *oid,
     
      ## t/t0004-unwritable.sh ##
    -@@ t/t0004-unwritable.sh: test_expect_success POSIXPERM,SANITY 'commit should notice unwritable repository
    +@@ t/t0004-unwritable.sh: test_expect_success POSIXPERM,SANITY,!SANITIZE_LEAK 'commit should notice unwrit
    + '
      
    + test_lazy_prereq COMMIT_OUT 'test -e "$TRASH_DIRECTORY"/out.commit'
    +-test_expect_failure COMMIT_OUT 'commit output on unwritable repository' '
    ++test_expect_success COMMIT_OUT 'commit output on unwritable repository' '
      	cat >expect <<-\EOF &&
      	error: insufficient permission for adding an object to repository database .git/objects
    --	error: insufficient permission for adding an object to repository database .git/objects
      	error: Error building trees
    - 	EOF
    - 	test_must_fail git commit -m second 2>actual &&
-- 
2.33.0.1567.g7b23ce7ed9e


  parent reply	other threads:[~2021-10-12 14:30 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-09 13:34 [PATCH 1/2] unwritable tests: assert exact error output Ævar Arnfjörð Bjarmason
2021-10-09 13:34 ` [PATCH 2/2] commit: fix duplication regression in permission " Ævar Arnfjörð Bjarmason
2021-10-11 20:56   ` Junio C Hamano
2021-10-12 14:30   ` Ævar Arnfjörð Bjarmason [this message]
2021-10-12 14:30     ` [PATCH v2 1/2] unwritable tests: assert exact " Ævar Arnfjörð Bjarmason
2021-10-12 14:30     ` [PATCH v2 2/2] commit: fix duplication regression in permission " Ævar Arnfjörð Bjarmason
2021-10-11 20:50 ` [PATCH 1/2] unwritable tests: assert exact " Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover-v2-0.2-00000000000-20211012T142950Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=novalis@novalis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).