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

Proposal: Dockerfile transactions #9408

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
7 participants
@emilymaier
Contributor

emilymaier commented Nov 30, 2014

I've implemented "transactions" in Dockerfiles, where instructions inside a transaction are all executed inside the same intermediate container. This makes all the instructions part of a single layer inside the final image. It closes #332 and #2439

In the image history, a transaction is given the fictional command of /bin/sh -c #(transaction) , similar to the #(nop) used for other instructions.

Currently only RUN instructions are supported inside a transaction, because allowing other instructions make caching far more difficult to implement for transactions.

@erikh

This comment has been minimized.

Contributor

erikh commented Nov 30, 2014

We have a number of proposals for similar features. @shykes can you weigh in on #2439 in particular?

-Erik

On Nov 29, 2014, at 6:05 PM, emilymaier notifications@github.com wrote:

I've implemented "transactions" in Dockerfiles, where instructions inside a transaction are all executed inside the same intermediate container. This makes all the instructions part of a single layer inside the final image. It closes #332 #332 and #2439 #2439
In the image history, a transaction is given the fictional command of /bin/sh -c #(transaction) , similar to the #(nop) used for other instructions.

Currently only RUN instructions are supported inside a transaction, because allowing other instructions make caching far more difficult to implement for transactions.

You can merge this Pull Request by running

git pull https://github.com/emilymaier/docker transactions
Or view, comment on, or merge it at:

#9408 #9408
Commit Summary

Implemented transaction support in Dockerfiles.
File Changes

M builder/dispatchers.go https://github.com/docker/docker/pull/9408/files#diff-0 (72)
M builder/evaluator.go https://github.com/docker/docker/pull/9408/files#diff-1 (57)
M builder/internals.go https://github.com/docker/docker/pull/9408/files#diff-2 (41)
M builder/parser/parser.go https://github.com/docker/docker/pull/9408/files#diff-3 (30)
M builder/parser/utils.go https://github.com/docker/docker/pull/9408/files#diff-4 (5)
M docs/sources/reference/builder.md https://github.com/docker/docker/pull/9408/files#diff-5 (20)
M integration-cli/docker_cli_build_test.go https://github.com/docker/docker/pull/9408/files#diff-6 (156)
M runconfig/compare.go https://github.com/docker/docker/pull/9408/files#diff-7 (30)
M runconfig/config.go https://github.com/docker/docker/pull/9408/files#diff-8 (9)
Patch Links:

https://github.com/docker/docker/pull/9408.patch https://github.com/docker/docker/pull/9408.patch
https://github.com/docker/docker/pull/9408.diff https://github.com/docker/docker/pull/9408.diff

Reply to this email directly or view it on GitHub #9408.

@SvenDowideit

This comment has been minimized.

Contributor

SvenDowideit commented Dec 1, 2014

mmm, as this implementation is limited to RUN its basically like having a long winded HEREDOC

@jessfraz jessfraz changed the title from Dockerfile transactions to Proposal: Dockerfile transactions Dec 2, 2014

@emilymaier

This comment has been minimized.

Contributor

emilymaier commented Dec 6, 2014

I definitely agree that more instructions are needed and I've implemented a version with them, but getting caching to work with it is hairy so I wanted more general feedback first.

Implemented transaction support in Dockerfiles.
Signed-off-by: Emily Maier <emily@emilymaier.net>

@emilymaier emilymaier force-pushed the emilymaier:transactions branch from 574451b to 4355809 Dec 8, 2014

@bfirsh

This comment has been minimized.

Contributor

bfirsh commented Jan 14, 2015

In terms of syntax, I think I prefer some kind of block with either indentation or braces.

This is also closely related to #7115. This is running multiple commands inside the current build, the other is spinning off a new build and running multiple commands. They should at least be a similar syntax, perhaps even variations on a similar operation.

@icecrime

This comment has been minimized.

Contributor

icecrime commented Mar 17, 2015

Thanks for your contribution, I'm sorry this PR got so little attention. Today, we don't believe that this feature is a priority, so I'm afraid I'm gonna have to close this.

Please feel free to join us on IRC and discuss such significant design changes to make sure that we don't ruin your implementation efforts for things that unfortunately don't get merged in the end.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment