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

Silent failure on systems with Debian bash completion #239

Open
predakanga opened this issue Mar 14, 2017 · 4 comments
Open

Silent failure on systems with Debian bash completion #239

predakanga opened this issue Mar 14, 2017 · 4 comments

Comments

@predakanga
Copy link

predakanga commented Mar 14, 2017

Background: I'm currently working to improve our developer experience by rolling herokuish into a Vagrant image, discovered this issue in the final steps.

Symptoms: When running herokuish procfile start web on a system with Debian's bash completion installed, herokuish exits silently with exit code 1

Trace: trace.txt

Diagnosis:
The problem occurs at procfile-load-profile, during the loading of bash completion. The problem occurs because bash completion and set -e are incompatible, as documented in this debian bug.
Ordinarily the bash completion doesn't run for interactive shells, but it detects interactivity by checking PS1 (i.e. if [ -n "$BASH_VERSION" -a -n "$PS1" -a -z "$BASH_COMPLETION_COMPAT_DIR" ]; then, which is expected behaviour as per bash's documentation), while herokuish sets it's own PS1 unconditionally.

Unfortunately I don't know what the best way to solve this is, otherwise I would provide a PR - does herokuish actually need to set that PS1? Should it only be set immediately prior to procfile-exec?

In the meantime I've modified the interactivity check in my VM, but that's not great long term.

@michaelshobbs
Copy link
Member

It is my understanding that herokuish was never intended to be executed via anything other than the Ubuntu-based docker image distributed via Docker Hub here.

@progrium Feel free to chime in. Unless @progrium disagrees, this behavior will probably not change.

@progrium
Copy link
Contributor

progrium commented Mar 14, 2017 via email

@michaelshobbs
Copy link
Member

I'm 👍 on a well test PR that solves this. README entry is also fine. @predakanga feel like taking either of those on?

@predakanga
Copy link
Author

Certainly, I'll have a look and see what kind of test coverage is needed this afternoon - at a glance I think covering that PS1 is set during execution but not during profile load for all code paths should do the trick.

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

No branches or pull requests

4 participants