Add build command #13
Conversation
marcomorain
force-pushed
the
refactor-common-code
branch
2 times, most recently
from
15af7f2
to
94e21c9
Compare
Port the majority of the current bash script that runs the existing CLI tool. The allows people to run `circleci build` locally using the new CLI.
cmd/build.go
Outdated
| func newBuildCommand() *cobra.Command { | ||
|
|
||
| command := &cobra.Command{ | ||
| Use: "build", |
There was a problem hiding this comment.
Since this command is still running a job (not a full build) I've been wondering if we should rename it to either run or job run. We can always add build as an alias for backwards compatbility.
cmd/build.go
Outdated
| */ | ||
|
|
||
| flags := command.Flags() | ||
| flags.StringVarP(&dockerAPIVersion, "docker-api-version", "d", dockerAPIVersion, "The Docker API version to use") |
There was a problem hiding this comment.
I don't think you want to duplicate all the flags here. There is a better way!
One option is to use flags.SetIntersperced(false) https://godoc.org/github.com/spf13/pflag#SetInterspersed. That way any flags after a positional argument (or a --) are kept in Args. It might also work for any not recognized flags, I don't remember. If that doesn't work I would just omit flgs all together and pass all the args along.
You're already appending args so that should be it.
There was a problem hiding this comment.
Oh sweet, that will make things easier 👌
cmd/build.go
Outdated
| // Otherwise pulling latest image in background | ||
| // TODO - this should write to a file to record | ||
| // the fact that we have the latest image. | ||
| newPullLatestImageCommand().Start() |
There was a problem hiding this comment.
I think we should reconsider this "pull latest in background on every run". I think it's unexpected behaviour and not a good user experience. This is Windows Updates.
Instead I think we could provide a subcommand to update it.
cmd/build.go
Outdated
|
|
||
| // TODO: marc: | ||
| // I can't find a way to pass `-it` and have carriage return | ||
| // characters work correctly. |
There was a problem hiding this comment.
Because you are setting stdout to a pipe, which is not a tty. You could use a pty, but I think there is a better option.
Instead of fork/exec, why not just exec (https://golang.org/pkg/syscall/#Exec)? That way you don't need to stream output (not sure how that works on windows). Alternatively just set Stdout/Stderr to os.Stdout/os.Stderr, which also fixes the tty issue and removes the need to stream output.
There was a problem hiding this comment.
That's a good idea. We will need to do something else on Windows I imagine?
There was a problem hiding this comment.
Ya, I think there will be other problems with windows as well. I don't think bind mounting /var/run/docker.sock works, because it uses a named pipe if I remember correctly.
cmd/build.go
Outdated
| // I can't find a way to pass `-it` and have carriage return | ||
| // characters work correctly. | ||
| arguments := []string{"run", "--rm", | ||
| "-e", fmt.Sprintf("DOCKER_API_VERSION=%s", dockerAPIVersion), |
There was a problem hiding this comment.
This doesn't need to be a flag. Use -e DOCKER_API_VERSION and it will use the value from cmd.Env, which defaults to os.Environ() which is exactly what you want I think.
This replaces the current process with the spawn process, simplifying everything.
marcomorain
force-pushed
the
refactor-common-code
branch
from
94e21c9
to
781bebe
Compare
|
:thatmotivatesme: |
marcomorain
force-pushed
the
refactor-common-code
branch
from
ca4d7cd
to
707ce2f
Compare
marcomorain
force-pushed
the
refactor-common-code
branch
from
94662bd
to
d16bd3b
Compare
Codecov Report
@@ Coverage Diff @@
## master #13 +/- ##
===========================================
+ Coverage 22.13% 39.34% +17.21%
===========================================
Files 9 10 +1
Lines 488 615 +127
===========================================
+ Hits 108 242 +134
+ Misses 366 357 -9
- Partials 14 16 +2
Continue to review full report at Codecov.
|
Port the majority of the current bash script that runs the existing CLI tool.
The allows people to run
circleci buildlocally using the new CLI.