Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lowjoel
Copy link
Contributor

@lowjoel lowjoel commented Aug 17, 2023

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 [email protected]
+ 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
Copy link
Member

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
Copy link
Contributor Author

lowjoel commented Aug 21, 2023

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.

There are situations where a Zip archive from Sourcegraph causes the
directory to be improperly recreated within the container. Using
libarchive-tools fixes this.
@lowjoel lowjoel force-pushed the replace-unzip-bsdtar branch from 0334ed9 to ed8be10 Compare August 22, 2023 02:30
@lowjoel
Copy link
Contributor Author

lowjoel commented Aug 22, 2023

Fixed tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants