Skip to content

Conversation

@dnephin
Copy link

@dnephin dnephin commented Aug 3, 2015

Resolves #169
Resolves #1662

The image is ~50Mb.

Thoughts?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does everything work fine if $(pwd) is a subdir of $HOME and we get overlapping volume mounts?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya, that doesn't seem to be a problem from what I can tell

@bfirsh
Copy link

bfirsh commented Aug 5, 2015

Interesting.

Would this make Windows support easier? Paths, TTYs, etc... #1085

@dnephin
Copy link
Author

dnephin commented Aug 5, 2015

It would run easily enough in cygwin I think, but I don't think windows comes with bash, right?

We could re-create this bash script in golang, and publish it as a statically linked binary, which might make it more portable.

@bfirsh
Copy link

bfirsh commented Aug 5, 2015

Sorry - yes. Assuming we have a similar script or binary running on Windows that runs Compose in a container, would Compose running inside a container work on Windows?

@aanand
Copy link

aanand commented Aug 5, 2015

Yes, it would. The other assumption is that volume mounting would 'just work', as it does on OS X (as long as you're within your home directory).

docs/install.md Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we added compose-run to the image itself, using docker as the distribution mechanism, and a small install script to echo/output it, e.g.;

# To install the script wrapper, run
$ docker run --rm docker-compose install > /usr/local/bin/docker-compose
$ chmod +x /usr/local/bin/docker-compose

# Validate it's working
$ docker-compose --version
docker-compose version: 1.4.0rc3

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a cool idea, I think that could work.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a cool idea. We should probably put an explicit version tag in, so we can change the run script if we need to.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could it be a separate image? If you install Compose with the Toolbox, the "install" command might be confusing.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya, since this command is really just printing out the bash script, we could probably call it echo-run-script or something like that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The advantage of it being in the same image, is that the image is pulled during "install". The first "run" is instant then :-)

@mnowster
Copy link

mnowster commented Oct 2, 2015

@dnephin if you can do a rebase, I'll review.

@dnephin
Copy link
Author

dnephin commented Oct 2, 2015

Rebased

docs/install.md Outdated
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor edit: Can we make this a link to the alternative install section? Then we can remove the, 'below'.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya, it seems like there is no standard way to do that with markdown? I guess I have to do something like this https://gohugo.io/extras/crossreferences/ , I'll try it out

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested it out locally, seems to work

@dnephin dnephin force-pushed the run_script branch 2 times, most recently from 52af569 to 8e85709 Compare October 2, 2015 15:31
docs/install.md Outdated
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest Install as a container as a more descriptive header.

@dnephin dnephin force-pushed the run_script branch 2 times, most recently from d9bc968 to 1b5a13c Compare October 2, 2015 15:34
docs/install.md Outdated
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is missing a $ at the beginning so it'll get rendered as a command.

@dnephin
Copy link
Author

dnephin commented Oct 2, 2015

Cool, I've made those changes

…it with the

correct volumes.

Signed-off-by: Daniel Nephin <dnephin@gmail.com>
@mnowster
Copy link

mnowster commented Oct 2, 2015

LGTM

@mnowster
Copy link

mnowster commented Oct 2, 2015

Come on janky 🐎 🏇

mnowster added a commit that referenced this pull request Oct 2, 2015
Docker image and bash script for running compose in a container
@mnowster mnowster merged commit 1927189 into docker:master Oct 2, 2015
@dnephin dnephin deleted the run_script branch October 2, 2015 16:25
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll open an issue to get this image published to a better repo, and add notes to the release process doc about updating it on release.

@dnephin dnephin added this to the 1.5.0 milestone Oct 2, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use "$PWD" (which will be set by the shell) and avoid the subshells here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would probably work as well. There are times when $PWD isn't set (if you fork/exec without a shell for example).

Does this cause a problem?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No serious problem, just calling out the unnecessary subshell.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants