Skip to content
This repository has been archived by the owner on Jul 29, 2018. It is now read-only.

--path feature for install cli not working as expected #376

Closed
naina-verma opened this issue Aug 12, 2016 · 11 comments
Closed

--path feature for install cli not working as expected #376

naina-verma opened this issue Aug 12, 2016 · 11 comments
Labels
Milestone

Comments

@naina-verma
Copy link

--path feature for install cli not working as expected
Find issue details : https://gist.github.com/naina-verma/e03bdfb7bc60adaa22b2dbb75e48f604
tested on latest vsm
vagrant-registration (1.2.3) Version Constraint: 1.2.3 vagrant-service-manager (1.3.0.dev) Version Constraint: 1.3.0.dev vagrant-sshfs (1.2.0)

@brgnepal brgnepal added this to the v1.4.0 milestone Aug 12, 2016
@LalatenduMohanty
Copy link
Collaborator

LalatenduMohanty commented Sep 16, 2016

Copying the actuall output from gist @naina-verma mentioned . Check for The system cannot find the path specified. in below output

cdk@WIN10 ~/naina/unify/pk
$ vagrant.exe service-manager install-cli  docker --cli-version 1.10.3 --path /home/cdk/naina/docker
The system cannot find the path specified.
# Binary now available at /home/cdk/naina/docker
# run binary as:
# docker.exe <command>
export PATH=/home/cdk/naina:$PATH

# run following command to configure your shell:
# eval "$(VAGRANT_NO_COLOR=1 vagrant service-manager install-cli docker | tr -d '\r')"

@hferentschik
Copy link
Contributor

@budhrg what's the state on this. Is the --path option available or not. It is not documented at least.

@brgnepal
Copy link
Contributor

@hferentschik Ya not properly documented. Will work with @Preeticp on it.

@hferentschik
Copy link
Contributor

Well, afaict there is also no help in the CLI itself. So it is not just a docs issue for sure. Also, the issue that the help system is only explorable if the VM is running still needs addressing.

@brgnepal
Copy link
Contributor

Related to #326

@brgnepal
Copy link
Contributor

Fixed in #391

@brgnepal
Copy link
Contributor

brgnepal commented Oct 19, 2016

@hferentschik I was wrong here about the --path option value

The value can be any path which doesn't need permission.
like $HOME/unknown/dir/docker

Do you think the install-cli command should create those intermediate directories?

In case the path need permission, the command will fail with system permission error.

@hferentschik
Copy link
Contributor

The value can be any path which doesn't need permission like $HOME/unknown/dir/docker

Sure. That was my expectation anyways. Provided the path exists and is writable there is no reason why we could not copy the binary there.

Do you think the install-cli command should create those intermediate directories?

Not sure. I guess both works. Thinking about other tools, I think, however, that is is more common practice to expect the directory to exist. It also avoids the case that we create a recursive directory structure due to some simple typo in the path. WDYT on this one?

In case the path need permission, the command will fail with system permission error.

So assuming we require the path to exist, our code would verify it exists and is writable. If not an error would be logged and the command aborts. If we allow the path to not exist, we would create it recursively.

On a side note, do we properly set the executable bit on the binary? It would imo make sense to verify that the file is executable after copying it into the specified location. This is regardless of --path is used or not.

@brgnepal
Copy link
Contributor

On a side note, do we properly set the executable bit on the binary

Yes, we are making it executable. I think this test definition is checking it so no need to worry 😄

@hferentschik
Copy link
Contributor

Yes, we are making it executable. I think this test definition is checking it so no need to worry 😄

👍

@hferentschik
Copy link
Contributor

Resolved via pull request #416

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants