buildinspace / peru

a generic package manager, for including other people's code in your projects
MIT License
1.12k stars 69 forks source link

Peru should check for the presence of external tools before invoking them #225

Open colindean opened 2 years ago

colindean commented 2 years ago

What inspired #224 and #223 (to a degree) was me installing Peru as a dev-dependency in a project where I need to get data from internal artifact storage before running tests. Turns out that the container in which we're running tests does not have git installed! This makes sense, because the container only has the bare minimum stuff necessary to retrieve dependencies and run tests. Another container has already handled the initial git clone.

Poking around a little, the git plugin uses git, obviously:

https://github.com/buildinspace/peru/blob/f16cd2dd443ff210c748f4d021eab3249be52c08/peru/resources/plugins/git/git_plugin.py#L13-L17

and the cache module uses it for the backend:

https://github.com/buildinspace/peru/blob/f16cd2dd443ff210c748f4d021eab3249be52c08/peru/cache.py#L54-L58

I think both of these could stand to benefit from another module that abstracts away some git command building, minimally starting with a "is git available?" check. Having a user-friendly error that git isn't installed would have saved me (and the person who actually started using Peru at my behest) some time. This probably applies to the other plugins, although I didn't look at them.

The abstraction probably shouldn't grow to the size of GitPython without considering migrating to that tool.

oconnor663 commented 2 years ago

The main reason we haven't done something like this already is that peru tries pretty hard to keep itself to a single external invocation of the git binary in the happy path. So like if you run peru sync it does a tun a work and runs a bunch of commands. But if you run peru sync again, it does very little. Currently only a single command to check the status of the tree. This is an imporant part of the "put peru sync in your Makefile or at the top of your Bash script and then forget about it" strategy.

So if checking for git meant shelling out to git --version or similar, that wouldn't be ideal. Is there another way of checking that you had in mind?

colindean commented 2 years ago

That hidden abstraction layer is wonderful and one of the things I really like about the tool.

I think this check could be achieved through the use of which , which the shutil built-in has in shutil.which():

import shutil

def is_utility_available(cmd):
  return shutil.which(cmd) is not None

def require_git():
  if not is_utility_available("git"):
    raise MissingUtilityException("git is unavailable. Please install git and re-run your command.")

Drop require_git() early into any path that will need git and the user who forgot git will love the helpful error message instead of a FileNotFoundError or whatever is thrown with a long traceback.