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

Re: [PATCH] test: fix t7001 cp to use POSIX options

From
Jens Lehmann <jens.lehmann@web.de>
Date
Apr 12, 2014, 21:52 UTC
Message-ID
<5349B5A3.30601@web.de>
In-Reply-To
<xmqq1tx3u0ts.fsf@gitster.dls.corp.google.com>
Am 11.04.2014 21:23, schrieb Junio C Hamano:
Show 36 quoted lines
> Jeff King <peff@peff.net> writes:
> 
>> On Fri, Apr 11, 2014 at 01:24:02AM -0700, Kyle J. McKay wrote:
>>
>>> Since 11502468 and 04c1ee57 (both first appearing in v1.8.5), the
>>> t7001-mv test has used "cp -a" to perform a copy in several of the
>>> tests.
>>>
>>> However, the "-a" option is not required for a POSIX cp utility and
>>> some platforms' cp utilities do not support it.
>>>
>>> The POSIX equivalent of -a is -R -P -p.
>>>
>>> Change "cp -a" to "cp -R -P -p" so that the t7001-mv test works
>>> on systems with a cp utility that only implements the POSIX
>>> required set of options and not the "-a" option.
>>
>> I wonder if the "-R" is the part that we actually care about here.
>> Including the others does not hurt in that case, but using only "-R"
>> would perhaps make it more obvious to a later reader of the code exactly
>> what we are trying to do.
> 
> These calls to "cp" are about "We know that earlier 'update' created
> the GIT_DIR of the submodule in the top-level superproject, because
> we are running a modern Git.  But we want to make sure the command
> we are testing, "git mv", would work correctly if the repository
> were made with an older Git that created the GIT_DIR embedded in the
> working tree of the submodule, so let's emulate that case."  As we
> create files and directories in GIT_DIR with explicit permission
> bits, we may care about
> 
>  (1) making sure "git mv" can actually move the directory, with some
>      paths having mode bits different from the umasked default; and
> 
>  (2) checking that the GIT_DIR after "git mv" has the permission
>      bits right.

When writing these tests I didn't care about (2), but - in addition to the first half of (1) - I had another thing in mind:

(3) "git mv" shouldn't try to update the 'core.worktree' setting
    when the GIT_DIR is embedded in the submodule's work tree.
Show 19 quoted lines
> and if we cared, "-R -p" would be required here, not just "-R".
> 
> If core.prefersymlinkrefs is set, GIT_DIR would have a symbolic link
> HEAD pointing at the current branch, and "-P" may become relevant,
> but manually running "cp -R .git git && ls -l git/HEAD" in such an
> old repository tells me that symbolic link HEAD is not dereferenced
> without an explicit "-L", so I dunno.
> 
> Because we do not check anything inside GIT_DIR of the moved
> submodule after "git mv" is done, the more correct use of "cp" is
> moot for the purpose of (2), but it could be possible that "git mv"
> fails to move a submodule with GIT_DIR created embedded in its
> working tree by an older version of Git, while successfully copying
> an emulated one, due to differences such as modes and symlinks.
> 
> The current implementation just does rename(2) on the whole
> submodule working tree and let its contents move together, so I do
> not think it matters at the moment for the purpose of (1); use of
> flags other than "-R" are purely for future-proofing, I would think.

Thanks for your detailed analysis and sorry to all parties involved for the hassle caused by my knee-jerk reaction to just use "cp -a" when I wanted to have an exact copy of 'that' directory 'there'.

Given that all other tests just use "cp -R" too in that situation I'm all for the second version of Kyle's patch, so an "Acked-by" from me on that one.

Previous: Junio C Hamano
Message 5 of 5 in “test: fix t7001 cp to use POSIX options”
  1. test: fix t7001 cp to use POSIX optionsKyle J. McKay, Apr 11, 2014
  2. Jeff KingApr 11, 2014
  3. Kyle J. McKayApr 11, 2014
  4. Junio C HamanoApr 11, 2014
  5. Jens LehmannApr 12, 2014

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.