git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 1/3] t0033: add tests for safe.directory

From
Derrick Stolee <derrickstolee@github.com>
Date
Apr 13, 2022, 16:29 UTC
Message-ID
<7f5bebcd-5622-51c7-2904-b793337843b1@github.com>
In-Reply-To
<xmqqwnft9cjg.fsf@gitster.g>
On 4/13/2022 12:24 PM, Junio C Hamano wrote:
Show 18 quoted lines
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Derrick Stolee <derrickstolee@github.com>
>>
>> It is difficult to change the ownership on a directory in our test
>> suite, so insert a new GIT_TEST_ASSUME_DIFFERENT_OWNER environment
>> variable to trick Git into thinking we are in a differently-owned
>> directory. This allows us to test that the config is parsed correctly.
> 
> OK.
> 
>> -	if (is_path_owned_by_current_user(path))
>> +	if (is_path_owned_by_current_user(path) &&
>> +	    !git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0))
>>  		return 1;
> 
> Shouldn't the overriding "GIT_TEST_BLAH" be checked before the
> real logic kicks in, I wonder?
Either order would work. I bet that checking the environment is
faster than checking the disk, so swapping the order would be prudent
here.
 
Show 32 quoted lines
>> diff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh
>> new file mode 100755
>> index 00000000000..9380ff3d017
>> --- /dev/null
>> +++ b/t/t0033-safe-directory.sh
>> @@ -0,0 +1,34 @@
>> +#!/bin/sh
>> +
>> +test_description='verify safe.directory checks'
>> +
>> +. ./test-lib.sh
>> +
>> +GIT_TEST_ASSUME_DIFFERENT_OWNER=1
>> +export GIT_TEST_ASSUME_DIFFERENT_OWNER
>> +
>> +expect_rejected_dir () {
>> +	test_must_fail git status 2>err &&
>> +	grep "safe.directory" err
>> +}
>> +...
>> +test_expect_success 'safe.directory matches' '
>> +	git config --global --add safe.directory "$(pwd)" &&
>> +	git status
>> +'
> 
> Just double checking, as I know you are much closer to the affected
> platform than I'd ever be ;-) but is the use of $(pwd) safe and
> correct here?
> 
> I always get confused between $(pwd) and $PWD, which does not make
> any difference on platforms I have access to, but makes difference
> to hurt Windows users.

These tests pass CI on Windows. I've had issues before using $PWD, thinking back to tests in t7900-maintenance.sh that use $(pwd) instead.

Thanks, -Stolee

Previous: Junio C HamanoNext: Ævar Arnfjörð Bjarmason
Message 4 of 39 in “Updates to the safe.directory config option”
  1. 0/3 Updates to the safe.directory config optionDerrick Stolee via GitGitGadget, Apr 13, 2022
  2. 1/3 t0033: add tests for safe.directoryDerrick Stolee via GitGitGadget, Apr 13, 2022
  3. Junio C HamanoApr 13, 2022
  4. Derrick StoleeApr 13, 2022
  5. Ævar Arnfjörð BjarmasonApr 13, 2022
  6. Junio C HamanoApr 13, 2022
  7. Derrick StoleeApr 13, 2022
  8. 2/3 setup: fix safe.directory key not being checkedMatheus Valadares via GitGitGadget, Apr 13, 2022
  9. Junio C HamanoApr 13, 2022
  10. 3/3 setup: opt-out of check with safe.directory=*Derrick Stolee via GitGitGadget, Apr 13, 2022
  11. Junio C HamanoApr 13, 2022
  12. Junio C HamanoApr 13, 2022
  13. Derrick StoleeApr 13, 2022
  14. Junio C HamanoApr 13, 2022
  15. Junio C HamanoApr 13, 2022
  16. Taylor BlauApr 13, 2022
  17. Junio C HamanoApr 13, 2022
  18. 0/3 t0033-safe-directory: test improvements and a documentation clarificationSZEDER Gábor, Apr 27, 2022
  19. 1/3 t0033-safe-directory: check the error message without matching the trash dirSZEDER Gábor, Apr 27, 2022
  20. Taylor BlauMay 9, 2022
  21. Carlo Marcelo Arenas BelónMay 10, 2022
  22. 2/3 t0033-safe-directory: check when 'safe.directory' is ignoredSZEDER Gábor, Apr 27, 2022
  23. Junio C HamanoApr 27, 2022
  24. Derrick StoleeApr 29, 2022
  25. Junio C HamanoApr 29, 2022
  26. SZEDER GáborApr 29, 2022
  27. Derrick StoleeApr 29, 2022
  28. SZEDER GáborMay 10, 2022
  29. Taylor BlauMay 10, 2022
  30. Carlo Marcelo Arenas BelónMay 10, 2022
  31. Taylor BlauMay 10, 2022
  32. Eric SunshineMay 10, 2022
  33. 3/3 safe.directory: document and check that it's ignored in the environmentSZEDER Gábor, Apr 27, 2022
  34. Junio C HamanoApr 27, 2022
  35. Junio C HamanoApr 27, 2022
  36. Derrick StoleeApr 29, 2022
  37. SZEDER GáborMay 9, 2022
  38. Junio C HamanoMay 9, 2022
  39. SZEDER GáborMay 10, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.