Cleanup: Refactor Dockerfile parser

This completely rewrites the Dockerfile parser/evaluator. Most logic is not changed and all tests pass related to the dockerfile handling (including building docker itself). See below for caveats. I will squash this after the review is done unless requested otherwise. I realize the commits are kind of a mess right now.

**This is a huge change and I need testers and broken Dockerfiles.**

It implements two new packages: `builder/parser` and `builder/evaluator`. The parser parses the Dockerfile, and the evaluator evaluates it and executes the instructions. Both use a dispatcher system to hook statement parsing and evaluation of parsed statements. The result is a clean, extensible interface to both, one can adjust the syntax of the Dockerfile without affecting the processing, or at least if necessary will require minimal adjustment.

`builder/parser/dumper` can be installed with `go get` and runs against a dockerfile and outputs the statements in s-expression format.

**CHANGES**:

* env handling is across all statements now. `WORKDIR`, `VOLUME`, even `ENTRYPOINT` can interpolate environment variables now. There have been a lot of requests for this in the issue tracker, but the old parser was too brittle to support it. I took the liberty of implementing this change.
* ONBUILD structure changes. This ends up in no different functionality, but because of the way it is dispatched, "step" notifications for ONBUILD instructions have their own effective "stack" and start at step 0 in the output.

**SUGGESTIONS FOR ADDITIONAL REFACTORING**:

I made no attempt to add new functionality because I did not want to cloud the PR. However, I think these things should be done:

* `builder/evaluator/internals.go` needs to be broken out into another library, and the functionality needs to be partitioned cleaner.
* features: `docker build --debug` should be a thing to output the parser result, and `docker build --shell` could allow us to provide a REPL to building docker images without `docker commit`.
* syntax transformations: it is more possible now than ever to manipulate how dockerfiles are parsed and evaluated as separate entities, we should at least discuss the option of higher level DSLs which can inject themselves into this process. Related, statement proposals like `INCLUDE` and `IN` can be functions of hte parser and the evaluator can mostly be left alone.

Last but not least, given the impact of this change I would like to get it in soon, or block other builder edits until it gets in. I can port simple requests into this PR instead.
Both are needed in internals, and pretty much nowhere else. I hope to move some of this stuff away from the builder core in the future and this is an easier transition (or at least I saw it that way) in the future.

If you have another approach you’d like me to take, lmk.


On Aug 25, 2014, at 2:40 PM, Tibor Vass <notifications@github.com> wrote:

> In builder/evaluator.go:
> 
> > +	Options    *BuildOpts        // see below
> > +
> > +	// both of these are controlled by the Remove and ForceRemove options in BuildOpts
> > +	TmpContainers map[string]struct{} // a map of containers used for removes
> > +
> > +	image       string         // image name for commit processing
> > +	maintainer  string         // maintainer name. could probably be removed.
> > +	cmdSet      bool           // indicates is CMD was set in current Dockerfile
> > +	context     *tarsum.TarSum // the context is a tarball that is uploaded by the client
> > +	contextPath string         // the path of the temporary directory the local context is unpacked to (server side)
> > +
> > +}
> > +
> > +type BuildOpts struct {
> > +	Daemon *daemon.Daemon
> > +	Engine *engine.Engine
> Might be a stupid question, but why put Daemon and Engine in BuildOpts?
> 
> —
> Reply to this email directly or view it on GitHub.
>