Page MenuHomePhabricator
Create Task
Maniphest T371255

scap backport broken on deploy1003 (bullseye, Git 2.30)
Closed, ResolvedPublic
Actions

Description

lucaswerkmeister-wmde@deploy1003 /srv/mediawiki-staging $ scap backport https://gerrit.wikimedia.org/r/c/operations/mediawiki-config/+/1055434                                  
13:01:50 Unhandled error:                                                                                                                                                                     
Traceback (most recent call last):                                                                                                                                                            
  File "/var/lib/scap/scap/lib/python3.9/site-packages/scap/cli.py", line 563, in run                                                                                                         
    exit_status = app.main(app.extra_arguments)                                                                                                                                               
  File "/var/lib/scap/scap/lib/python3.9/site-packages/scap/plugins/backport.py", line 335, in main                                                                                           
    self.git_repos = GitRepos(                                                                                                                                                                
  File "/var/lib/scap/scap/lib/python3.9/site-packages/scap/plugins/backport.py", line 89, in __init__                                                                                        
    self.config_repos = self._get_submodules_paths(self.mediawiki_location)                                                                                                                   
  File "/var/lib/scap/scap/lib/python3.9/site-packages/scap/plugins/backport.py", line 95, in _get_submodules_paths                                                                           
    paths_urls = git.list_submodules_paths_urls(location, "--recursive")                                                                                                                      
  File "/var/lib/scap/scap/lib/python3.9/site-packages/scap/git.py", line 707, in list_submodules_paths_urls                                                                                  
    return gitcmd(                                                                                                                                                                            
  File "/var/lib/scap/scap/lib/python3.9/site-packages/scap/runcmd.py", line 88, in gitcmd                                                                                                    
    return _runcmd(["git", subcommand] + list(args), **kwargs)                                                                                                                                
  File "/var/lib/scap/scap/lib/python3.9/site-packages/scap/runcmd.py", line 75, in _runcmd                                                                                                   
    raise FailedCommand(argv, p.returncode, stdout, stderr)                                                                                                                                   
scap.runcmd.FailedCommand: Command 'git submodule -q foreach echo -n "$PWD " && git remote get-url origin --recursive' failed with exit code 128;                                             
stdout:                                                                                                                                                                                       
/srv/mediawiki-staging/lib/excimer-ui-client                                                                                                                                                  
stderr:                                                                                                                                                                                       
error: unknown option `recursive'                                                                                                                                                             
usage: git remote get-url [--push] [--all] <name>                                                                                                                                             
usage: git remote get-url [--push] [--all] <name>                                                                                                                                             
                                                                                                                                                                                              
    --push                query push URLs rather than fetch URLs                                                                                                                              
    --all                 return all URLs                                                                                                                                                     

fatal: run_command returned non-zero status for lib/excimer-ui-client

I can’t find any evidence that git remote get-url ever had a --recursive option; I suspect Git just used to silently ignore it, and now it doesn’t. The option was added in backport.py: minor refactor.

Details

Related Changes in GitLab:
TitleReferenceAuthorSource BranchDest Branch
backport: pass argument to correct git commandrepos/releng/scap!391jnucheT371255master
Customize query in GitLab

Related Objects

Event Timeline

• Lucas_Werkmeister_WMDE created this task.Jul 29 2024, 1:10 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 29 2024, 1:10 PM
KartikMistry subscribed.Jul 29 2024, 1:16 PM
• Lucas_Werkmeister_WMDE added a subscriber: akosiaris.Jul 29 2024, 1:19 PM

According to @akosiaris in IRC:

it's passing --recursive to the wrong git subcommand, it should be passing --recursive to git submodule foreach

• Lucas_Werkmeister_WMDE added a comment.Jul 29 2024, 1:24 PM

Hm, but the code looks to me like it’s supposed to do the right thing?

def list_submodules_paths_urls(repo, args):
    """Return a list of the paths and URLs of the submodules of the given repository, separated by a space"""
    ensure_dir(repo)

    return gitcmd(
        "submodule",
        "-q",
        "foreach",
        'echo -n "$PWD " && git remote get-url origin',
        args,
        cwd=repo,
    ).splitlines()

I would assume that this runs the equivalent of:

git submodule -q foreach 'echo -n "$PWD " && git remote get-url origin' $args

i.e. that the && and git remote get-url is all part of the for-each-submodule command, and --recursive in --args is then interpreted as another option to git submodule foreach. But maybe it’s being quoted or unquoted differently somewhere?

• Lucas_Werkmeister_WMDE added a comment.Jul 29 2024, 1:26 PM

Okay, it’s Git behaving weirdly (IMHO):

lucaswerkmeister-wmde@deploy1003 /srv/mediawiki-staging $ git submodule foreach --recursive 'echo a && echo b'
Entering 'lib/excimer-ui-client'
a
b
Entering 'portals'
a
b
lucaswerkmeister-wmde@deploy1003 /srv/mediawiki-staging $ git submodule foreach 'echo a && echo b' --recursive
Entering 'lib/excimer-ui-client'
a
b --recursive
Entering 'portals'
a
b --recursive

So --recursive needs to come before the command to run in each submodule.

CodeReviewBot added a project: Patch-For-Review.Jul 29 2024, 1:29 PM

jnuche opened https://gitlab.wikimedia.org/repos/releng/scap/-/merge_requests/391

backport: pass argument to correct git command

• Lucas_Werkmeister_WMDE added a comment.Jul 29 2024, 1:32 PM

(Unfortunately I can’t easily repeat the above test on deploy1002 because /src/mediawiki-staging no longer exists as a git repo, and I couldn’t figure out how to set up a homedir-local repo with submodules either (adding a submodule gave me “transport 'file' not allowed” errors). But I think the fix above looks good.)

• Lucas_Werkmeister_WMDE added a comment.Jul 29 2024, 1:38 PM

Scratch that, I was just accidentally on mwdebug1002 instead of deploy1002 😅 we can see Git’s old behavior:

lucaswerkmeister-wmde@deploy1002 /srv/mediawiki-staging $ git submodule foreach --recursive 'echo a && echo b'
Entering 'lib/excimer-ui-client'
a
b
Entering 'portals'
a
b
lucaswerkmeister-wmde@deploy1002 /srv/mediawiki-staging $ git submodule foreach 'echo a && echo b' --recursive
Entering 'lib/excimer-ui-client'
a
b
Entering 'portals'
a
b
• Lucas_Werkmeister_WMDE added a subscriber: jnuche.Jul 29 2024, 1:42 PM

Git used to interpret the --recursive wherever it was in the argv:

lucaswerkmeister-wmde@deploy1002 /srv/mediawiki-staging $ git -C php-1.43.0-wmf.15/ submodule foreach 'echo a && echo b' | wc -l
606
lucaswerkmeister-wmde@deploy1002 /srv/mediawiki-staging $ git -C php-1.43.0-wmf.15/ submodule foreach --recursive 'echo a && echo b' | wc -l
636
lucaswerkmeister-wmde@deploy1002 /srv/mediawiki-staging $ git -C php-1.43.0-wmf.15/ submodule foreach 'echo a && echo b' --recursive | wc -l
636

So @jnuche’s fix above looks correct IMHO. (If Git had previously just ignored the --recursive when it was at the end, then moving it to a position where new Git will recognize it might have introduced a change in behavior that would warrant more investigation.)

CodeReviewBot added a comment.Jul 29 2024, 1:46 PM

jnuche merged https://gitlab.wikimedia.org/repos/releng/scap/-/merge_requests/391

backport: pass argument to correct git command

• thcipriani subscribed.Jul 29 2024, 1:54 PM

I think the update to from git v2.20.1 to v.2.30.2 explains this. this commit may be what changed to make @jnuche 's fix needed.

• Lucas_Werkmeister_WMDE added a comment.Jul 29 2024, 2:05 PM

Seems to be working again with scap 4.94.0 (SAL), thanks all!

jnuche mentioned this in rMSCAdfc84a2b8fbc: backport: pass argument to correct git command.Jul 29 2024, 2:06 PM
• Lucas_Werkmeister_WMDE added a comment.Jul 29 2024, 2:13 PM

Though this feels concerning, partway through the scap backport:

14:11:07 Started sync-masters                                                                                                                                                                 
14:11:08 ['/usr/bin/scap', 'pull-master', 'deploy1003.eqiad.wmnet'] (ran as mwdeploy@deploy1002.eqiad.wmnet) returned [1]: Traceback (most recent call last):                                 
  File "/usr/bin/scap", line 32, in <module>                                                                                                                                                  
    from scap import cli  # noqa: E402                                                                                                                                                        
ImportError: cannot import name 'cli' from 'scap' (unknown location)                                                                                                                          

14:11:08 ['/usr/bin/scap', 'pull-master', 'deploy1003.eqiad.wmnet'] (ran as mwdeploy@deploy2002.codfw.wmnet) returned [1]: Traceback (most recent call last):
  File "/usr/bin/scap", line 32, in <module>
    from scap import cli  # noqa: E402
ImportError: cannot import name 'cli' from 'scap' (unknown location)

14:11:08 sync-masters: 100% (in-flight: 0; ok: 0; fail: 2; left: 0)             
14:11:08 2 masters had sync errors
14:11:08 Finished sync-masters (duration: 00m 00s)
• Lucas_Werkmeister_WMDE mentioned this in T371261: scap broken on deploy1002 / deploy2002 (buster).Jul 29 2024, 2:17 PM
In T371255#10023148, @Lucas_Werkmeister_WMDE wrote:

Though this feels concerning, partway through the scap backport:

Made a separate task for that at T371261 – it’s not directly related to the git submodule foreach --recursive issue, I think.

Maintenance_bot removed a project: Patch-For-Review.Jul 29 2024, 2:30 PM
• Lucas_Werkmeister_WMDE closed this task as Resolved.Jul 29 2024, 2:45 PM
• Lucas_Werkmeister_WMDE assigned this task to jnuche.
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits