Path is not mounted correctly when running Docker hooks from Docker

[okainov]

Situation:

• In our CI we want to run pre-commit inside Docker.
• Some of our hooks are docker_image

Problem
This line mostly

pre-commit/pre_commit/languages/docker.py

Line 94 in 528c7af

'-v', f'{os.getcwd()}:/src:rw,Z',

Currently pre-commit mounts the current directory to /src and uses current directory name as mount base.
However this does not work when pre-commit is run inside the container on some mounted path already, because mount points are relative to the host, not to the container.

Example:

/opt/my_code      <- host, mounts /opt/my_code:/project
/project              <- in Docker running pre-commit, pre-commit is doing mount /project:/src
/src                   <-  (in Dockerized hook)

Currently pre-commit will try to mount it as -v /project:/src,rw,Z. Expected - to mount it as -v /opt/my_code:/src

Possible solution:

When I replaced os.getcwd() from the code above to translate_path(os.getcwd()) where translate_path is taken from https://gist.github.com/dpfoose/f96d4e4b76c2e01265619d545b77987a, it worked perfectly. It does add extra docker pip-dependency though.

See also: https://forums.docker.com/t/mounting-a-volume-not-working-with-running-docker-in-docker/25775/2

[asottile]

the docker* hook type assume you're on the host -- changing that breaks some of our assumptions but it might be possible to implement your suggestion

the call into docker-py can be replaced with subprocess.check_call(('docker', 'inspect', hostname)) -- would you like to take a stab at fixing it?

[asottile]

@okainov would you be interested in working on this?

[okainov]

@asottile I'd really like to and I have also the update for not using docker dependency, but there might be some bureaucracy issues from my side, so I'm not sure whether I could publish it soon...

[djh82]

Is there any appetite for mounting in a similar way to jenkins, where the docker container essentially becomes transparent since it would be mounted like so:

-v os.getcwd():os.getcwd()

[asottile]

that would be a breaking change from the current set of guarantees

[okainov]

@asottile it took me some time :D But here is PR #1885, feel free to take a look and provide feedback.