<feed xmlns='http://www.w3.org/2005/Atom'>
<title>git/t/lib-submodule-update.sh, branch v2.29.2</title>
<subtitle>Mirror of https://git.kernel.org/pub/scm/git/git.git/
</subtitle>
<id>https://git.shady.money/git/atom?h=v2.29.2</id>
<link rel='self' href='https://git.shady.money/git/atom?h=v2.29.2'/>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/'/>
<updated>2020-07-30T16:16:49Z</updated>
<entry>
<title>t: remove test_oid_init in tests</title>
<updated>2020-07-30T16:16:49Z</updated>
<author>
<name>brian m. carlson</name>
<email>sandals@crustytoothpaste.net</email>
</author>
<published>2020-07-29T23:14:28Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=e023ff0691ca207d421a0e75ea23c132ada9142a'/>
<id>urn:sha1:e023ff0691ca207d421a0e75ea23c132ada9142a</id>
<content type='text'>
Now that we call test_oid_init in the setup for all test scripts,
there's no point in calling it individually.  Remove all of the places
where we've done so to help keep tests tidy.

Signed-off-by: brian m. carlson &lt;sandals@crustytoothpaste.net&gt;
Reviewed-by: Eric Sunshine &lt;sunshine@sunshineco.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
</entry>
<entry>
<title>Merge branch 'dl/test-must-fail-fixes-5'</title>
<updated>2020-07-07T05:09:18Z</updated>
<author>
<name>Junio C Hamano</name>
<email>gitster@pobox.com</email>
</author>
<published>2020-07-07T05:09:18Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=efafdca42162843e90cefdeefd8e6c244cee91f5'/>
<id>urn:sha1:efafdca42162843e90cefdeefd8e6c244cee91f5</id>
<content type='text'>
The effort to avoid using test_must_fail on non-git command continues.

* dl/test-must-fail-fixes-5:
  lib-submodule-update: pass 'test_must_fail' as an argument
  lib-submodule-update: prepend "git" to $command
  lib-submodule-update: consolidate --recurse-submodules
  lib-submodule-update: add space after function name
</content>
</entry>
<entry>
<title>lib-submodule-update: pass 'test_must_fail' as an argument</title>
<updated>2020-06-24T15:54:18Z</updated>
<author>
<name>Denton Liu</name>
<email>liu.denton@gmail.com</email>
</author>
<published>2020-06-24T08:50:18Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=5b0ac09fb17656553d85c7e3fbe0583ba3023e5e'/>
<id>urn:sha1:5b0ac09fb17656553d85c7e3fbe0583ba3023e5e</id>
<content type='text'>
When we run a test helper function in test_submodule_switch_common(), we
sometimes specify a whole helper function as the $command. When we do
this, in some test cases, we just mark the whole function with
`test_must_fail`. However, it's possible that the helper function might
fail earlier or later than expected due to an introduced bug. If this
happens, then the test case will still report as passing but it should
really be marked as failing since it didn't actually display the
intended behaviour.

Instead of invoking `test_must_fail $command`, pass the string
"test_must_fail" as the second argument in case where the git command is
expected to fail.

When $command is a helper function, the parent function calling
test_submodule_switch_common() is test_submodule_switch_func(). For all
test_submodule_switch_func() invocations, increase the granularity of
the argument test helper function by prefixing the git invocation which is
meant to fail with the second argument like this:

	$2 git checkout "$1"

In the other cases, test_submodule_switch() and
test_submodule_forced_switch(), instead of passing in the git command
directly, wrap it using the git_test_func() and pass the git arguments
using the global variable $gitcmd. Unfortunately, since closures aren't
a thing in shell scripts, the global variable is necessary. Another
unfortunate result is that the "git_test_func" will used as the test
case name when $command is printed but it's worth it for the cleaner
code.

Finally, as an added bonus, `test_must_fail` will now only run on git
commands.

Signed-off-by: Denton Liu &lt;liu.denton@gmail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
</entry>
<entry>
<title>lib-submodule-update: prepend "git" to $command</title>
<updated>2020-06-12T18:33:40Z</updated>
<author>
<name>Denton Liu</name>
<email>liu.denton@gmail.com</email>
</author>
<published>2020-06-11T17:41:49Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=aa06180ac90984204e59ac74bb4e3a4d09037ac2'/>
<id>urn:sha1:aa06180ac90984204e59ac74bb4e3a4d09037ac2</id>
<content type='text'>
Since all invocations of test_submodule_forced_switch() are git
commands, automatically prepend "git" before invoking
test_submodule_switch_common().

Similarly, many invocations of test_submodule_switch() are also git
commands so automatically prepend "git" before invoking
test_submodule_switch_common() as well.

Finally, for invocations of test_submodule_switch() that invoke a custom
function, rename the old function to test_submodule_switch_func().

This is necessary because in a future commit, we will be adding some
logic that needs to distinguish between an invocation of a plain git
comamnd and an invocation of a test helper function.

Signed-off-by: Denton Liu &lt;liu.denton@gmail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
</entry>
<entry>
<title>lib-submodule-update: consolidate --recurse-submodules</title>
<updated>2020-04-29T17:36:55Z</updated>
<author>
<name>Denton Liu</name>
<email>liu.denton@gmail.com</email>
</author>
<published>2020-04-29T12:22:24Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=add245234820be6cec3f54174d44f738aa1a80f9'/>
<id>urn:sha1:add245234820be6cec3f54174d44f738aa1a80f9</id>
<content type='text'>
Both test_submodule_switch_recursing_with_args() and
test_submodule_forced_switch_recursing_with_args() call the internal
function test_submodule_recursing_with_args_common() with the final
argument of `--recurse-submodules`. Consolidate this duplication by
appending the argument in test_submodule_recursing_with_args_common().

Signed-off-by: Denton Liu &lt;liu.denton@gmail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
</entry>
<entry>
<title>lib-submodule-update: add space after function name</title>
<updated>2020-04-29T17:36:55Z</updated>
<author>
<name>Denton Liu</name>
<email>liu.denton@gmail.com</email>
</author>
<published>2020-04-29T12:22:23Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=27691ca27e0be0195e30450cc0316c2cc090dfca'/>
<id>urn:sha1:27691ca27e0be0195e30450cc0316c2cc090dfca</id>
<content type='text'>
In the shell scripts in this codebase, the usual style is to include a
space between the function name and the (). Add these missing spaces to
conform to the usual style of the code.

Signed-off-by: Denton Liu &lt;liu.denton@gmail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
</entry>
<entry>
<title>Merge branch 'jk/test-cleanup'</title>
<updated>2020-04-22T20:42:44Z</updated>
<author>
<name>Junio C Hamano</name>
<email>gitster@pobox.com</email>
</author>
<published>2020-04-22T20:42:44Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=dfe48154b16e35ea01795aa94c7e8686f084e2c3'/>
<id>urn:sha1:dfe48154b16e35ea01795aa94c7e8686f084e2c3</id>
<content type='text'>
Test cleanup.

* jk/test-cleanup:
  t/lib-*.sh: drop executable bit
  t/lib-credential.sh: drop shebang line
</content>
</entry>
<entry>
<title>t/lib-*.sh: drop executable bit</title>
<updated>2020-03-27T18:26:48Z</updated>
<author>
<name>Jeff King</name>
<email>peff@peff.net</email>
</author>
<published>2020-03-26T08:25:53Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=12dc0879f18b39e1020bd6afec117bc38110d8c9'/>
<id>urn:sha1:12dc0879f18b39e1020bd6afec117bc38110d8c9</id>
<content type='text'>
There's no need for shell libraries to have the executable bit. They're
meant to be sourced, and running them stand-alone is pointless. Let's
reduce any possible confusion by making it more clear they're not meant
to be run this way.

Signed-off-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
</entry>
<entry>
<title>t/lib-submodule-update: add test removing nested submodules</title>
<updated>2020-02-19T23:28:23Z</updated>
<author>
<name>Philippe Blain</name>
<email>levraiphilippeblain@gmail.com</email>
</author>
<published>2020-02-17T04:53:06Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=846f34d3514b81764dea7c2a4851f6187ab31aad'/>
<id>urn:sha1:846f34d3514b81764dea7c2a4851f6187ab31aad</id>
<content type='text'>
The previous commit fixed a bug with the (no submodule) -&gt; (nested
submodules) transition for commands in the unpack-trees machinery.

Let's add a test for the reverse transition (going from nested
submodules to no submodule), as it is not being tested currently.

While at it, uniformize the capitalization in the list of tests.

Signed-off-by: Philippe Blain &lt;levraiphilippeblain@gmail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
</entry>
<entry>
<title>unpack-trees: check for missing submodule directory in merged_entry</title>
<updated>2020-02-19T23:27:35Z</updated>
<author>
<name>Philippe Blain</name>
<email>levraiphilippeblain@gmail.com</email>
</author>
<published>2020-02-17T04:53:05Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=e84704f15ca793a377cb6ef24ab614b95775d008'/>
<id>urn:sha1:e84704f15ca793a377cb6ef24ab614b95775d008</id>
<content type='text'>
Using `git checkout --recurse-submodules` to switch between a
branch with no submodules and a branch with initialized nested
submodules currently causes a fatal error:

    $ git checkout --recurse-submodules branch-with-nested-submodules
    fatal: exec '--super-prefix=submodule/nested/': cd to 'nested'
failed: No such file or directory
    error: Submodule 'nested' could not be updated.
    error: Submodule 'submodule/nested' cannot checkout new HEAD.
    error: Submodule 'submodule' could not be updated.
    M	submodule
    Switched to branch 'branch-with-nested-submodules'

The checkout succeeds but the worktree and index of the first level
submodule are left empty:

    $ cd submodule
    $ git -c status.submoduleSummary=1 status
    HEAD detached at b3ce885
    Changes to be committed:
      (use "git restore --staged &lt;file&gt;..." to unstage)
          deleted:    .gitmodules
          deleted:    first.t
          deleted:    nested

    fatal: not a git repository: 'nested/.git'
    Submodule changes to be committed:

    * nested 1e96f59...0000000:

    $ git ls-files -s
    $ # empty
    $ ls -A
    .git

The reason for the fatal error during the checkout is that a child git
process tries to cd into the yet unexisting nested submodule directory.
The sequence is the following:

1. The main git process (the one running in the superproject) eventually
reaches write_entry() in entry.c, which creates the first level
submodule directory and then calls submodule_move_head() in submodule.c,
which spawns `git read-tree` in the submodule directory.

2. The first child git process (the one in the submodule of the
superproject) eventually calls check_submodule_move_head() at
unpack_trees.c:2021, which calls submodule_move_head in dry-run mode,
which spawns `git read-tree` in the nested submodule directory.

3. The second child git process tries to chdir() in the yet unexisting
nested submodule directory in start_command() at run-command.c:829 and
dies before exec'ing.

The reason why check_submodule_move_head() is reached in the first child
and not in the main process is that it is inside an
if(submodule_from_ce()) construct, and submodule_from_ce() returns a
valid struct submodule pointer, whereas it returns a null pointer in the
main git process.

The reason why submodule_from_ce() returns a null pointer in the main
git process is because the call to cache_lookup_path() in config_from()
(called from submodule_from_path() in submodule_from_ce()) returns a
null pointer since the hashmap "for_path" in the submodule_cache of
the_repository is not yet populated. It is not populated because both
repo_get_oid(repo, GITMODULES_INDEX, &amp;oid) and repo_get_oid(repo,
GITMODULES_HEAD, &amp;oid) in config_from_gitmodules() at
submodule-config.c:639-640 return -1, as at this stage of the operation,
neither the HEAD of the superproject nor its index contain any
.gitmodules file.

In contrast, in the first child the hashmap is populated because
repo_get_oid(repo, GITMODULES_HEAD, &amp;oid) returns 0 as the HEAD of the
first level submodule, i.e. .git/modules/submodule/HEAD, points to a
commit where .gitmodules is present and records 'nested' as a submodule.

Fix this bug by checking that the submodule directory exists before
calling check_submodule_move_head() in merged_entry() in the `if(!old)`
branch, i.e. if going from a commit with no submodule to a commit with a
submodule present.

Also protect the other call to check_submodule_move_head() in
merged_entry() the same way as it is safer, even though the `else if
(!(old-&gt;ce_flags &amp; CE_CONFLICTED))` branch of the code is not at play in
the present bug.

The other calls to check_submodule_move_head() in other functions in
unpack_trees.c are all already protected by calls to lstat() somewhere
in
the program flow so we don't need additional protection for them.

All commands in the unpack_trees machinery are affected, i.e. checkout,
reset and read-tree when called with the --recurse-submodules flag.

This bug was first reported in [1].

[1]
https://lore.kernel.org/git/7437BB59-4605-48EC-B05E-E2BDB2D9DABC@gmail.com/

Reported-by: Philippe Blain &lt;levraiphilippeblain@gmail.com&gt;
Reported-by: Damien Robert &lt;damien.olivier.robert@gmail.com&gt;
Signed-off-by: Philippe Blain &lt;levraiphilippeblain@gmail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
</entry>
</feed>
