Skip to content
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

direnv exec: impossible command line #492

Closed
Profpatsch opened this issue Apr 12, 2019 · 6 comments
Closed

direnv exec: impossible command line #492

Profpatsch opened this issue Apr 12, 2019 · 6 comments

Comments

@Profpatsch
Copy link
Contributor

exec [DIR] COMMAND [...ARGS]:

Means that both DIR and an unspecified amount of ARGS are optional.
It should be impossible for direnv to figure out whether the second argument is a DIR or COMMAND.

And indeed:

> direnv exec echo a b c
direnv: error stat echo: no such file or directory
> direnv exec $PWD echo a b c
direnv: loading .envrc
a b c
  • target shell: fish
  • direnv version: 2.19.2
  • OS release: NixOS 18.09
@zimbatm
Copy link
Member

zimbatm commented Apr 13, 2019

yes it's not great, the result of organic growth. But I don't want to break backward-compatibility now.

The best solution is to always pass the DIR and use . as the default.

@Profpatsch
Copy link
Contributor Author

Since the behavior always requires DIR in practice, we can just mark it as non-optional in the docs.

@Profpatsch
Copy link
Contributor Author

Profpatsch commented Apr 15, 2019

Oh wait … wait

direnv/cmd_exec.go

Lines 33 to 43 in e05d32a

if fi.IsDir() {
if len(args) < 3 {
return fmt.Errorf("missing COMMAND argument")
}
command = args[2]
args = args[2:]
} else {
command = rcPath
rcPath = filepath.Dir(rcPath)
args = args[1:]
}

I didn’t expect this to be a thing.

@zimbatm
Copy link
Member

zimbatm commented Apr 16, 2019

yeah, it's bad...

@Profpatsch
Copy link
Contributor Author

The “just a command” case is broken at the moment however:

> direnv exec bash
direnv: error stat bash: no such file or directory

I think that needs to be fixed nonetheless.

@zimbatm
Copy link
Member

zimbatm commented Apr 17, 2019

Ok, let's make the DIR mandatory then. It must have been broken for a long time and nobody complained.

Profpatsch added a commit to Profpatsch/direnv that referenced this issue Apr 17, 2019
```
$ direnv exec bash -c "true"
direnv: error stat bash: no such file or directory
$ direnv exec bash
direnv: error stat bash: no such file or directory
```

Meaning leaving out the DIR argument was never really an option.

Closes: direnv#492
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants