Page MenuHomePhabricator

Quibble should errors out when a git submodule fails
Closed, ResolvedPublic

Description

When git submodule update fails, Quibble does not catch the issue and keep processing:

INFO:quibble.cmd:Updating git submodules of extensions and skins
extensions/Wikibase/.gitmodules
...
Cloning into 'view/lib/wikibase-serialization'...
Submodule path 'view/lib/wikibase-serialization': checked out '3cd428fa063d375291f87359c02d8d1147b6e0fb'
Unable to checkout '7581ce4c6f98a36209bebb8e2d4f4158c943c629' in submodule path 'view/lib/wikibase-data-model'
INFO:zuul.CloneMapper:Workspace path set to: ./

Probably git submodule just exit 0 when that should be a hard fail.

The relevant Quibble code is:

quibble/cmd.py
def ext_skin_submodule_update(self):
    self.log.info('Updating git submodules of extensions and skins')
    # From JJB macro ext-skins-submodules-update
    # jjb/mediawiki-extensions.yaml
    subprocess.check_call([
        # Do not add ., or that will process mediawiki/core submodules in
        # wmf branches which is a mess.
        'find', 'extensions', 'skins',
        '-maxdepth', '2',
        '-name', '.gitmodules',
        '-print',
        '-execdir', 'bash', '-xe', '-c',
        '\n'.join([
             'git submodule foreach git clean -xdff -q',
             'git submodule update --init --recursive',
             'git submodule status',
             ]),
        ';',  # end of -execdir
         ], cwd=self.mw_install_path)

Event Timeline

hashar created this task.Jul 6 2018, 5:07 PM
hashar moved this task from Backlog to Defect on the Quibble board.Jul 19 2018, 2:50 PM

I do not have any idea how to detect the submodule fetching failed. Maybe by running git submodule status and processing its output :/

hashar updated the task description. (Show Details)Mar 19 2019, 8:07 AM

At least with git 2.11, the git submodule update seems to exit 1 when I kill the underlying git-remote-https subprocess.

The reason seems to be find -execdir which shallows the command exit code:

$ find -name .gitmodules -execdir /bin/false  \; ; echo $?
0

From the man page:

-execdir command ;
-execdir command {} +
If any invocation returns a non-zero value as exit status, then find returns a non-zero exit status.
If find encounters an error, this can sometimes cause an immediate exit, so some pending commands may not be run at all.
The result of the action depends on whether the + or the ; variant is being used;
-execdir command {} + always returns true,
-execdir command {} ; returns true only if command returns 0.

Test case:

mkdir /tmp/finder
touch /tmp/finder/{bar,foo}.txt
$ find /tmp/finder -type f -print -execdir /bin/false \; ; echo $?
/tmp/finder/foo.txt
/tmp/finder/bar.txt
0
$ find /tmp/finder -type f -print -execdir /bin/false {} \+ ; echo $?
/tmp/finder/foo.txt
/tmp/finder/bar.txt
1
hashar claimed this task.Mar 19 2019, 10:23 AM
hashar added a project: Patch-For-Review.

Gerrit is not reachable but here is the patch:

hashar triaged this task as High priority.Mar 19 2019, 10:23 AM
hashar moved this task from Defect to In progress on the Quibble board.

Change 497474 had a related patch set uploaded (by Hashar; owner: Hashar):
[integration/quibble@master] Properly abort when git submodule processing fails

https://gerrit.wikimedia.org/r/497474

Change 497474 merged by jenkins-bot:
[integration/quibble@master] Properly abort when git submodule processing fails

https://gerrit.wikimedia.org/r/497474

hashar closed this task as Resolved.Apr 2 2019, 9:52 AM

Should be good since Qiubble 0.0.31

hashar added a comment.Apr 4 2019, 7:49 PM

And it indeed works:

Submodule 'lib/ve' (https://gerrit.wikimedia.org/r/VisualEditor/VisualEditor.git) registered for path 'lib/ve'
Cloning into '/workspace/src/extensions/VisualEditor/lib/ve'...
error: no such remote ref 60b18553f831cbdce201e693dbfca33cc9024d16
Fetched in submodule path 'lib/ve', but it did not contain 60b18553f831cbdce201e693dbfca33cc9024d16. Direct fetching of that commit failed.
ERROR:quibble.cmd:Failed to process git submodules for /workspace/src/extensions/VisualEditor
Traceback (most recent call last):
  File "/usr/local/bin/quibble", line 11, in <module>
    load_entry_point('quibble==0.0.0', 'console_scripts', 'quibble')()
  File "/usr/local/lib/python3.5/dist-packages/quibble/cmd.py", line 573, in main
    cmd.execute()
  File "/usr/local/lib/python3.5/dist-packages/quibble/cmd.py", line 444, in execute
    self.ext_skin_submodule_update()
  File "/usr/local/lib/python3.5/dist-packages/quibble/cmd.py", line 264, in ext_skin_submodule_update
    raise e
  File "/usr/local/lib/python3.5/dist-packages/quibble/cmd.py", line 259, in ext_skin_submodule_update
    subprocess.check_call(cmd, cwd=dirpath)
  File "/usr/lib/python3.5/subprocess.py", line 271, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['git', 'submodule', 'update', '--init', '--recursive']' returned non-zero exit status 1
Build step 'Execute shell' marked build as failure