Page MenuHomePhabricator

CI failures in "git-status" task related to submodules
Open, Needs TriagePublic

Description

Patches in mediawiki/extensions/VisualEditor have been recently failing CI with errors like this:

21:39:56 Running "git-status" task
21:39:56 >> Unstaged changes in these files:
21:39:56 >> lib/ve
21:39:57 --- a/lib/ve
21:39:57 +++ b/lib/ve
21:39:57 @@ -1 +1 @@
21:39:57 -Subproject commit 1f76dea7e63ca6b7bd5f1ef6a372c1878429f973
21:39:57 +Subproject commit 3891890c29a72941b0eae1bfd7b3b829bc58b343
21:39:57 Warning: Task "git-status" failed. Use --force to continue.

This is despite the patch not touching the lib/ve submodule.

Examples:

Timing seems to match the switch to Node 12 from Node 10 (T284345). No idea how it could be related.

git-status comes from:

Gruntfile.js
grunt.registerTask( 'git-status', function () {
    var done = this.async();
    // Are there unstaged changes?
    require( 'child_process' ).exec( 'git ls-files --modified', function ( err, stdout, stderr ) {
        var ret = err || stderr || stdout;
        if ( ret ) {
            grunt.log.error( 'Unstaged changes in these files:' );
            grunt.log.error( ret );
            // Show a condensed diff
            require( 'child_process' ).exec( 'git diff -U1 | tail -n +3', function ( err2, stdout2, stderr2 ) {
                grunt.log.write( err2 || stderr2 || stdout2 );
                done( false );
            } );
        } else {
            grunt.log.ok( 'No unstaged changes.' );
            done();
        }
    } );
} );

And the CI job did:

14:49:01 + git submodule update --init --remote
14:49:06 Submodule path 'lib/ve': checked out '3891890c29a72941b0eae1bfd7b3b829bc58b343'

But the submodule got set to 1f76dea7e63ca6b7bd5f1ef6a372c1878429f973 by a previous change: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/VisualEditor/+/698062

Event Timeline

Given that 1f76dea7e63ca6b7bd5f1ef6a372c1878429f973 is the current value pointer for VE-MW HEAD, but 3891890c29a72941b0eae1bfd7b3b829bc58b343 is the current VE-standalone HEAD, possibly the git submodule checkout isn't respecting the pointer? I quickly made https://gerrit.wikimedia.org/r/c/mediawiki/extensions/VisualEditor/+/698619 to see if that resets the state to work…

The node10 images have git 2.20.1; the node12 ones have git 2.30.2. Possibly something changed about how git submodule checkouts work?

Looking at the build for https://gerrit.wikimedia.org/r/c/mediawiki/extensions/VisualEditor/+/698547 which is https://integration.wikimedia.org/ci/job/mwgate-node12-docker/417/console the job fetch the submodules twice for some reason:

First with releng/ci-src-setup-simple:0.4.2-s1:

git fetch --quiet --update-head-ok --depth 2 git://contint1001.wikimedia.org/mediawiki/extensions/VisualEditor +refs/zuul/master/Z38e73f48b0a34aa8bd142391ca3d5337:refs/zuul/master/Z38e73f48b0a34aa8bd142391ca3d5337
git submodule --quiet update --init --recursive

And a second time via releng/node12-test:0.0.1:

14:28:37 + npm ci
14:29:10 
14:29:10 added 880 packages, and audited 881 packages in 33s
14:29:10 
14:29:10 12 vulnerabilities (3 low, 9 moderate)
14:29:10 
14:29:10 To address issues that do not require attention, run:
14:29:10   npm audit fix
14:29:10 
14:29:10 To address all issues (including breaking changes), run:
14:29:10   npm audit fix --force
14:29:10 
14:29:10 Run `npm audit` for details.
14:29:10 + git submodule update --init --remote
14:29:10 Submodule path 'lib/ve': checked out '3891890c29a72941b0eae1bfd7b3b829bc58b343'

That second commands passes --remote: git submodule update --init --remote. That instructs git to ignore the registered submodule branch and instead request it from the remote repository ( VisualEditor/VisualEditor.git ). So essentially that commands bypass the logic and force the extension to always be tested with the tip of the VisualEditor/VisualEditor.git master branch. That is wrong?

The command can be found via:

dockerfiles/node12-test/run.sh:git submodule update --init --remote

It might have been copy pasted from dockerfiles/node10-portals/run.sh which has git submodule update --init --remote.

TLDR: the node12-test entry point is wrong :]

Change 698624 had a related patch set uploaded (by Hashar; author: Hashar):

[integration/config@master] dockerfiles: node12-test do not process submodules

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

And for the context, the --remote used to be in the original job we crafted for portals ( https://gerrit.wikimedia.org/r/c/integration/config/+/393252 ), the reason is not captured maybe on the task or it was the result of a discussion with Jan. The logic got later moved from the Jenkins job into the image entry point ( https://gerrit.wikimedia.org/r/c/integration/config/+/589414 and for jjb https://gerrit.wikimedia.org/r/c/integration/config/+/589724 ).

Change 698624 merged by jenkins-bot:

[integration/config@master] dockerfiles: [node12-test] Do not process submodules

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

Mentioned in SAL (#wikimedia-releng) [2021-06-07T21:40:25Z] <James_F> Docker: Pushing node12-test ano node12-test-browser 0.0.2 for T284492

Change 698627 had a related patch set uploaded (by Jforrester; author: Jforrester):

[integration/config@master] jjb: Switch node12 images to ones without a git submodule over-ride

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

Change 698627 merged by jenkins-bot:

[integration/config@master] jjb: Switch node12 images to ones without a git submodule over-ride

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

Jdforrester-WMF claimed this task.

This should be back to working everywhere. If not, please shout.

That is also breaking the wikimedia-portals-build job which needs to have the submodule updated from the remote.