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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Aug 1, 2010 at 5:25 AM, Santi Béjar <santi@xxxxxxxxxxx> wrote:
> 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.

> +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.

>  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.)

>        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?

>        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.

> +--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/

> +       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.

> +       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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]