keep can't init when howdoi is installed via pipx

[justin-f-perez]

What happened:

➜  ~ howdoi --save create an upstream pr using gh cli
sh: keep: command not found
ERROR: No commands found in stash. Add a command with "howdoi --save <query>".
Traceback (most recent call last):
  File "/Users/username/.local/pipx/venvs/howdoi/lib/python3.9/site-packages/howdoi/howdoi.py", line 576, in _stash_save
    keep_utils.save_command(cmd_key, answer, title)
  File "/Users/username/.local/pipx/venvs/howdoi/lib/python3.9/site-packages/keep/utils.py", line 192, in save_command
    with open(json_path, 'w') as f:
FileNotFoundError: [Errno 2] No such file or directory: '/Users/username/.keep/commands.json'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/username/.local/bin/howdoi", line 8, in <module>
    sys.exit(command_line_runner())
  File "/Users/username/.local/pipx/venvs/howdoi/lib/python3.9/site-packages/howdoi/howdoi.py", line 810, in command_line_runner
    utf8_result = howdoi(args).encode('utf-8', 'ignore')
  File "/Users/username/.local/pipx/venvs/howdoi/lib/python3.9/site-packages/howdoi/howdoi.py", line 623, in howdoi
    return _parse_cmd(args, res)
  File "/Users/username/.local/pipx/venvs/howdoi/lib/python3.9/site-packages/howdoi/howdoi.py", line 589, in _parse_cmd
    _stash_save(cmd_key, title, answer)
  File "/Users/username/.local/pipx/venvs/howdoi/lib/python3.9/site-packages/howdoi/howdoi.py", line 579, in _stash_save
    keep_utils.save_command(cmd_key, answer, title)
  File "/Users/username/.local/pipx/venvs/howdoi/lib/python3.9/site-packages/keep/utils.py", line 192, in save_command
    with open(json_path, 'w') as f:
FileNotFoundError: [Errno 2] No such file or directory: '/Users/username/.keep/commands.json'

What you expected to happen:

The command to be saved without error

Output with --explain

N/A

[justin-f-perez]

why

This happens because keep doesn't end up on the path when you do a normal pipx install howdoi

bad workaround

exposes keep but also includes other dependencies that the user may not want

$ pipx install --include-deps howdoi
$ pipx list
# ...
   package howdoi 2.0.18, Python 3.9.7
    - howdoi
    - keep
    - normalizer
    - pygmentize

better workaround

only adds keep

$ pipx install howdoi
$ pipx inject howdoi keep --include-apps
$ pipx list
# ...
   package howdoi 2.0.18, Python 3.9.7
    - howdoi
    - keep

[gleitz]

Hmm - I had not tried an install with pipx before. Might this be an issue with how pipx manages the path for dependencies of packages? Seems like an issue that could be raised with them, at least to figure out if either of these workaround are recommended.

[justin-f-perez]

I think there are a couple of ways to resolve this:

1. catch the error from the os.system(keep init) call by checking the result and printing a user-friendly error message
2. add an entrypoint for keep (this makes it available on PATH by pipx)
3. stop making os.system() calls and implement the init logic in python

I lean toward (3). Staying inside of the python interpreter is more predictable than issuing commands to an unknown shell- not to mention the security implications/greater attack surface such calls create.

I tried invoking keep's init cmd from within howdoi, but for some reason this didn't work (maybe something to do with click, I'm not super familiar). We could do the check for the keep directory ourselves but, unfortunately, the ~/.keep directory is hard-coded everywhere in keeps src code (not to mention polluting users' home directories, being unconfigurable, ignoring XDG). so, if we do check for and create the path, our implementation would break if keep ever changes the path.

thoughts?

[xzhang8102]

Same problem when howdoi is installed via Homebrew.

[gleitz]

I'm surprised to see that homebrew has been keeping their howdoi package up to date. If we fix this issue then I could add homebrew (and pipx) as a supported method of installation.

I'd be happy to accept a pull request to fix this. I'd also like to understand the core issue a bit better.

Going by @justin-f-perez's original submission, I see the output:

File "/Users/username/.local/pipx/venvs/howdoi/lib/python3.9/site-packages/howdoi/howdoi.py", line 579, in _stash_save
    keep_utils.save_command(cmd_key, answer, title)
File "/Users/username/.local/pipx/venvs/howdoi/lib/python3.9/site-packages/keep/utils.py", line 192, in save_command
    with open(json_path, 'w') as f:
FileNotFoundError: [Errno 2] No such file or directory: '/Users/username/.keep/commands.json'

This suggests that keep does indeed get installed, but some internal workings of keep are broken when installed via pipx or brew.

Looking at the keep repo, I see that the commands.json file should be created by this first_time_use function. This usually gets called here or here by the CLI.

howdoi attempts to call keep init if we get an error saving to the stash. However, I don't believe that the keep command is going to be on the PATH, because it's only installed as a dependency of howdoi and not as a top level application.

So:

1. Can we remove the os.system call to keep init and replace it with keep_utils.first_time_use? I wonder if the sys.exit(0) is going to get in our way?
2. Might we need to ask the maintainer of keep to expose a new init function that perhaps operates slightly differently?

[justin-f-perez]

Hi meant to respond sooner, been slammed

This suggests that keep does indeed get installed, but some internal workings of keep are broken when installed via pipx or brew.

yes, but it's not so much that any 'internal workings' are broken as it is the command simply cannot be found by the shell because it's not in any directory in the $PATH where the user's shell will search for executables.

howdoi attempts to call keep init if we get an error saving to the stash. However, I don't believe that the keep command is going to be on the PATH, because it's only installed as a dependency of howdoi and not as a top level application.

This is also correct. Additionally, note that:

The subprocess module provides more powerful facilities for spawning new processes and retrieving their results; using that module is preferable to using this function.
https://docs.python.org/3/library/os.html#os.system

The preferred method, for security reasons, when calling system commands where an exception should be raised on a non-zero exit code, and which don't require the shell (as would be the case here if keep were on the PATH) is

subprocess.check_output(['keep', 'init'], shell=False)

To keep, keep, or not to keep, keep?

So:

1. Can we remove the os.system call to keep init and replace it with keep_utils.first_time_use? I wonder if the sys.exit(0) is going to get in our way?
2. Might we need to ask the maintainer of keep to expose a new init function that perhaps operates slightly differently?

I recall having similar questions/thoughts when I examined the source. It didn't appear that the author had designed it in a way that was very flexible or re-usable. I believe also that the path to the keep directory was hard-coded/non-configurable and didn't conform to XDG Base Directory Specification

Furthermore, in https://github.com/OrkoHunter/keep/blob/master/keep/utils.py#L53-L63 ... why sleep? It looks to me like a poor attempt at pretending to do work and adds an extra 1 second to initialization- not a huge deal for end users, but it means every unit test that needs to exercise keep init is a full 1s slower (which is, in many cases, slow for a unit test). Not only that, but if for some reason keep init fails, it does so after printing "OK".

OK, done ranting. Just pointing out (for future design consideration) that keep should be treated with a little bit of caution (i.e., should design howdoi in such a way that swapping out keep if things go sideways in the future won't be too painful). Additionally, it might be a good idea to pin a specific version. If keep isn't designed to be used as a library, there's little reason to believe the developers won't make breaking changes to their API.

Getting back on track- er, I mean PATH

You might be able to put keep on the path by making it an "extra" with a console_script (users would install it like: pipx install 'howdoi[keep]'). I would guess that this also involves notifying the maintainers of the homebrew formula (maybe via pull request) about the new extra so it can be added by default. I've never done this before and unfortunately don't have time to figure out how to test it, but hopefully it's enough to help someone else get started down the right... PATH. It would probably be wise to print a user-friendly error message when someone attempts to save a query without either the keep extra installed independently or via the "extra".

[gleitz]

Thanks for this! I think we may be able to bring some of these issues up with keep. A basic init function without a sleep would be a great start.