Should we drop our runtime dependency on git?
nickserv opened this issue · 9 comments
Libraries like Rugged open the possibility of using git features implemented in Ruby instead of shelling out to git. But should homesick still depend on having git installed at runtime? The alternative would be to keep depending on git and to show warning/error messages when a command fails because git could not be found.
Related issues
I'd prefer to drop the runtime dependency on git.
While I think that depending on git is reasonable for dotfile managers built with the Bourne Shell or bash (rcm, dotum, ellipsis, etc), I think that we should remove any runtime dependencies that we can. Are we shelling out for anything else, or is git the only offender?
Regardless, I strongly support #97 being merged – I think that there are tons of use-cases that have nothing to do with git. Thanks for starting this issue @nicolasmccurdy!
I completely agree with @nicolasmccurdy and @christianbundy and would like to see us drop the runtime dependency on git. I think the main Homesick class is beginning to get a little bloated and I would really like to see us take a hard look at how we can make sure the code adheres to single responsibility principles (as well as other SOLID principles). I think anything that can remove dependencies can be helpful with that process.
What would be the best way to proceed with this? Is it worth starting a poll for any contributors to vote on this issue?
Are we shelling out for anything else, or is git the only offender?
@christianbundy We're still shelling out to non-git commands in master, but the only commands that I haven't found Ruby implementations for are git commands. You can see my progress on this here.
I think the main Homesick class is beginning to get a little bloated and I would really like to see us take a hard look at how we can make sure the code adheres to single responsibility principles (as well as other SOLID principles). I think anything that can remove dependencies can be helpful with that process.
@JCook21 Agreed. I'm more interested in removing unnecessary git wrapping code (to make things simpler and more maintainable) than removing the need for git to be installed. That being said, I still think it would be neat if homesick could be used without git.
Is it worth starting a poll for any contributors to vote on this issue?
@JCook21 I started this issue with the main intention of seeing how many people would agree/disagree to this. Unfortunately GitHub isn't the best place for votes, so we could always start a poll elsewhere. That being said, if we're going to tell people about the poll here anyway it would probably be easiest to just keep this issue and not use any external polls.
I don't think we can fully drop it until all the commands we use. That said, we probably can lessen our dependency. I've been casually following these related threads, and my current thinking is like:
- create a git facade/adapter, which all got interactions go through
- have this facade check for git availability and version at construction time
- have a method for each git interaction we need, and raise an exception if it's not installed or not recent enough
I would start with just shelling out to git. But then, could write a different implementation using rugged or grit or whatever. And then have a composite that tries a the rugged adapter, and falls back to the system, and raises an error if nothing else.
I think this gives us some flexibility to try out other git implementations. It also is a boon for testing, as it gives a place for the main tests to stub out (and not have to rely on command output).
@technicalpickles I think that sounds like a good approach and is something very much like what I was thinking of too.
@technicalpickles I would rather simplify homesick's commands by requiring git commands like pull, push, commit, etc. to be used with homesick exec git COMMAND instead of homesick exec. This way, we can remove a lot of extra code that simply wraps git commands and simplify things for the user as well with the new homesick exec command. However, I do think that if we did have to stick with both git and rugged for a while, using the adapter pattern would be an awesome idea.
I also realized something that could be an issue with using an adapter for git and rugged. According to what you said, it seems like you want the adapter to prioritize rugged over shell calls to git. However, this could lead to a situation where the rugged version of a command does not have any command line output, or very simple output, as supposed to the git counterpart.
By the way, please check out my recent research on what commands would have to change if we removed our git dependency with rugged.
In my opinion, all of these commands except clone and generate should be removed, and users could just use homesick exec git COMMAND to use any of the deprecated git commands.
Another alternative that I'm starting to like more is to make simple stubs so that homesick exec DEPRECATED_GIT_COMMAND could be an alias to homesick exec git DEPRECATED_GIT_COMMAND. This would be similar to what we're doing now with shelling out, except it would be much simpler because we could remove all our extra command line output and args support and alias all commands at once instead of making a method for every wrapped git shell command. This way, we could still have commands similar to all our existing commands while using rugged to lower the dependency on git while removing a lot of lines of code. I'm curious to what you think about this alternative (especially @technicalpickles).
I would rather simplify homesick's commands by requiring git commands like pull, push, commit, etc. to be used with homesick exec git COMMAND instead of homesick exec. This way, we can remove a lot of extra code that simply wraps git commands and simplify things for the user as well with the new homesick exec command.
Agree. I was pretty hesistant to add those in the first place, as their presence would beget other additions. Now that we have them though, we'll need to make sure to tread carefully to avoid breaking people's workflows.
Maybe we deprecate those now, and then drop them next major release?
Sure, sounds good! I started working on replacing some of the git commands with exec aliases.
I'm thinking that for now, we could use the aliases for exec and warn that the arguments might be different because they would be handled with git directly instead of homesick (the advantage being that we wouldn't have to write arg support and all git args would automatically be supported). We might also want to show deprecation warnings on the command line for these aliases, showing the commands you could use in newer versions of homesick. Then, maybe we could remove the commands entirely in homesick 2.0 or something like that. What do you think?