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

progress: if not escaped, backslashes \ not printed on the progress stream dockerfile steps #5250

Open
profnandaa opened this issue Aug 13, 2024 · 3 comments · May be fixed by #5269
Open

progress: if not escaped, backslashes \ not printed on the progress stream dockerfile steps #5250

profnandaa opened this issue Aug 13, 2024 · 3 comments · May be fixed by #5269
Labels

Comments

@profnandaa
Copy link
Collaborator

profnandaa commented Aug 13, 2024

However, prints when escaped or put within "..":

Minimal repro dockerfile:

FROM mcr.microsoft.com/windows/nanoserver:ltsc2022

RUN echo C:\hello\world\path
RUN echo C:\\hello\\escaped\\path
RUN echo "C:\hello\quoted\path"

Result:

#5 [2/4] RUN echo C:helloworldpath
#5 1.359 C:\hello\world\path
#5 DONE 1.7s

#6 [3/4] RUN echo C:\hello\escaped\path
#6 1.734 C:\\hello\\escaped\\path
#6 DONE 2.1s

#7 [4/4] RUN echo "C:\hello\quoted\path"
#7 1.765 "C:\hello\quoted\path"
#7 DONE 2.1s

With classic docker build:

Result:

Step 2/4 : RUN echo C:\hello\world\path
 ---> Running in 94d37a87e491
C:\hello\world\path
 ---> Removed intermediate container 94d37a87e491
 ---> b098d8792b7c
Step 3/4 : RUN echo C:\\hello\\escaped\\path
 ---> Running in d2fcea089fb1
C:\\hello\\escaped\\path
 ---> Removed intermediate container d2fcea089fb1
 ---> e6c9869a97f8
Step 4/4 : RUN echo "C:\hello\quoted\path"
 ---> Running in 2e2c603aebe2
"C:\hello\quoted\path"
 ---> Removed intermediate container 2e2c603aebe2
 ---> 56ba89eb540e
@ardrabczyk
Copy link
Contributor

It can also be reproduced on Linux:

$ cat Dockerfile
FROM alpine

RUN echo mkdir C:\another\sample
RUN echo mkdir C:\\another\\sample
$ docker build -t backslash-test --no-cache .
[+] Building 0.8s (7/7) FINISHED                                                                                                                                                                     docker:default
 => [internal] load .dockerignore                                                                                                                                                                              0.0s
 => => transferring context: 2B                                                                                                                                                                                0.0s
 => [internal] load build definition from Dockerfile                                                                                                                                                           0.0s
 => => transferring dockerfile: 118B                                                                                                                                                                           0.0s
 => [internal] load metadata for docker.io/library/alpine:latest                                                                                                                                               0.2s
 => CACHED [1/3] FROM docker.io/library/alpine@sha256:0a4eaa0eecf5f8c050e5bba433f58c052be7587ee8af3e8b3910ef9ab5fbe9f5                                                                                         0.0s
 => [2/3] RUN echo mkdir C:anothersample                                                                                                                                                                       0.2s
 => [3/3] RUN echo mkdir C:\another\sample                                                                                                                                                                     0.2s
 => exporting to image                                                                                                                                                                                         0.1s
 => => exporting layers                                                                                                                                                                                        0.1s
 => => writing image sha256:cfbd890f4a908e48975d6c462a8b7b447effe943208c64625f2e0601942af236                                                                                                                   0.0s
 => => naming to docker.io/library/backslash-test                                                                                                                                                              0.0s

@profnandaa profnandaa changed the title WCOW: if not escaped, backslashes \ not printed on the progress stream dockerfile steps progress: if not escaped, backslashes \ not printed on the progress stream dockerfile steps Aug 22, 2024
@profnandaa
Copy link
Collaborator Author

profnandaa commented Aug 22, 2024

@ardrabczyk -- good catch, thanks! Updated the title and description.

@profnandaa
Copy link
Collaborator Author

profnandaa commented Aug 22, 2024

I have narrowed down to this, frontend/dockerfile/dockerfile2llb/convert.go:2055

> github.com/moby/buildkit/frontend/dockerfile/dockerfile2llb.processCmdEnv() C:/dev/buildkit/frontend/dockerfile/dockerfile2llb/convert.go:2057 (PC: 0x238dec1)

  2055: func processCmdEnv(shlex *shell.Lex, cmd string, env shell.EnvGetter) string {
  2056:         w, _, err := shlex.ProcessWord(cmd, env)
=>2057:         if err != nil {
  2058:                 return cmd
  2059:         }
  2060:         return w
  2061: }
  2062:
(dlv) p cmd
"RUN echo C:\\hello\\world\\path"
(dlv) p w
"RUN echo C:helloworldpath"
~~~~~~~~~~~^^--- backslashes being stripped off by the lexer.
(dlv)

I'm not really conversant with this lexer, will appreciate any tips? @tonistiigi

profnandaa added a commit to profnandaa/buildkit that referenced this issue Aug 22, 2024
`shlex` in was treating single backslash "\\" in string as
escape sequence, hence stripping them off. Modify `processCmdEnv`
to double escape `\\`.

This was in turn affecting the vertex `Name`, and mostly pronounced
on Windows where backslashes are prevalent in path names. For example
`C:\hello\world\path` was being rendered as `C:helloworldpath`, hence
confusing users.

Fixes moby#5250

Build progress before fix:
```
    moby#5 [2/4] RUN echo C:helloworldpath
    moby#5 1.359 C:\hello\world\path
    moby#5 DONE 1.7s

    moby#6 [3/4] RUN echo C:\hello\escaped\path
    moby#6 1.734 C:\\hello\\escaped\\path
    moby#6 DONE 2.1s

    moby#7 [4/4] RUN echo "C:\hello\quoted\path"
    moby#7 1.765 "C:\hello\quoted\path"
    moby#7 DONE 2.1s
```

Build progress after fix:
```
	moby#5 [2/4] RUN echo C:\hello\world\path
	moby#5 1.458 C:\hello\world\path
	moby#5 DONE 1.8s

	moby#6 [3/4] RUN echo C:\\hello\\escaped\\path
	moby#6 1.730 C:\\hello\\escaped\\path
	moby#6 DONE 2.1s

	moby#7 [4/4] RUN echo "C:\hello\quoted\path"
	moby#7 1.702 "C:\hello\quoted\path"
	moby#7 DONE 2.1s
```

You can also see that now paths in the step/vertex names now
match those in the log lines right after the name.

Signed-off-by: Anthony Nandaa <[email protected]>
profnandaa added a commit to profnandaa/buildkit that referenced this issue Aug 22, 2024
`shlex` in was treating single backslash "\\" in string as
escape sequence, hence stripping them off. Modify `processCmdEnv`
to double escape `\\`.

This was in turn affecting the vertex `Name`, and mostly pronounced
on Windows where backslashes are prevalent in path names. For example
`C:\hello\world\path` was being rendered as `C:helloworldpath`, hence
confusing users.

Fixes moby#5250

Build progress before fix:
```
	moby#5 [2/4] RUN echo C:helloworldpath
	moby#5 1.359 C:\hello\world\path
	moby#5 DONE 1.7s

	moby#6 [3/4] RUN echo C:\hello\escaped\path
	moby#6 1.734 C:\\hello\\escaped\\path
	moby#6 DONE 2.1s

	moby#7 [4/4] RUN echo "C:\hello\quoted\path"
	moby#7 1.765 "C:\hello\quoted\path"
	moby#7 DONE 2.1s
```

Build progress after fix:
```
	moby#5 [2/4] RUN echo C:\hello\world\path
	moby#5 1.458 C:\hello\world\path
	moby#5 DONE 1.8s

	moby#6 [3/4] RUN echo C:\\hello\\escaped\\path
	moby#6 1.730 C:\\hello\\escaped\\path
	moby#6 DONE 2.1s

	moby#7 [4/4] RUN echo "C:\hello\quoted\path"
	moby#7 1.702 "C:\hello\quoted\path"
	moby#7 DONE 2.1s
```

You can also see that now paths in the step/vertex names now
match those in the log lines right after the name.

Signed-off-by: Anthony Nandaa <[email protected]>
profnandaa added a commit to profnandaa/buildkit that referenced this issue Sep 9, 2024
`shlex` in was treating single backslash "\\" in string as
escape sequence, hence stripping them off. Modify `processCmdEnv`
to double escape `\\`.

This was in turn affecting the vertex `Name`, and mostly pronounced
on Windows where backslashes are prevalent in path names. For example
`C:\hello\world\path` was being rendered as `C:helloworldpath`, hence
confusing users.

Fixes moby#5250

Build progress before fix:
```
	moby#5 [2/4] RUN echo C:helloworldpath
	moby#5 1.359 C:\hello\world\path
	moby#5 DONE 1.7s

	moby#6 [3/4] RUN echo C:\hello\escaped\path
	moby#6 1.734 C:\\hello\\escaped\\path
	moby#6 DONE 2.1s

	moby#7 [4/4] RUN echo "C:\hello\quoted\path"
	moby#7 1.765 "C:\hello\quoted\path"
	moby#7 DONE 2.1s
```

Build progress after fix:
```
	moby#5 [2/4] RUN echo C:\hello\world\path
	moby#5 1.458 C:\hello\world\path
	moby#5 DONE 1.8s

	moby#6 [3/4] RUN echo C:\\hello\\escaped\\path
	moby#6 1.730 C:\\hello\\escaped\\path
	moby#6 DONE 2.1s

	moby#7 [4/4] RUN echo "C:\hello\quoted\path"
	moby#7 1.702 "C:\hello\quoted\path"
	moby#7 DONE 2.1s
```

You can also see that now paths in the step/vertex names now
match those in the log lines right after the name.

Signed-off-by: Anthony Nandaa <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants