Allow for Dockerfile to be named something else. #9707

Merged
merged 1 commit into from Jan 7, 2015

Projects

None yet
@duglin
Contributor
duglin commented Dec 17, 2014

This is a follow-on to #7995 and #7284.

This PR allows for the Dockerfile to be named something other than Dockerfile via a -f/--file option. Aside from the name/path of the Dockerfile being variable, nothing else should change from today's behavior. The location of the Dockerfile MUST be within the build context.

This PR contains the:

  • code changes
  • docs updates
  • testcases

Signed-off-by: Doug Davis dug@us.ibm.com

@crosbymichael crosbymichael added this to the 1.5.0 milestone Dec 17, 2014
@crosbymichael
Member

@duglin you are so fast!

@crosbymichael crosbymichael commented on an outdated diff Dec 17, 2014
api/client/commands.go
if err != nil {
return fmt.Errorf("Bad .dockerignore pattern: '%s', error: %s", pattern, err)
}
if ok {
- return fmt.Errorf("Dockerfile was excluded by .dockerignore pattern '%s'", pattern)
+ return fmt.Errorf("Dockerfile(", *dockerfileName, "was excluded by .dockerignore pattern '%s'", pattern)
@crosbymichael
crosbymichael Dec 17, 2014 Member

This looks kinda weird, can you please use the traditional printf format here?

@jessfraz jessfraz commented on an outdated diff Dec 17, 2014
integration-cli/docker_cli_build_test.go
@@ -4052,3 +4052,69 @@ CMD cat /foo/file`,
logDone("build - volumes retain contents in build")
}
+
+func TestBuildRenamedDockerfile(t *testing.T) {
+ name := "testbuildrenameddockerfile"
+ defer deleteImages(name)
@jessfraz
jessfraz Dec 17, 2014 Contributor

maybe Drone is failing because we need defer deleteContainers, to get rid of residual containers from builds, unsure if deleteImages handles this

@cyphar
Contributor
cyphar commented Dec 17, 2014

IMO, I don't like that the Dockerfiles use the root context. While it may make sense from a short-sighted end-user point of view, there's an issue regarding symmetry.

Specifically:

$ docker build -f dockerfiles/some-file
$ cd dockerfiles && docker build -f some-file

The above two lines are not synonymous, because the Dockerfiles are built in different contexts. This results in two (IMO not acceptable) side effects:

  1. The above symmetry is broken. This means that the directory in which docker build is called is now important, despite having a flag that allows you to specify relative directories.
  2. Dockerfiles now need to be context aware on a different level -- they need to care about where they are called from.

IMHO, Dockerfiles specified by the -f flag should be used as the root of the context, not applied to the root context of the docker run command.

Just my $0.02.

@tianon
Member
tianon commented Dec 17, 2014

The "context" argument is still required, right?

@duglin
Contributor
duglin commented Dec 17, 2014

@tianon yes I think that's a typo in his example.
So, let's say you do this (starting at the root of my build context on the cli):
cd foo
docker build -f myDockerfile ..

Right now this will fail because the CLI blindly passed the -f value to the daemon w/o taking into account that we're in "foo" and the daemon will look for myDockerfile in the root of the build context. IOW, the CLI really should convert "-f myDockerfile" to "-f foo/myDockerfile" before it sends it to the REST API so the daemon will find it.

@cyphar I think you're correct but let's see what other think before I change it.

@duglin
Contributor
duglin commented Dec 17, 2014

@crosbymichael I fixed the errorf stuff you noticed.
@jfrazelle I got drone working. It turns out I had to name my containers as they were built. Go figure.

@tianon
Member
tianon commented Dec 17, 2014

Cool, SGTM. I think the CLI would then also need to throw an error when
attempting to use a Dockerfile from outside the context.

ADD/COPY are also going to get a little confusing here. :)

@duglin
Contributor
duglin commented Dec 17, 2014

@tianon yes, it already throws an error if -f (on the daemon) points to something outside of the context, but yes this will force an additional check on the CLI side.

@cyphar
Contributor
cyphar commented Dec 17, 2014

@tianon a little more confusing. The reason for the whole context beef is that Dockerfiles now will need to either reference things relative to the root context when ADD or COPYing (which means they must know where they should be called from, meaning you lose some level of predictability) or relative to the Dockerfile (which means you now have Dockerfiles that care where they are placed inside the repository).

Neither of those options sound favourable to me (also the above symmetry concerns are quite important IMO, and if we're going to add this new functionality we might as well get it right the first time :P).

@crosbymichael
Member

@cyphar but also, you don't have to use -f. The people who want this feature want more control and therefore must pay the portability cost that it comes with.

@duglin
Contributor
duglin commented Dec 17, 2014

right, I think you should be able to put your dockerfile anywhere you want, but in the end any relative paths in there (e.g. on ADD/COPY) are relative to the context root. I think the docs are pretty clear on this already.

@cyphar
Contributor
cyphar commented Dec 17, 2014

@crosbymichael Surely they can place Dockerfile.prod, Dockerfile.test, etc in the root directory and call it a day with -f in the example? It's not just a portability thing (although that is an issue), it's a symmetry and consistency thing.

But idk, I might be acting far too idealistically for a feature like this.

@duglin
Contributor
duglin commented Dec 17, 2014

@cyphar are you saying you don't think I should make your suggested change?
Seemed right to me.

@cyphar
Contributor
cyphar commented Dec 17, 2014

@duglin Nono, I think that the change should be made. I just thought I was the only one who thought that :P.

@LK4D4
Contributor
LK4D4 commented Dec 17, 2014

I'm not sure about what we talking about, if context parameter is still necessary.

@cyphar
Contributor
cyphar commented Dec 17, 2014

@LK4D4 Oh, I completely forgot about the context parameter. That makes -f even weirder. Do you have to do this now:

$ docker build -f somedir/Dockerfile.magic somedir/

Or should it assume that you want the context to be somedir, unless you specify the context parameter?

@LK4D4
Contributor
LK4D4 commented Dec 17, 2014

@cyphar Yes, you have to do this.

@duglin
Contributor
duglin commented Dec 17, 2014

so I guess the question is:
should -f be relative to the current dir or to the root of the build context?
As an end user, my first reaction was to think it should be relative to the current dir. Your first example was the most compelling to me:
cd foo
docker build -f myDockerfile ..
it would be really odd to me to have to write:
docker build -f foo/myDockerfile ..

@slafs
slafs commented Dec 17, 2014

Just wanted to add that I think we have a potential backwards incompatible change here. Because having that kind of structure:

.
├── dockercontext
│   ├── Dockerfile
│   └── ...
├── Dockerfile
└── ...

and running docker build dockercontext/ (without -f option) could build different images on different versions of Docker. Right? Or am I missing something?

@duglin
Contributor
duglin commented Dec 17, 2014

@slafs no it should not change that, if it does then I broke something :-)

@thaJeztah
Member

Hope I'm not messing up the discussion with this, but something I brought up in #7284; Should each Dockerfile also be able to have its own .dockerignore file?

My use case; being able to build multiple images (dev/prod) from the same context, but exclude some large files (or large amount of files) from the image that are not needed when building a dev image. Being able to exclude those files gives a huge speed gain when building the image.

@crosbymichael
Member

@duglin what you said here sounds like what we would expect as an end user "As an end user, my first reaction was to think it should be relative to the current dir."

We can go that route.

@duglin
Contributor
duglin commented Dec 18, 2014

@thajeztah if we did want to support multiple .dockerignores then I would think it could be done in a similar way to the -f option, so: -i/--ignore=file but I think that would be a new PR.

@duglin
Contributor
duglin commented Dec 18, 2014

ok, -f is now relative to the current dir.

@SvenDowideit SvenDowideit and 2 others commented on an outdated diff Dec 18, 2014
docs/man/docker-build.1.md
@@ -30,6 +31,9 @@ When a Git repository is set as the **URL**, the repository is used
as context.
# OPTIONS
+**-f**, **--file**=*Dockerfile*
+ Location of the Dockerfile to use. The file must be within the build context. The default is *Dockerfile*.
@SvenDowideit
SvenDowideit Dec 18, 2014 Collaborator

the file must be within the build context, but is specified relaitve to where you're running the docker command -

mmm, or maybe something to the effect of 'the file must begin with the path of the context'

imo confusing, but much more so if we don't document it (and the alternative is also confusing)

@jamtur01
jamtur01 Dec 18, 2014 Contributor

Inconsistent formatting of Dockerfile.

@duglin
duglin Dec 18, 2014 Contributor

@jamtur01 can you elaborate? I tried to make it consistent with how I saw other options (like true/false in --force-rm) specified.

@duglin
duglin Dec 18, 2014 Contributor

@SvenDowideit I don't think we want to force the client to specify a full path, which is what "the file must begin with the path of the context" would force. I think being able to do "-f file" or "-f ../../foo/bar/file" should be allowed. The point we need to get clear, and we may not be there yet, is that the file needs to be in the build context directory structure regardless of how the user chooses to reference it.

@jamtur01
jamtur01 Dec 18, 2014 Contributor

Ah don't worry about it - it appears it's not your issue - the problem appears elsewhere.

@SvenDowideit
SvenDowideit Jan 5, 2015 Collaborator

@duglin I don't think the docs here are clear on which it is - is -f relative to the pwd, or relative to the location of the context dir.

and as i note in the cli.md docs, if its relative to the pwd (which is how i read your docs atm), then the -f value will be different to the value sent to the daemon.

@duglin
duglin Jan 5, 2015 Contributor

ok I updated the docs here to make it clear that its relative to the current dir. See if it helps.

@SvenDowideit SvenDowideit commented on an outdated diff Dec 18, 2014
docs/sources/reference/commandline/cli.md
@@ -497,6 +498,12 @@ For example, the files `tempa`, `tempb` are ignored from the root directory.
Currently there is no support for regular expressions. Formats
like `[^temp*]` are ignored.
+By default the `docker build` command will look for a `Dockerfile` at the
+root of the build context. By using the `-f`, `--file`, option you can specify
+the path to an alternative file that should be used instead. This is useful
+in cases where the same set of files are used for multiple builds. The path
+must be to a file within the build context.
@SvenDowideit
SvenDowideit Dec 18, 2014 Collaborator

as above in the man page

@SvenDowideit SvenDowideit and 1 other commented on an outdated diff Dec 18, 2014
docs/sources/reference/commandline/cli.md
@@ -599,6 +606,18 @@ repository is used as Dockerfile. Note that you
can specify an arbitrary Git repository by using the `git://` or `git@`
schema.
+ $ sudo docker build -f Dockerfile.debug .
+
+This will use a file called `Dockerfile.debug` instead of `Dockerfile`
+during the build process. If a `Dockerfile` does exist it will be ignored.
+
+ $ sudo docker build -f dockerfiles/Dockerfile.debug -t myapp_debug .
+ $ sudo docker build -f dockerfiles/Dockerfile.prod -t myapp_prod .
+
@SvenDowideit
SvenDowideit Dec 18, 2014 Collaborator

will sudo docker build -f ./dockerfiles/Dockerfile.prod -t myapp_prod . work too?

and I think you'll need an example with a non-. context to show what that implies..

@duglin
duglin Dec 18, 2014 Contributor

@SvenDowideit yes that should work too. I added another example with some more text see if that helps. I tried to use a non-. case and elaborated a bit on how the dockerfile needs to be in the context regardless of how we point to it.

@SvenDowideit
Collaborator
@jamtur01 jamtur01 and 1 other commented on an outdated diff Dec 18, 2014
docs/sources/reference/commandline/cli.md
@@ -599,6 +606,18 @@ repository is used as Dockerfile. Note that you
can specify an arbitrary Git repository by using the `git://` or `git@`
schema.
+ $ sudo docker build -f Dockerfile.debug .
+
+This will use a file called `Dockerfile.debug` instead of `Dockerfile`
+during the build process. If a `Dockerfile` does exist it will be ignored.
@jamtur01
jamtur01 Dec 18, 2014 Contributor

also exists?

@duglin
duglin Dec 18, 2014 Contributor

done

@fredlf fredlf commented on an outdated diff Dec 19, 2014
docs/sources/reference/commandline/cli.md
@@ -497,6 +498,12 @@ For example, the files `tempa`, `tempb` are ignored from the root directory.
Currently there is no support for regular expressions. Formats
like `[^temp*]` are ignored.
+By default the `docker build` command will look for a `Dockerfile` at the
+root of the build context. By using the `-f`, `--file`, option you can specify
+the path to an alternative file that should be used instead. This is useful
@fredlf
fredlf Dec 19, 2014 Contributor

Let's simplify this to: "The -f, --file, option lets you specify the path to an alternative file to use instead."

@fredlf fredlf commented on an outdated diff Dec 19, 2014
docs/sources/reference/commandline/cli.md
+during the build process. If a `Dockerfile` also exists it will be ignored.
+
+ $ sudo docker build -f dockerfiles/Dockerfile.debug -t myapp_debug .
+ $ sudo docker build -f dockerfiles/Dockerfile.prod -t myapp_prod .
+
+The above commands will build the current build context twice, once
+using a debug version of a `Dockerfile` and one using a production
+version.
+
+ $ cd /home/me/myapp/some/dir/really/deep
+ $ sudo docker build -f /home/me/myapp/dockerfiles/debug /home/me/myapp
+ $ sudo docker build -f ../../../../dockerfiles/debug /home/me/myapp
+
+These two `docker build` commands do the exact same thing. They will both
+use the contents of the `debug` file instead of looking for a `Dockerfile`
+and use `/home/me/myapp` as the root of the build context. The key
@fredlf
fredlf Dec 19, 2014 Contributor

and will use

@fredlf fredlf commented on an outdated diff Dec 19, 2014
docs/sources/reference/commandline/cli.md
@@ -599,6 +606,28 @@ repository is used as Dockerfile. Note that you
can specify an arbitrary Git repository by using the `git://` or `git@`
schema.
+ $ sudo docker build -f Dockerfile.debug .
+
+This will use a file called `Dockerfile.debug` instead of `Dockerfile`
+during the build process. If a `Dockerfile` also exists it will be ignored.
+
+ $ sudo docker build -f dockerfiles/Dockerfile.debug -t myapp_debug .
+ $ sudo docker build -f dockerfiles/Dockerfile.prod -t myapp_prod .
+
+The above commands will build the current build context twice, once
+using a debug version of a `Dockerfile` and one using a production
@fredlf
fredlf Dec 19, 2014 Contributor

one > once

@fredlf fredlf commented on an outdated diff Dec 19, 2014
docs/sources/reference/commandline/cli.md
+ $ sudo docker build -f dockerfiles/Dockerfile.prod -t myapp_prod .
+
+The above commands will build the current build context twice, once
+using a debug version of a `Dockerfile` and one using a production
+version.
+
+ $ cd /home/me/myapp/some/dir/really/deep
+ $ sudo docker build -f /home/me/myapp/dockerfiles/debug /home/me/myapp
+ $ sudo docker build -f ../../../../dockerfiles/debug /home/me/myapp
+
+These two `docker build` commands do the exact same thing. They will both
+use the contents of the `debug` file instead of looking for a `Dockerfile`
+and use `/home/me/myapp` as the root of the build context. The key
+aspect is that `debug` is within the build context directory structure
+irrespective of how we reference it on the command line.
+
@fredlf
fredlf Dec 19, 2014 Contributor

I found that last sentence hard to follow. Suggest something like: "Note that debug is in the directory structure of the build context, regardless of how you refer to it on the command line.

@fredlf
Contributor
fredlf commented Dec 19, 2014

A few small edits, otherwise docs LGTM.

@duglin
Contributor
duglin commented Dec 19, 2014

@fredlf fixed your "small edits".
Also, rebased to resolve a merge conflict.

@fredlf
Contributor
fredlf commented Dec 19, 2014

Many thanks, Doug. LGTM.

@crosbymichael
Member

LGTM

@SvenDowideit
Collaborator

Docs LGTM

@icecrime
Member
icecrime commented Jan 2, 2015

LGTM

Ping @jfrazelle @LK4D4: if one of you could please give it a last look and merge if it's ok!

@tianon tianon and 1 other commented on an outdated diff Jan 3, 2015
api/client/commands.go
+
+ absRoot, err := filepath.Abs(root)
+ if err != nil {
+ return err
+ }
+
+ var filename string
+ origDockerfile := *dockerfileName
+
+ if cmd.IsSet("f") || cmd.IsSet("file") {
+ if filename, err = filepath.Abs(*dockerfileName); err != nil {
+ return err
+ }
+ } else {
+ filename = path.Join(absRoot, *dockerfileName)
+ }
@tianon
tianon Jan 3, 2015 Member

I'm a little confused as to why we've got this extra logic in here detecting whether -f or --file got passed in.

Couldn't we just change the default to be the empty string, and swap based on that to CONTEXT/Dockerfile? I think it's going to be confusing for users if they see that the default for -f is "Dockerfile", but if they explicitly do -f Dockerfile it's now actually semantically different, and means the Dockerfile in the current directory, regardless of the context.

@duglin
duglin Jan 3, 2015 Contributor

Before I make a change I want to get agreement on what I think you're asking for:

  • change the default Dockerfile to be "CONTEXT/Dockerfile" (keeps backwards compat)
  • this will not only be a code change but a help-text change, so the end user will see:
    -f, --file="CONTEXT-ROOT/Dockerfile" Name of the Dockerfile
  • then if the user wants to be explicit and specify -f but still use the default value then they would need to include the path to the Dockerfile (including build context) on the -f arg.

Right?
Seems reasonable to me.

@duglin
duglin Jan 3, 2015 Contributor

Actually, thinking more about how the flag stuff works, I think the user might need to see:
-f, --file="" Name of Dockerfile (default is CONTEXT-ROOT/Dockerfile)

@tianon
tianon Jan 3, 2015 Member

I meant change the default to the empty string, and use that to determine
when to change it to <context-root>/Dockerfile. Text-parsing for
CONTEXT/ or CONTEXT-ROOT/ is going to be flaky IMO. Then the text can
describe that the default is a file named Dockerfile at the root of the
context, but that an argument can be supplied which is either absolute or
relative to the CWD.

@duglin
duglin Jan 3, 2015 Contributor

Right, we're on the same page. Will do...

@tianon tianon and 2 others commented on an outdated diff Jan 3, 2015
api/client/commands.go
@@ -222,6 +251,9 @@ func (cli *DockerCli) CmdBuild(args ...string) error {
if *pull {
v.Set("pull", "1")
}
+
+ v.Set("f", *dockerfileName)
@tianon
tianon Jan 3, 2015 Member

Since we have the opportunity now before this merges and gets set in stone, shouldn't we give this API parameter a more verbose/meaningful name like dockerfile? 👼

@tianon
tianon Jan 3, 2015 Member

The dockerfileName we use as the variable name everywhere sounds pretty good, too. 😉

@duglin
duglin Jan 3, 2015 Contributor

@tianon which "API parameter" are you referring to ? The line of code this comment is on already uses "dockerfileName".

@tianon
tianon Jan 3, 2015 Member

I mean that "f" which is the API parameter. A third-party API client
wanting to use this feature would have to specify a query parameter of
"f=..." to use this feature, which isn't very intuitive for an API IMO (but
maybe I'm alone in this? we have an absolute plethora of similarly
badly-named API parameters already).

@duglin
duglin Jan 3, 2015 Contributor

Ah, well, the original PR I submitted used "-d" instead but at some point people seemed to gravitate towards -f (perhaps because 'make' uses -f to point to the makefile?) so I switched it. I do think that if we use -d on the API then we should probably use it on the cmd line too.
I'm ok with either, but would need the appropriate UX approval first I think.

@tianon
tianon Jan 3, 2015 Member

Nonono this string is for the API parameter, not the CLI. I 100% agree
with "-f" for the CLI. This line represents the API.

@duglin
duglin Jan 3, 2015 Contributor

ah ok but still the question is whether the API should match the CLI. Doesn't it usually?

@tianon
tianon Jan 3, 2015 Member

In some places, yes, in other places not so much.

@duglin
duglin Jan 3, 2015 Contributor

who gives the final decision on UX questions?

@LK4D4
LK4D4 Jan 5, 2015 Contributor

I'm +1 for changing name of this api parameter. All this t, q are pretty bad :(

@duglin
duglin Jan 6, 2015 Contributor

ok changed it to "dockerfile"

@tianon tianon and 1 other commented on an outdated diff Jan 3, 2015
docs/sources/reference/commandline/cli.md
@@ -448,11 +448,12 @@ To kill the container, use `docker kill`.
Build a new image from the source code at PATH
- --force-rm=false Always remove intermediate containers, even after unsuccessful builds
- --no-cache=false Do not use cache when building the image
- -q, --quiet=false Suppress the verbose output generated by the containers
- --rm=true Remove intermediate containers after a successful build
- -t, --tag="" Repository name (and optionally a tag) to be applied to the resulting image in case of success
+ -f, --file="Dockerfile" Location of the Dockerfile to use
@tianon
tianon Jan 3, 2015 Member

Is this indented more than the following flags on purpose? 😉

@duglin
duglin Jan 3, 2015 Contributor

there was a tab in there - converted it to spaces, so things should align now.

@duglin
Contributor
duglin commented Jan 3, 2015

@tianon I made the change to the default value of -f per your suggestion.

@duglin
Contributor
duglin commented Jan 5, 2015

@SvenDowideit I updated the API v1.17 docs - I missed adding "f" to the /build parameters. See if you're ok with the edits.

@SvenDowideit SvenDowideit and 1 other commented on an outdated diff Jan 5, 2015
docs/sources/reference/commandline/cli.md
@@ -599,6 +606,28 @@ repository is used as Dockerfile. Note that you
can specify an arbitrary Git repository by using the `git://` or `git@`
schema.
+ $ sudo docker build -f Dockerfile.debug .
+
+This will use a file called `Dockerfile.debug` instead of `Dockerfile`
+during the build process. If a `Dockerfile` also exists it will be ignored.
@SvenDowideit
SvenDowideit Jan 5, 2015 Collaborator

perhaps it would be better not to use the word ignored - as .dockerignore means the file isn't added to the context?

maybe

This will use a file called `Dockerfile.debug` for the build instructions, instead of the `Dockerfile`.
@duglin
duglin Jan 5, 2015 Contributor

done

@SvenDowideit SvenDowideit and 1 other commented on an outdated diff Jan 5, 2015
docs/sources/reference/commandline/cli.md
@@ -599,6 +606,28 @@ repository is used as Dockerfile. Note that you
can specify an arbitrary Git repository by using the `git://` or `git@`
schema.
+ $ sudo docker build -f Dockerfile.debug .
+
+This will use a file called `Dockerfile.debug` instead of `Dockerfile`
+during the build process. If a `Dockerfile` also exists it will be ignored.
+
+ $ sudo docker build -f dockerfiles/Dockerfile.debug -t myapp_debug .
+ $ sudo docker build -f dockerfiles/Dockerfile.prod -t myapp_prod .
+
+The above commands will build the current build context twice, once
+using a debug version of a `Dockerfile` and once using a production
+version.
@SvenDowideit
SvenDowideit Jan 5, 2015 Collaborator

its worth re-enforcing that the context is build from the location of the final .

@duglin
duglin Jan 5, 2015 Contributor

Done

@SvenDowideit SvenDowideit and 1 other commented on an outdated diff Jan 5, 2015
docs/sources/reference/commandline/cli.md
@@ -599,6 +606,28 @@ repository is used as Dockerfile. Note that you
can specify an arbitrary Git repository by using the `git://` or `git@`
schema.
+ $ sudo docker build -f Dockerfile.debug .
+
+This will use a file called `Dockerfile.debug` instead of `Dockerfile`
+during the build process. If a `Dockerfile` also exists it will be ignored.
+
+ $ sudo docker build -f dockerfiles/Dockerfile.debug -t myapp_debug .
+ $ sudo docker build -f dockerfiles/Dockerfile.prod -t myapp_prod .
+
+The above commands will build the current build context twice, once
+using a debug version of a `Dockerfile` and once using a production
+version.
+
+ $ cd /home/me/myapp/some/dir/really/deep
+ $ sudo docker build -f /home/me/myapp/dockerfiles/debug /home/me/myapp
+ $ sudo docker build -f ../../../../dockerfiles/debug /home/me/myapp
@SvenDowideit
SvenDowideit Jan 5, 2015 Collaborator

indenting?

@SvenDowideit
SvenDowideit Jan 5, 2015 Collaborator

ooo, so if i grok right, the value of -f will be different to the f= value passed to the daemon - as the first is relative to the dir we're in (i get weirded out by this) and the second will be relative to the context?

I wonder if thats worth mentioning (in the API docs)

@duglin
duglin Jan 5, 2015 Contributor

Fixed the indenting - had a tab in there by mistake.

re: -f vs f= I added a bit more text under the -f/--file section to make it clear that if the value is a relative path then it must be relative to the current dir.

@SvenDowideit SvenDowideit and 1 other commented on an outdated diff Jan 5, 2015
docs/sources/reference/commandline/cli.md
+
+ $ sudo docker build -f dockerfiles/Dockerfile.debug -t myapp_debug .
+ $ sudo docker build -f dockerfiles/Dockerfile.prod -t myapp_prod .
+
+The above commands will build the current build context twice, once
+using a debug version of a `Dockerfile` and once using a production
+version.
+
+ $ cd /home/me/myapp/some/dir/really/deep
+ $ sudo docker build -f /home/me/myapp/dockerfiles/debug /home/me/myapp
+ $ sudo docker build -f ../../../../dockerfiles/debug /home/me/myapp
+
+These two `docker build` commands do the exact same thing. They both
+use the contents of the `debug` file instead of looking for a `Dockerfile`
+and will use `/home/me/myapp` as the root of the build context. Note that
+debug is in the directory structure of the build context, regardless of how
@SvenDowideit
SvenDowideit Jan 5, 2015 Collaborator

Note that the debug file must be within the directory...

@duglin
duglin Jan 5, 2015 Contributor

fixed

@LK4D4 LK4D4 and 2 others commented on an outdated diff Jan 5, 2015
api/client/commands.go
+ var filename string // path to Dockerfile
+ var origDockerfile string // used for error msg
+
+ if *dockerfileName == "" {
+ // No -f/--file was specified so use the default
+ origDockerfile = "Dockerfile"
+ *dockerfileName = origDockerfile
+ filename = path.Join(absRoot, *dockerfileName)
+ } else {
+ origDockerfile = *dockerfileName
+ if filename, err = filepath.Abs(*dockerfileName); err != nil {
+ return err
+ }
+ }
+
+ if !strings.HasSuffix(absRoot, "/") {
@LK4D4
LK4D4 Jan 5, 2015 Contributor

Why this checks done for default Dockerfile too? Shouldn't this be in non-default name condition?

@LK4D4
LK4D4 Jan 5, 2015 Contributor

Also, I am not sure, but probably better not do magic with trailing / and just use filepath.Rel

@thaJeztah
thaJeztah Jan 5, 2015 Member

^^ also if this is on the client, it will probably break on Windows?

@duglin
duglin Jan 6, 2015 Contributor

ok I moved that logic to just be for user-defined paths and used filepath.Rel (didn't know that existed, cool!)

@duglin
duglin Jan 6, 2015 Contributor

Apparently filepath.Rel() will try really really hard to find a path that works - even if it has to generate one with ../../../.. to do it - notice the description even says:
On success, the returned path will always be relative to basepath, even if basepath and targpath share no elements

which doesn't make it nearly as useful. Reverting parts of that change but open to other ideas.

@LK4D4 LK4D4 and 1 other commented on an outdated diff Jan 5, 2015
builder/evaluator.go
@@ -136,7 +138,13 @@ func (b *Builder) Run(context io.Reader) (string, error) {
}
}()
- filename := path.Join(b.contextPath, "Dockerfile")
+ filename := path.Join(b.contextPath, b.dockerfileName)
+
+ absContextPath, _ := filepath.Abs(b.contextPath)
@LK4D4
LK4D4 Jan 5, 2015 Contributor

Just curious, why you not checking errors here? :) You checked them on client side.

@duglin
duglin Jan 6, 2015 Contributor

Not sure what I was thinking at the time other than it seemed pretty unlikely that they would fail but I'll add 'em.

@SvenDowideit
Collaborator

Docs LGTM - merci-much

@LK4D4
Contributor
LK4D4 commented Jan 6, 2015

@duglin need rebase :) sorry

@duglin
Contributor
duglin commented Jan 6, 2015

@LK4D4 ok rebased

@erikh erikh commented on an outdated diff Jan 6, 2015
api/client/commands.go
@@ -136,10 +141,42 @@ func (cli *DockerCli) CmdBuild(args ...string) error {
if _, err := os.Stat(root); err != nil {
return err
}
- filename := path.Join(root, "Dockerfile")
+
+ absRoot, err := filepath.Abs(root)
+ if err != nil {
+ return err
+ }
+
+ var filename string // path to Dockerfile
+ var origDockerfile string // used for error msg
+
+ if *dockerfileName == "" {
+ // No -f/--file was specified so use the default
+ origDockerfile = "Dockerfile"
@erikh
erikh Jan 6, 2015 Contributor

can we make this a constant?

@erikh
Contributor
erikh commented Jan 6, 2015

need the constant, otherwise LGTM

@erikh erikh self-assigned this Jan 6, 2015
@duglin
Contributor
duglin commented Jan 6, 2015

@erikh ok created a constant and used it on client and server

@duglin
Contributor
duglin commented Jan 6, 2015

ok rebased and constant is made/used.

@crosbymichael
Member

@duglin Either we are working really fast today or you forgot to push your rebase ;)

@duglin duglin Allow for Dockerfile to be named something else.
Add a check to make sure Dockerfile is in the build context
Add docs and a testcase
Make -f relative to current dir, not build context

Signed-off-by: Doug Davis <dug@us.ibm.com>
eb3ea3b
@duglin
Contributor
duglin commented Jan 7, 2015

no, I pushed it, but then #8748 caused a conflict.
rebased and added a testcase to make sure that the new .dockerignore processing (from #8748) works even when the Dockerfile is called something else.

@crosbymichael
Member

Nice

@crosbymichael
Member

Cool, i think this has enough LGTMs

@crosbymichael crosbymichael merged commit a67e738 into docker:master Jan 7, 2015

1 check passed

default The build succeeded on drone.io
Details
@duglin duglin deleted the duglin:RenameDockerfile branch Jan 22, 2015
@erikh erikh was unassigned by duglin Feb 5, 2015
@dreamcat4

We also need this feature on the DockerHub (hub.docker.com) for it's Automated Builds. In the 'Edit Build Settings' page where it has the Field named 'Dockerfile Location'.

https://support.docker.com/hc/en-us/requests/3494

@gurjeet gurjeet added a commit to gurjeet/docker that referenced this pull request Oct 13, 2015
@gurjeet gurjeet Add ability to specify custom filename to be used as Dockerfile.
NOTE on Oct 13, 2015:
This patch has now been obsoleted, since the merge of #9707
docker#9707
END NOTE

Now one can perform the following commands in the same directory:

    docker build -t db_container -f db.dockerfile .
    docker build -t web_container -f web.dockerfile .
    docker build -t client_container -f client.dockerfile .
    docker build -t client_container .

The last one will use the default Dockerfile file.
9f7d1a0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment