Skip to content

Fix cleaning of removed sub-submodules when using fetch strategy

DmtiryK requested to merge dkozlov/gitlab-runner:master into main

What does this MR do?

Fixes cleaning of removed sub-submodules when using the fetch strategy.

Why was this MR needed?

In https://gitlab.com/gitlab-org/gitlab-runner/-/blob/4d48abb0891720bd1350394654b1c5062f6c51d2/shells/abstract.go#L442-444 the sequence is effectively as follows for that function:

...
git submodule sync --recursive
git submodule foreach --recursive git clean -ffxd
git submodule foreach --recursive git reset --hard
git submodule update --init --recursive
...

What this does is:

  1. Update all the submodules' URLs, recursively
  2. Recursively remove any untracked files in the submodules
  3. Recursively put back any removed, tracked files in the submodules
  4. Recursively check out the correct commit in all submodules

The problem is step 2 happening before step 4. Let's say a submodule itself has a submodule, AND that in the new commit hash, that submodule has been removed. When we do step 2, no problem, we're on the earlier commit, and that sub-submodule is tracked and retained. When we do step 4, suddenly that directory that used to be sub-submodule is now some unknown directory that's left behind. If we had done the clean after the update, this would have been correctly removed.

Note: This MR adds a second git clean -ffxd after step 4 in the original sequence fix the issue !2351 (closed) According to the !2351 (comment 403269219) by @pedropombeiro

What's the best way to test this MR?

  1. Fork the test branch of the https://gitlab.com/pedropombeiro/merge_request-2883 repo;

  2. Create a new branch from the 9df3ab8f commit;

  3. Register a runner to build jobs with the gitlab.com, shell, and bash tags:

    gitlab-runner register --executor "shell" --shell "bash" --url "https://gitlab.com/" --description "bash-runner" --tag-list "shell,bash,gitlab.com" --locked="false" --access-level="not_protected" --registration-token="<your token>" --non-interactive
  4. Push the new branch;

  5. The job will succeed with the following log:

    $ find . -not -path './.git*'
    .
    ./README.md
    ./merge_request-2883-submodule-a
    ./merge_request-2883-submodule-a/merge_request-2883-sub-submodule-b-2
    ./merge_request-2883-submodule-a/merge_request-2883-sub-submodule-b-2/README.md
    ./merge_request-2883-submodule-a/merge_request-2883-sub-submodule-b-2/.git
    ./merge_request-2883-submodule-a/.gitmodules
    ./merge_request-2883-submodule-a/README.md
    ./merge_request-2883-submodule-a/merge_request-2883-sub-submodule-b-1
    ./merge_request-2883-submodule-a/merge_request-2883-sub-submodule-b-1/README.md
    ./merge_request-2883-submodule-a/merge_request-2883-sub-submodule-b-1/.git
    ./merge_request-2883-submodule-a/.git
  6. Cherry-pick the 52dc75f commit;

  7. Push the new commit;

  8. The job will succeed with the following log (notice the missing module b-2):

    $ find . -not -path './.git*'
    .
    ./README.md
    ./merge_request-2883-submodule-a
    ./merge_request-2883-submodule-a/.gitmodules
    ./merge_request-2883-submodule-a/README.md
    ./merge_request-2883-submodule-a/merge_request-2883-sub-submodule-b-1
    ./merge_request-2883-submodule-a/merge_request-2883-sub-submodule-b-1/README.md
    ./merge_request-2883-submodule-a/merge_request-2883-sub-submodule-b-1/.git
    ./merge_request-2883-submodule-a/.git

Results with current master branch

With current master, after executing the first job on 9df3ab8f, we see that all files are checked out:

$ find . -not -path './.git*'
.
./README.md
./merge_request-2883-submodule-a
./merge_request-2883-submodule-a/merge_request-2883-sub-submodule-b-2
./merge_request-2883-submodule-a/merge_request-2883-sub-submodule-b-2/README.md
./merge_request-2883-submodule-a/merge_request-2883-sub-submodule-b-2/.git
./merge_request-2883-submodule-a/.gitmodules
./merge_request-2883-submodule-a/README.md
./merge_request-2883-submodule-a/merge_request-2883-sub-submodule-b-1
./merge_request-2883-submodule-a/merge_request-2883-sub-submodule-b-1/README.md
./merge_request-2883-submodule-a/merge_request-2883-sub-submodule-b-1/.git
./merge_request-2883-submodule-a/.git

However, after cherry-picking the commit that removes the b-2 module, we see that after the fetch on the job runs, the repository directory still contains the removed sub-module:

$ find . -not -path './.git*'
.
./README.md
./merge_request-2883-submodule-a
./merge_request-2883-submodule-a/merge_request-2883-sub-submodule-b-2
./merge_request-2883-submodule-a/merge_request-2883-sub-submodule-b-2/README.md
./merge_request-2883-submodule-a/merge_request-2883-sub-submodule-b-2/.git
./merge_request-2883-submodule-a/.gitmodules
./merge_request-2883-submodule-a/README.md
./merge_request-2883-submodule-a/merge_request-2883-sub-submodule-b-1
./merge_request-2883-submodule-a/merge_request-2883-sub-submodule-b-1/README.md
./merge_request-2883-submodule-a/merge_request-2883-sub-submodule-b-1/.git
./merge_request-2883-submodule-a/.git

What are the relevant issue numbers?

Closes gitlab#331042 (closed)

Edited by Arran Walker

Merge request reports