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

"EXTEND" to mark "non-instantiating" extension that doesnt run "ONBUILD" #11917

Closed
shaunc opened this Issue Mar 30, 2015 · 11 comments

Comments

Projects
None yet
5 participants
@shaunc

shaunc commented Mar 30, 2015

I have a base image nodebase for node apps which has

ONBUILD ADD package.json /tmp/package.json
ONBUILD RUN cd /tmp && npm install
ONBUILD RUN mkdir -p /app && cp -a /tmp/node_modules /app

I'd like to define an extension of it that includes postgres tools in the image. However, I don't want to have the "ONBUILDS" triggered in the extension -- rather they should be triggered in the "final" build with the app.

It would be nice to have

EXTEND nodebase

as a variant of from which supresses the "ONBUILD"s -- another Dockerfile built FROM the extension would then trigger them.

I see there are various proposals for "ONBUILD" floating around. Perhaps EXTEND would be a good place to control the staging of ONBUILDs, so that FROM could be kept simple.

@erikh

This comment has been minimized.

Show comment
Hide comment
@erikh

erikh Mar 30, 2015

Contributor

Check out the proposals for various INCLUDE statements or "Nested Builds".

I don't think this identifies any new behavior from them.

-Erik

On Mar 29, 2015, at 9:19 PM, Shaun Cutts notifications@github.com wrote:

I have a base image nodebase for node apps which has

ONBUILD ADD package.json /tmp/package.json
ONBUILD RUN cd /tmp && npm install
ONBUILD RUN mkdir -p /app && cp -a /tmp/node_modules /app
I'd like to define an extension of it that includes postgres tools in the image. However, I don't want to have the "ONBUILDS" triggered in the extension -- rather they should be triggered in the "final" build with the app.

It would be nice to have

EXTEND nodebase
as a variant of from which supresses the "ONBUILD"s -- another Dockerfile built FROM the extension would then trigger them.

I see there are various proposals for "ONBUILD" floating around. Perhaps EXTEND would be a good place to control the staging of ONBUILDs, so that FROM could be kept simple.


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

Contributor

erikh commented Mar 30, 2015

Check out the proposals for various INCLUDE statements or "Nested Builds".

I don't think this identifies any new behavior from them.

-Erik

On Mar 29, 2015, at 9:19 PM, Shaun Cutts notifications@github.com wrote:

I have a base image nodebase for node apps which has

ONBUILD ADD package.json /tmp/package.json
ONBUILD RUN cd /tmp && npm install
ONBUILD RUN mkdir -p /app && cp -a /tmp/node_modules /app
I'd like to define an extension of it that includes postgres tools in the image. However, I don't want to have the "ONBUILDS" triggered in the extension -- rather they should be triggered in the "final" build with the app.

It would be nice to have

EXTEND nodebase
as a variant of from which supresses the "ONBUILD"s -- another Dockerfile built FROM the extension would then trigger them.

I see there are various proposals for "ONBUILD" floating around. Perhaps EXTEND would be a good place to control the staging of ONBUILDs, so that FROM could be kept simple.


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

@duglin

This comment has been minimized.

Show comment
Hide comment
@duglin

duglin Mar 30, 2015

Contributor

@shaunc ONBUILD is easy because its basically asking for those steps be executed the next time the image is used during a build. I believe you're asking for EXTEND to be executed the last time an image is used in a build. I'm not sure how we can determine that because there's no way to know if "this" build is the last one or just one of many that the user may do. Or am I missing something?

Contributor

duglin commented Mar 30, 2015

@shaunc ONBUILD is easy because its basically asking for those steps be executed the next time the image is used during a build. I believe you're asking for EXTEND to be executed the last time an image is used in a build. I'm not sure how we can determine that because there's no way to know if "this" build is the last one or just one of many that the user may do. Or am I missing something?

@sheetweaver

This comment has been minimized.

Show comment
Hide comment
@sheetweaver

sheetweaver Mar 30, 2015

@erikh -- there are several different proposals that seem to cover a superset of the functionality here. I proposed this because I thought it would be simpler to implement. I'm not in a position to implement myself, though, so if the community likes a different proposal, please go ahead. For the record I would be happy with (a version of) INCLUDE as well. I thought it might be harder to coordinate with the layering implementation.

@duglin -- The EXTEND keyword just suspends any ONBUILDs in the parent -- they are still queued up for any use of FROM: so not the last time, but the first "non-EXTEND" time.

sheetweaver commented Mar 30, 2015

@erikh -- there are several different proposals that seem to cover a superset of the functionality here. I proposed this because I thought it would be simpler to implement. I'm not in a position to implement myself, though, so if the community likes a different proposal, please go ahead. For the record I would be happy with (a version of) INCLUDE as well. I thought it might be harder to coordinate with the layering implementation.

@duglin -- The EXTEND keyword just suspends any ONBUILDs in the parent -- they are still queued up for any use of FROM: so not the last time, but the first "non-EXTEND" time.

@erikh

This comment has been minimized.

Show comment
Hide comment
@erikh

erikh Mar 30, 2015

Contributor

This is probably a @shykes call. @crosbymichael ?

-Erik

On Mar 30, 2015, at 7:23 AM, sheetweaver notifications@github.com wrote:

@erikh https://github.com/erikh -- there are several different proposals that seem to cover a superset of the functionality here. I proposed this because I thought it would be simpler to implement. I'm not in a position to implement myself, though, so if the community likes a different proposal, please go ahead. For the record I would be happy with (a version of) INCLUDE as well. I thought it might be harder to coordinate with the layering implementation.

@duglin https://github.com/duglin -- The EXTEND keyword just suspends any ONBUILDs in the parent -- they are still queued up for any use of FROM: so not the last time, but the first "non-EXTEND" time.


Reply to this email directly or view it on GitHub #11917 (comment).

Contributor

erikh commented Mar 30, 2015

This is probably a @shykes call. @crosbymichael ?

-Erik

On Mar 30, 2015, at 7:23 AM, sheetweaver notifications@github.com wrote:

@erikh https://github.com/erikh -- there are several different proposals that seem to cover a superset of the functionality here. I proposed this because I thought it would be simpler to implement. I'm not in a position to implement myself, though, so if the community likes a different proposal, please go ahead. For the record I would be happy with (a version of) INCLUDE as well. I thought it might be harder to coordinate with the layering implementation.

@duglin https://github.com/duglin -- The EXTEND keyword just suspends any ONBUILDs in the parent -- they are still queued up for any use of FROM: so not the last time, but the first "non-EXTEND" time.


Reply to this email directly or view it on GitHub #11917 (comment).

@duglin

This comment has been minimized.

Show comment
Hide comment
@duglin

duglin Mar 30, 2015

Contributor

Can you elaborate on how INCLUDE would solve the issue for you? I viewed INCLUDE as just a mechanism by which you could split your Dockerfile up into smaller pieces, so just a parsing thing. Do you think it changes how the builder interprets the data in the Dockerfiles?

Contributor

duglin commented Mar 30, 2015

Can you elaborate on how INCLUDE would solve the issue for you? I viewed INCLUDE as just a mechanism by which you could split your Dockerfile up into smaller pieces, so just a parsing thing. Do you think it changes how the builder interprets the data in the Dockerfiles?

@sheetweaver

This comment has been minimized.

Show comment
Hide comment
@sheetweaver

sheetweaver Mar 30, 2015

I guess there are two differences w/ INCLUDE (at least as I picture it):

  1. EXTEND is meant for the start of a Dockerfile (like FROM -- what happens if you have a FROM in the middle, btw?)
  2. EXTEND references an image, whereas INCLUDE references a Dockerfile itself.

I'm conceptualizing an image built directly from a Dockerfile with ONBUILD commands as an "abstract" image. EXTEND means "dont instantiate yet -- add to that abstract image."

If you have the Dockerfile associated with an image onhand, then

  1. Use INCLUDE at start of "derived" Dockerfile referencing the "super" Dockerfile, and
  2. EXTEND the image built by the "super" Dockerfile

amount to the same thing, with the caveat that in the first case, the "image you are extending" is the layer in the cache created by running the "super" Dockerfile, whereas in the 2nd case you use docker resolution to find an image.

sheetweaver commented Mar 30, 2015

I guess there are two differences w/ INCLUDE (at least as I picture it):

  1. EXTEND is meant for the start of a Dockerfile (like FROM -- what happens if you have a FROM in the middle, btw?)
  2. EXTEND references an image, whereas INCLUDE references a Dockerfile itself.

I'm conceptualizing an image built directly from a Dockerfile with ONBUILD commands as an "abstract" image. EXTEND means "dont instantiate yet -- add to that abstract image."

If you have the Dockerfile associated with an image onhand, then

  1. Use INCLUDE at start of "derived" Dockerfile referencing the "super" Dockerfile, and
  2. EXTEND the image built by the "super" Dockerfile

amount to the same thing, with the caveat that in the first case, the "image you are extending" is the layer in the cache created by running the "super" Dockerfile, whereas in the 2nd case you use docker resolution to find an image.

@sheetweaver

This comment has been minimized.

Show comment
Hide comment
@sheetweaver

sheetweaver Mar 30, 2015

(So -- there may be cases where EXTEND would go beyond INCLUDE -- when you need/want Docker resolution -- say in automation, when you want to EXTEND some library image and don't know/care what is in the Dockerfile. In the case I was working on right now I have the Dockerfiles.)

sheetweaver commented Mar 30, 2015

(So -- there may be cases where EXTEND would go beyond INCLUDE -- when you need/want Docker resolution -- say in automation, when you want to EXTEND some library image and don't know/care what is in the Dockerfile. In the case I was working on right now I have the Dockerfiles.)

@duglin

This comment has been minimized.

Show comment
Hide comment
@duglin

duglin Mar 30, 2015

Contributor

Ah thanks - now I understand it better (I think).
In that case I would actually suggest that if we did want a way to "turn off" the ONBUILD stuff, I wonder if supporting something like:

FROM --defer-on-build [image]

might be more natural for people, rather than an entirely new command.
Of course, it requires: #10775 first.

Contributor

duglin commented Mar 30, 2015

Ah thanks - now I understand it better (I think).
In that case I would actually suggest that if we did want a way to "turn off" the ONBUILD stuff, I wonder if supporting something like:

FROM --defer-on-build [image]

might be more natural for people, rather than an entirely new command.
Of course, it requires: #10775 first.

@sheetweaver

This comment has been minimized.

Show comment
Hide comment
@sheetweaver

sheetweaver Mar 30, 2015

So far Dockerfile directives don't have (many?) options... I would thow out for consideration that introducing a separate command keeps the language simpler, as users who don't need it will find it easier to ignore. I don't have a strong opinion, however. (EDIT -- oh thats what the issue you referenced is about :) sorry for not reading immediately)

sheetweaver commented Mar 30, 2015

So far Dockerfile directives don't have (many?) options... I would thow out for consideration that introducing a separate command keeps the language simpler, as users who don't need it will find it easier to ignore. I don't have a strong opinion, however. (EDIT -- oh thats what the issue you referenced is about :) sorry for not reading immediately)

@duglin

This comment has been minimized.

Show comment
Hide comment
@duglin

duglin Mar 30, 2015

Contributor

Its a toughie because as an example we have ADD and COPY which are almost identical and trying to explain the difference to people is really annoying. I would have preferred: COPY --do-add-extra-stuff ... over an entirely new command. But we'll see...

Contributor

duglin commented Mar 30, 2015

Its a toughie because as an example we have ADD and COPY which are almost identical and trying to explain the difference to people is really annoying. I would have preferred: COPY --do-add-extra-stuff ... over an entirely new command. But we'll see...

@jessfraz

This comment has been minimized.

Show comment
Hide comment
@jessfraz

jessfraz Jul 10, 2015

Contributor

Hello!
We are no longer accepting patches to the Dockerfile syntax as you can read about here: https://github.com/docker/docker/blob/master/ROADMAP.md#22-dockerfile-syntax

Mainly:

Allowing the Builder to be implemented as a separate utility consuming the Engine's API will open the door for many possibilities, such as offering alternate syntaxes or DSL for existing languages without cluttering the Engine's codebase

Then from there, patches/features like this can be re-thought. Hope you can understand.

Contributor

jessfraz commented Jul 10, 2015

Hello!
We are no longer accepting patches to the Dockerfile syntax as you can read about here: https://github.com/docker/docker/blob/master/ROADMAP.md#22-dockerfile-syntax

Mainly:

Allowing the Builder to be implemented as a separate utility consuming the Engine's API will open the door for many possibilities, such as offering alternate syntaxes or DSL for existing languages without cluttering the Engine's codebase

Then from there, patches/features like this can be re-thought. Hope you can understand.

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