Closed ansemjo closed 5 years ago
Merging #256 into master will not change coverage. The diff coverage is
0%
.
@@ Coverage Diff @@
## master #256 +/- ##
=====================================
Coverage 0% 0%
=====================================
Files 16 16
Lines 957 984 +27
=====================================
- Misses 957 984 +27
Impacted Files | Coverage Δ | |
---|---|---|
build.go | 0% <0%> (ø) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 5a8119f...0250f1a. Read the comment docs.
I force-pushed a few changes. I think I'm handling all known exporter types now.
Docs / README probably need an update, too?
Added a section to the README and looking at tests now.
All tests are run with the doRun()
function at their core it seems? This function only returns combined stderr and stdout, which is a problem if -- for example -- I'd like to test wether the stdout is a valid tar archive and contains an expected file. If I leave that particular test out I could add a few tests for different exporter types or missing fields, etc.
I've added a few tests now and TestBuildOutputTarStdout
is failing because there is Buildkit output on Stdout. It works fine when running the built binary in a shell manually with:
cat Dockerfile | ./img build - -o - | tar tvf -
Is there a flag on the session I need to explicitly set to tell the progress output to not use stdout?
edit: Yes, I amended the showProgress
function to optionally display to stderr. The reason it worked in a shell is because os.Stderr
is a valid console and progress was shown there instead.
A couple of open questions:
Building ...
, Setting up ...
and Successfully built ...
messages to Stderr permanently ok?stdoutUsed
variable is only set to true
when the special case -
is used. The equivalent case of -o type=tar,dest=/dev/stdout
is not handled and the output is mangled when os.Stderr
is not a valid console for the progress output.What do you think, @AkihiroSuda?
The latest push fails in Travis, because I've switched to using the upstream ParseOutput
func from moby/buildkit/cmd/buildctl/build
, which is not vendored. What's the preferred solution? Update vendored files or copy relevant source files? The upstream version has updated the parsing a little and e.g. removed the special case for -o -
.
Just running make vendor
includes a lot of new crypto too:
git status
[...]
vendor/github.com/moby/buildkit/cmd/
vendor/github.com/moby/buildkit/session/secrets/secretsprovider/
vendor/github.com/moby/buildkit/session/sshforward/sshprovider/
vendor/golang.org/x/crypto/curve25519/
vendor/golang.org/x/crypto/ed25519/
vendor/golang.org/x/crypto/internal/
vendor/golang.org/x/crypto/poly1305/
vendor/golang.org/x/crypto/ssh/agent/
vendor/golang.org/x/crypto/ssh/buffer.go
vendor/golang.org/x/crypto/ssh/certs.go
vendor/golang.org/x/crypto/ssh/channel.go
vendor/golang.org/x/crypto/ssh/cipher.go
vendor/golang.org/x/crypto/ssh/client.go
vendor/golang.org/x/crypto/ssh/client_auth.go
vendor/golang.org/x/crypto/ssh/common.go
vendor/golang.org/x/crypto/ssh/connection.go
vendor/golang.org/x/crypto/ssh/doc.go
vendor/golang.org/x/crypto/ssh/handshake.go
vendor/golang.org/x/crypto/ssh/kex.go
vendor/golang.org/x/crypto/ssh/keys.go
vendor/golang.org/x/crypto/ssh/mac.go
vendor/golang.org/x/crypto/ssh/messages.go
vendor/golang.org/x/crypto/ssh/mux.go
vendor/golang.org/x/crypto/ssh/server.go
vendor/golang.org/x/crypto/ssh/session.go
vendor/golang.org/x/crypto/ssh/streamlocal.go
vendor/golang.org/x/crypto/ssh/tcpip.go
vendor/golang.org/x/crypto/ssh/transport.go
Yay! :tada:
This is a ~first -- very rough --~ patch to port the build output flag from buildkit. I saw @AkihiroSuda's #225 and thought this would be a killer feature, so I tried my hand at it. It's mostly copy-pasted snippets from @tonistiigi's patch and the buildkit client.
Current issues:
type=
s. Is support for all possible types necessary or should it be limited totar
andlocal
?~-t tag
flag when-o
is used because it does not make sense fortar
orlocal
output and is given in the form of aname
field for imagesBuilding ...
,Setting up ...
andSuccessfully built ...
prints because it gets in the way of tar output to stdout.~ The thing is, we only properly parse the output spec after the session is created, so we can't easily use thestdoutUsed
approach or I don't yet see how without splitting the parser further. edit: I simply let them print tostderr
since the buildkit output lands there already anyway.~It's not meant for merging just yet but~ with this patch I can build quasi-arbitrary artifacts with a multi-stage Dockerfile, where the last stage
FROM scratch
only copies files of interest: