Re: [PATCH/RFC V2] stash: implement builtin stash

From: Jeff King <peff@peff.net>
Date: Mar 21, 2017, 05:31 UTC
Message-ID: <20170321053135.thk77soxc4irxbqj@sigill.intra.peff.net>
In-Reply-To: <20170313222318.23312-1-joel@teichroeb.net>

On Mon, Mar 13, 2017 at 03:23:18PM -0700, Joel Teichroeb wrote:

Show 5 quoted lines

> I've been working on rewriting git stash as a c builtin and I have all
> but three tests passing. I'm having a bit of trouble fixing them, as
> well as a few other issues, so I'd really appreciate some help. Don't
> bother commenting on the small details yet as I still need to go
> though the code to make sure it matches the code style guidelines.

Be careful that this is a bit of a moving target. There's been some feature work and some bug-fixes in git-stash.sh the past few weeks.

Show 11 quoted lines

> git stash uses the GIT_INDEX_FILE environment variable in order to not
> trash the main index. I ended up doing things like this:
> 
>     discard_cache();
>     ret = read_cache_from(index_path);
>     write_index_as_tree(orig_tree.hash, &the_index, index_path, 0, NULL);
>     discard_cache();
>     ret = read_cache_from(index_path);
> 
> in order to have an empty cache. Could someone take a look at my uses
> of the index and point out better ways to do it?

I suspect in the long run that you could pull this off without writing the intermediate index to disk at all. You can store multiple indices if you need to (read_cache_from is just a wrapper to operate on the_index). But while you're executing sub-programs, you're still probably going to need to do a lot of re-reading of the index.

Two things that I think might help break up the work:

  1. Rather than convert stash all at once, you can implement a "stash
     helper" in C that does individual sub-commands (or even smaller
     chunks), and call that from git-stash.sh. Eventually it
     git-stash.sh will just be a skeleton, and you can move that over to
     C and call the functions directly.

  2. You may want to start purely as a C wrapper that calls the
     sub-programs, which communicate with each other via GIT_INDEX_FILE,
     etc. Then you don't need to worry about manipulating the index
     inside the C program at first, and can focus on all the other
     boilerplate.

     Then piece by piece, you can replace sub-program calls with C
     calls. But you know you'll be working on top of a functional base.

I don't know if that's helpful or not.

-Peff