sourcegraph / src-cli

Sourcegraph CLI
https://sourcegraph.com
Apache License 2.0
267 stars 57 forks source link

Replace `unzip` with `bsdtar` in the Batch Change Volume Workspace image #1020

Open lowjoel opened 11 months ago

lowjoel commented 11 months ago

There are situations where a Zip archive from Sourcegraph causes the directory to be improperly recreated within the container. Using libarchive-tools fixes this.

Example broken output:

creating workspace: preparing local git repo: preparing workspace: Docker output:
+ git init
hint: Using 'master' as the name for the initial branch. This default branch name
hint: is subject to change. To configure the initial branch name to use in all
hint: of your new repositories, which will suppress this warning, call:
hint: 
hint:   git config --global init.defaultBranch <name>
hint: 
hint: Names commonly chosen instead of 'master' are 'main', 'trunk' and
hint: 'development'. The just-created branch can be renamed via this command:
hint: 
hint:   git branch -m <name>
Initialized empty Git repository in /work/.git/
+ git config user.name 'Sourcegraph Batch Changes'
+ git config user.email batch-changes@sourcegraph.com
+ git add --force --all
error: invalid path 'path-to-dir/.gitmodules'
error: unable to add 'path-to-dir/.gitmodules' to index
fatal: adding files failed: exit status 128

I then docker-exec'ed into the container. Unzip was creating .gitmodules as a symlink with the file contents rather than a normal file. Both the Git repository and Sourcegraph show the file correctly:

$ ls -la
total 72
drwxr-xr-x    6 root     root          4096 Apr 18 13:01 .
drwxr-xr-x   10 root     root          4096 Apr 18 13:01 ..
-rw-r--r--    1 root     root            47 Apr 18 13:01 .dockerignore
-rw-r--r--    1 root     root            85 Apr 18 13:01 .gitignore
lrwxrwxrwx    1 root     root            79 Aug 16 05:36 .gitmodules -> <file content redacted>

Unzipping this on my Mac host directly yields the correct directory. This means that the Zip file is fine, the unzip process is broken. Using bsdtar causes the directory to be correctly created.

Test plan

  1. Rebuild a new Docker image for the batch-change-volume-workspace image
  2. Locally, I've run my previously-failing batch change
  3. The new Docker image + recompiled src-cli completes the preview without errors.
eseliger commented 11 months ago

Hey and thanks for the fix! I'm trying to understand how this happens:

Unzipping this on my Mac host directly yields the correct directory. This means that the Zip file is fine, the unzip process is broken. Using bsdtar causes the directory to be correctly created.

What exactly are you unzipping? The same zip file that Sourcegraph gave src-cli, or one from the code host? (Just want to make sure our zip endpoint is not flawed instead). Also, is there any chance to get a minimally reproducible version of the repo you're seeing that error with? Can be completely of bogus content, just the same file structure that causes this issue to happen :)

lowjoel commented 11 months ago

What exactly are you unzipping? The same zip file that Sourcegraph gave src-cli, or one from the code host?

The one Sourcegraph gave the CLI when trying to perform the batch change. That's the raw endpoint:

https://sourcegraph.<domain>/<repo-path>/-/raw?format=zip

Also, is there any chance to get a minimally reproducible version of the repo you're seeing that error with? Can be completely of bogus content, just the same file structure that causes this issue to happen :)

I have no idea what causes it. Maybe it's size? I observed it on a 5Gib zip archive, but I don't know what may cause such a problem to manifest. I couldn't find a common theme across the different repositories that failed in this manner when running the batch change preview.

lowjoel commented 11 months ago

Fixed tests.