search

technicalpickles / homesick

Your home directory is your castle. Don't leave your dotfiles behind.
MIT License
2.39k stars 125 forks source link

Use file/git libraries instead of always shelling out #74

Open nickserv opened 10 years ago

nickserv commented 10 years ago

Advantages

Ruby has a lot of awesome command-line-inspired libraries built in, including FileUtils, which could easily replace many calls to commands like mv, cp, rm, and ln.

Git stuff

Grit looks pretty awesome, as it's what GitHub uses internally. See Supercharged Ruby-Git. Grit is deprecated, and its readme recommends switching to Rugged.

JCook21 commented 10 years ago

:+1:

trobrock commented 10 years ago

This should also make things easier to test :100:

technicalpickles commented 10 years ago

major performance and stability boosts

This is all relative. homesick in reality doesn't do so much work, so it doesn't really need to be super charged.

It's going to depend on a lot on what functions need to be done. When I first wrote jeweler, grit was mostly read-only access. I used ruby-git for a lot of things, but then still had to shell out.

Ruby has a lot of awesome command-line-inspired libraries built in, including FileUtils, which could easily replace many calls to commands like mv, cp, rm, and ln.

For what it's worth, these are calls still needing to be stubbed out in test. Current code uses system, which is just as stubbable.

Not saying "no", just saying "yes, but..." :grin:

nickserv commented 10 years ago

@technicalpickles You have a good point about still needing to stub, but I don't think it would be much of an issue. And I'm not too familiar with how stubs work especially with RSpec, but I feel like there might be nicer stubs built into things like grit. I could be wrong though.

technicalpickles commented 10 years ago

I feel like there might be nicer stubs built into things like grit.

This isn't a good assumption. Depending on how it's written, it might actually be more complex.

The best bet is probably to get a PR going to swap out system calls for grit or something else, review the implementation from there.

nickserv commented 10 years ago

The best bet is probably to get a PR going to swap out system calls for grit or something else, review the implementation from there.

If you want, I could try this after I'm done with some of the other refactoring I've been working on

technicalpickles commented 10 years ago

@thenickperson sure. I would suggest keeping the changes tightly focused around that, to make reviewing the diff easier.

nickserv commented 10 years ago

I'm pretty much done using libraries for the non-git system calls. That part is easy thanks to FileUtils. However, the git part is a bit trickier. I have managed to get some of the system calls switched over to Grit, which seems to be working nicely for replacing some of the system calls.

However, a lot of our tests expect certain output to result from git-related commands, because homesick can use part of git's command line interface for convenience. This means that homesick's output depends on the git shell, but libraries like Grit aren't meant to reimplement git's porcelain command output.

I'm thinking of three options for dealing with this:

  1. Switch to a git library entirely, removing all git-related system calls. This is probably a bad idea, since we would either have to show less output for git-tasks, or reimplement part of git's UI ourselves.
  2. Use a git library for commands that don't show output to the user.
  3. Keep shelling out for all git-related functionality. This may or may not be more efficient than option 2, depending on how long it takes to load libraries and if they would be slower or faster than shelling out. Another thing to consider is that some Ruby git libraries still shell out to git. I'm starting to feel like this would be our best option.
JCook21 commented 10 years ago

I think I'm responsible for a lot of the tests that check the text that shell commands return. I don't particularly like this as I think it makes for brittle tests but it was the simplest way of adding tests at the time. If you do replace the system calls I'd be in favour of mocking and stubbing the library calls wherever possible and I'd be happy to help with this.

nickserv commented 10 years ago

The problem is that not only would we have to change the expected test output, but we would probably have to remove it entirely. As far as I know, libraries like Grit don't show you any command line output for git commands. This means that we would either have to remove that from homesick, or reimplement large portions of git's command line output.

Thanks for offering your help, but I'm feeling like it won't be worth the trouble to replace our system calls to git. So far, I have only found two system calls that we use for git that can easily be swapped out to Grit without breaking anything (including command line output). We rely on the UI too heavily for it to be an easy transition. Let me know what you think, though.

nickserv commented 10 years ago

Now that Grit is deprecated in favor of Rugged, I just created a new use-rugged branch using Rugged to replace some shell calls. This new branch is based off of my older use-grit branch, though it's only as complete as my use-grit branch was previously (very incomplete).

While I'm starting to prefer Rugged's interface (it can discover repositories from subdirectories, which is pretty cool), I'm still having having the same problem with it being a Ruby client for git that does not display output on the command line by default (as supposed to shelling out to git).

If we're going to use a git wrapper library fully or partially, I think we should definitely use Rugged over Grit. However, we still have to deal with the UI problems I mentioned above.

CC @christianbundy @JCook21

nickserv commented 10 years ago

I have created a table of my current status on use-rugged.

Unfortunately, there are several important git features that homesick currently uses that Rugged does not support (such as committing, pulling, full status display, and submodules). There are places where Rugged could be used to replace shell calls, but not without losing git's useful command line output. Output for some git commands would have to be rewritten by us in order to fully switch to Rugged.

I think that if we wanted to remove our git dependency entirely, it would be best to use Rugged for some git features and require the user to use homesick exec for other functionality like pushing, pulling, committing, and status viewing. What do you think?