Skip to content

Conversation

@simonvanderveldt
Copy link

This adds the possibility to override the workdir from the command line, using either -w or --workdir just like docker run.
In essence it does the same as #332, though that code couldn't really be rebased so I wrote my own. Do we want to attribute anything of this to @scottynomad?

Also not really sure about the acceptance tests, there seem to be multiple different ways of doing them within tests/acceptance/cli_test.py. I basically mirrored the -u / --user tests.

Relevant for #363
Fixes #2811

@simonvanderveldt
Copy link
Author

@GordonTheTurtle doesn't seem to be happy because the DinD tag used in the test is missing

++ docker run -d --name compose-dind-1.10.2-rc1-2391 --privileged --volume=/var/lib/docker dockerswarm/dind:1.10.2-rc1 docker daemon -H tcp://0.0.0.0:2375 --storage-driver=aufs
Unable to find image 'dockerswarm/dind:1.10.2-rc1' locally
Pulling repository docker.io/dockerswarm/dind
Tag 1.10.2-rc1 not found in repository docker.io/dockerswarm/dind

@dnephin
Copy link

dnephin commented Feb 22, 2016

Edit: Oops, nevermind, this is different

I'm fixing the dind image build now

@simonvanderveldt simonvanderveldt force-pushed the add-run-workdir branch 3 times, most recently from b26bc80 to 892b7c5 Compare February 26, 2016 12:07
@simonvanderveldt
Copy link
Author

I noticed there were markdown files documenting the commands as well, so I've updated docs/reference/run.md to reflect the change in this PR.

The test on Janky now fails on the flake8 test, which works fine locally. Not sure why this is happening.
Note the test on Janky passed before I added the markdown file, so I guess something is broken/has changed in the test?

@dnephin
Copy link

dnephin commented Feb 26, 2016

I've got a fix for the max-complexity error in this commit: dnephin@8246c2a

I guess I rebased on master, so it doesn't merge cleanly on your branch, but you can cherry-pick this commit if you add my fork as a remote.

@simonvanderveldt
Copy link
Author

I've got a fix for the max-complexity error in this commit: dnephin/compose@8246c2a

Thanks!
Do you think the build_container_options() could be mistaken for docker build options?

I guess I rebased on master, so it doesn't merge cleanly on your branch, but you can cherry-pick this commit if you add my fork as a remote.

Done, works fine locally, hope Janky is happy this time :)

to the host.
-T Disable pseudo-tty allocation. By default `docker-compose run`
allocates a TTY.
-w, --workdir="" Override the workdir
Copy link

Choose a reason for hiding this comment

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

I just noticed, this should match what docker run --help says "Working directory inside the container"

Copy link
Author

Choose a reason for hiding this comment

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

I just noticed, this should match what docker run --help says "Working directory inside the container"

Done, thx for the hint!

@dnephin dnephin added this to the 1.7.0 milestone Feb 26, 2016
@dnephin
Copy link

dnephin commented Feb 26, 2016

With the fix to the help text LGTM

simonvanderveldt and others added 2 commits February 27, 2016 00:05
Signed-off-by: Simon van der Veldt <simon.vanderveldt@gmail.com>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
@dnephin
Copy link

dnephin commented Feb 26, 2016

LGTM

@simonvanderveldt
Copy link
Author

Janky errors seem to be unrelated?

____________________________ CLITestCase.test_kill _____________________________

tests/acceptance/cli_test.py:1163: in test_kill

    self.dispatch(['up', '-d'], None)

tests/acceptance/cli_test.py:128: in dispatch

    return wait_on_process(proc, returncode=returncode)

tests/acceptance/cli_test.py:48: in wait_on_process

    assert proc.returncode == returncode

E   AssertionError: assert 1 == 0

E    +  where 1 = <subprocess.Popen object at 0x7f30c77aacd0>.returncode

----------------------------- Captured stdout call -----------------------------

Running process: 205

Stderr: Creating simplecomposefile_simple_1

Starting simplecomposefile_another_1

Container is marked for removal and cannot be started.



Stdout: 

----------------------------- Captured stderr call -----------------------------

Killing simplecomposefile_simple_1 ... 



Killing simplecomposefile_simple_1 ... done
Removing simplecomposefile_simple_1 ... 


Removing simplecomposefile_another_1 ... 



Removing simplecomposefile_simple_1 ... done

Removing simplecomposefile_another_1 ... error


ERROR: for simplecomposefile_another_1  Driver aufs failed to remove root filesystem f75f290d301456234f1d37c18bd63254a3f6bd48c3efcc2b099768760c071b3c: rename /var/lib/docker/aufs/mnt/0cb124ee817b6072798a141ba784ce4c79f427269198d412424bd8117fdd58e8 /var/lib/docker/aufs/mnt/0cb124ee817b6072798a141ba784ce4c79f427269198d412424bd8117fdd58e8-removing: device or resource busy 

@dnephin
Copy link

dnephin commented Feb 26, 2016

yup, unrelated

@dnephin
Copy link

dnephin commented Mar 9, 2016

Needs a rebase to be merged

@simonvanderveldt
Copy link
Author

Needs a rebase to be merged

@dnephin When will you need it to be rebased? I'm on holidays till the 25th of march :)

@dnephin
Copy link

dnephin commented Mar 14, 2016

Ok, don't worry about it. I will pick this up and carry the commits for the release. Code freeze is this week.

@dnephin dnephin mentioned this pull request Mar 15, 2016
@dnephin
Copy link

dnephin commented Mar 15, 2016

Carried in #3136

@dnephin dnephin closed this Mar 15, 2016
@simonvanderveldt
Copy link
Author

Awesome, thanks!

@simonvanderveldt simonvanderveldt deleted the add-run-workdir branch February 2, 2017 08:13
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.

3 participants