git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Michael Haggerty <mhagger@alum.mit.edu>
Cc: Christian Couder <christian.couder@gmail.com>,
	Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org
Subject: Re: [PATCH] disable grafts during fetch/push/bundle
Date: Thu, 6 Mar 2014 12:48:03 -0500	[thread overview]
Message-ID: <20140306174803.GA30486@sigill.intra.peff.net> (raw)
In-Reply-To: <5318A537.4010400@alum.mit.edu>

On Thu, Mar 06, 2014 at 05:41:27PM +0100, Michael Haggerty wrote:

> > We can wrap that in "git replace --convert-grafts", but I do not think
> > grafts are so common that there would be a big demand for it.
> 
> It's probably easier to wrap it than to explain to Windows users what
> they have to do.

How would Windows users get a graft file in the first-place? There's no
GUI for it! ;)

It should be easy to do "--convert-grafts", though, and I think it fits
into the scheme we're discussing below.

> > I think it would be nice to have a set of "mode" options for
> > "git-replace" to do basic editing of a sha1 and install the result
> > (technically you could split the editing into a separate command, but I
> > do not see the point in editing a sha1 and then _not_ replacing it).
> 
> If modifying without replacing is needed, it would be pretty easy to add
> an option --stdout that writes the SHA1 of the modified object to stdout
> instead of creating a replace reference.  That way what you want 95% of
> the time is the default but there is still an escape hatch.

Agreed. I had originally though that perhaps something like this should
be part of "hash-object", and that "replace" should farm out the work.
But thinking on it more, it doesn't really make sense as part of
"hash-object".

> > Perhaps:
> > 
> >   # pretty-print sha1 based on type, start $EDITOR, create a
> >   # type-appropriate object from the result (e.g., using hash-object,
> >   # mktree, or mktag), and then set up the object as a replacement for
> >   # SHA1
> >   git replace --edit SHA1

Here's a rough series that gets us this far:

  [1/4]: replace: refactor command-mode determination
  [2/4]: replace: use OPT_CMDMODE to handle modes
  [3/4]: replace: factor object resolution out of replace_object
  [4/4]: replace: add --edit option

It shouldn't be too hard to do "--graft" or "--convert-grafts" on top.

I also noticed that doing:

    git replace foo foo

is less than friendly (we notice the cycle, but just barf). It's
especially easy to do with "git replace --edit", if you just exit the
editor without making changes.  Or if you make changes to an
already-replaced object to revert it back, in which case we would want
to notice and delete the replacement.

So I think we want to have "git replace foo foo" silently converted into
"git replace -d foo" (but without an error if there is no existing
replacement), and then "--edit" will just do the right thing, as it's
built on top.

I also noticed that the diff engine does not play well with replacements
of blobs. When we are diffing the trees, we see that the sha1 for path
"foo" is the same on either side, and do not look further, even though
feeding those sha1s to builtin_diff would fetch the replacements.  I
think compare_tree_entry would have to learn lookup_replace_object (and
I suspect it would make tree diffs noticeably slower when you have even
one replace ref).

> "git replace" could support some of the options that "git filter-branch"
> can take, like --env-filter, --msg-filter, etc. (at least if the target
> is a commit object).
> 
> All of this would make it possible to build up the changes that you want
> to integrate via "filter-branch" piecemeal instead of having to have a
> single monster filter-branch invocation.  For example,

Right. I was tempted to suggest that, too, but I think it can get rather
tricky, as you need to replace in a loop, and sometimes the exact
objects you need aren't obvious.  For example, a common use of
"--index-filter" is to remove a single file. But to remove
"foo/bar/baz", you would need to loop over each commit, find the tree
for "foo/bar", and then remove the "baz" entry in 

Still, I really like the workflow of having decent "replace" tools,
followed by "cementing" the changes into place with a "filter-branch"
run (which, btw, does not yet know how to cement trees and blobs into
place). It lets you work on the filtering incrementally, and even share
or work collaboratively on it by pushing refs/replace).

And as you mention, it could be a heck of a lot faster than what we have
now.

-Peff

  reply	other threads:[~2014-03-06 17:48 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-04 17:48 [PATCH] disable grafts during fetch/push/bundle Jeff King
2014-03-04 20:52 ` Junio C Hamano
2014-03-05  0:56   ` Jeff King
2014-03-05 18:49     ` Junio C Hamano
2014-03-05 18:52       ` Jeff King
2014-03-05 19:18         ` Junio C Hamano
2014-03-05 19:28           ` Jeff King
2014-03-05 20:24             ` Junio C Hamano
2014-03-06  8:42           ` Michael Haggerty
2014-03-06  9:17             ` Christian Couder
2014-03-06 15:56             ` Jeff King
2014-03-06 16:41               ` Michael Haggerty
2014-03-06 17:48                 ` Jeff King [this message]
2014-03-06 17:49                   ` [RFC/PATCH 1/4] replace: refactor command-mode determination Jeff King
2014-03-06 17:49                   ` [RFC/PATCH 2/4] replace: use OPT_CMDMODE to handle modes Jeff King
     [not found]                     ` <CAP8UFD2c0UKT8Uyw4j9SzKGx2oLn=o7N-dtvQHPaaBtLT6ggcw@mail.gmail.com>
2014-03-06 18:48                       ` Jeff King
2014-03-06 17:49                   ` [RFC/PATCH 3/4] replace: factor object resolution out of replace_object Jeff King
2014-03-06 17:51                   ` [RFC/PATCH 4/4] replace: add --edit option Jeff King
2014-03-07  1:57                     ` Eric Sunshine
2014-03-07 17:17                       ` Jeff King
2014-03-06 19:00                   ` [PATCH] disable grafts during fetch/push/bundle Junio C Hamano
2014-03-06 19:07                     ` Jeff King
2014-03-06 23:01                   ` Philip Oakley
2014-03-06 23:29                     ` Michael Haggerty
2014-03-06 23:39                       ` Junio C Hamano
2014-03-07  7:08                         ` Christian Couder
2014-03-07 17:19                           ` Jeff King
2014-03-19 22:39                             ` Junio C Hamano
2014-03-21  0:49                               ` Jeff King
2014-03-06 23:48                       ` Philip Oakley
2014-03-04 23:36 ` Eric Sunshine
2014-03-05  0:37   ` Jeff King
2014-03-05  1:00     ` Eric Sunshine
2014-03-05  1:05       ` Jeff King
2014-03-05  1:07         ` Eric Sunshine

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140306174803.GA30486@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mhagger@alum.mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).