Re: [PATCH] git-subtree: Add prune mode

From: Avery Pennarun <apenwarr@gmail.com>
Date: Aug 2, 2010, 02:54 UTC
Message-ID: <AANLkTinLuMwGw_0gXDBBdgT=eg2T_eqtRCNHh2PgV-7U@mail.gmail.com>
In-Reply-To: <1280654756-9039-1-git-send-email-santi@agolina.net>

On Sun, Aug 1, 2010 at 5:25 AM, Santi Béjar <santi@agolina.net> wrote:

Show 11 quoted lines

> Add prune mode (flag --prune) with the following properties:
>
> * The history must be as clean as possible
> * The directory content must be equal to the external module,
>  at least when you add/update it[b]
> * The subproject should be able to switch back and forth between
>  different versions.
>
> [b] A consequence of this is that it loses all changes
>    made in the subtree. If they are important you have to extract
>    them, apply them and add the subproject back.

I think I started to reply to this before but I can't quite remember what happened. Anyway, I have several concerns with this patch:

- calling it "prune" is pretty incorrect.  It doesn't remove anything
from your history.  It silently loses patches from your tree, but
that's not "pruning" really.  I suggest "--squash
--discard-local-changes" or something.  ie. it's a variant of squash,
and it throws things away.  We want both of those to be clear.

- I'm not convinced this concept is even a good idea.  Who on earth
wants to silently lose changes?  Default --squash behaviour is to
merge the old branch with the new branch.  If you want to throw stuff
away, that should probably be an entirely separate operation from the
merging.  ie. "git subtree discard-local-changes --prefix=whatever" to
create a new patch that undoes all the local changes to this branch;
then "git subtree merge --squash" to update to a different upstream.

- In what sense is the history from this any "cleaner" (or any
different) from --squash?  It seems like, in fact, the history will be
a *lie* since it talks about merging but actually reverts changes.

> As all the history is lost and you never merge commits
> 'split' is not necessary, but it is basically:
>
> $ git filter-branch --subdirectory-filter $prefix

Are you saying that 'git subtree split' doesn't work after the discard-local-changes operation? If so, we should fix that, I think. Otherwise it's very confusing.

Show 11 quoted lines

> +prune_msg()
> +{
> +       dir="$1"
> +       newsub="$2"
> +
> +       git show -s --pretty="tformat:Subtree '$dir/': %h %s" $newsub
> +       echo
> +       echo "git-subtree-dir: $dir"
> +       echo "git-subtree-split: $newsub"
> +}
> +

Hmm. I'm rather concerned about this one. What's the top line of the commit message? add_msg, add_squashed_msg, rejoin_msg, and squash_msg are all much clearer than this. It also seems that it doesn't honour the -m flag here.

Show 6 quoted lines

>  cmd_add()
>  {
> -       if [ -e "$dir" ]; then
> +       if [ -e "$dir" -a -z "$prune" ]; then
>                die "'$dir' already exists.  Cannot add."
>        fi

This is the 'add' command. I'm not sure it should have special behaviour with prune.

(Arguably 'add' should let you optionally overwrite an existing directory if it already exists. But if so, I don't think this should only happen with --prune; it should probably be a special --overwrite or --force option.)

Show 5 quoted lines

>        debug "Adding $dir as '$rev'..."
> +       if [ -d "$dir" ]; then
> +           git rm -r -q $dir
> +       fi
>        git read-tree --prefix="$dir" $rev || exit $?

Isn't there some plumbing command we can use instead of 'git rm'? I don't really know what. Does anyone have any suggestions?

Show 6 quoted lines

>        A new commit is created automatically, joining the imported
>        project's history with your own.  With '--squash', imports
>        only a single commit from the subproject, rather than its
> -       entire history.
> +       entire history. With '--prune', imports only the contents of
> +       the commit from the subproject without any history.

This is unclear. "Without any history" isn't really accurate, since a new commit message is generated; actually the history importing is precisely the same as what happens with --squash.

Show 5 quoted lines

> +--prune::
> +       Instead of merging the history (full or squashed) from the
> +       subtree project, produce only a single commit that
> +       reproduce the exact content in the preffix as in the
> +       subtree.

s/preffix/prefix/

Show 5 quoted lines

> +       It has similar features as the --squash option, namely
> +       reduces the clutter (althougth --prune reduce it even
> +       more), helps avoiding problems when the same subproject is
> +       include multiple time and can switch back and forth
> +       between different version of a subtree.

s/althougth/although/ s/include multiple/included multiple/

I don't understand how --prune supposedly "reduces it even more" than --squash. What clutter? How does it reduce it *more*? How does it avoid problems when the same subproject is included multiple times? What problems are those?

You can switch back and forth between different versions of a subtree, but this is true of --squash as well - that's why I think this operation should be a subcommand of --squash instead.

Show 6 quoted lines

> +       The main difference is that with --prune the content of
> +       the prefix always matches the content of the subproject,
> +       while with --squash it merges you changes with the changes
> +       from the subtree. If you want to keep your changes you
> +       have to extract them, apply them in the external
> +       repository and add the subproject back.

s/you changes/your changes/

This last part is especially strange: if you want to keep your own changes, why do you want to use --prune at all? --squash does that automatically.

Have fun,

Avery