All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Baudis <pasky@suse.cz>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 5/7] git mv: Support moving submodules
Date: Thu, 17 Jul 2008 15:06:51 +0200	[thread overview]
Message-ID: <20080717130651.GU32184@machine.or.cz> (raw)
In-Reply-To: <7vhcapme0q.fsf@gitster.siamese.dyndns.org>

  Hi!

On Wed, Jul 16, 2008 at 07:37:57PM -0700, Junio C Hamano wrote:
> Petr Baudis <pasky@suse.cz> writes:
> 
> > ... The horrid
> > index_path_src_sha1 hack is unfortunately much worse,
> 
> This one certainly is ugly beyond words.

  ;-)

> By the way, why is it even necessary to rehash the contents when you run
> "mv"?
> 
> IOW,
> 
> 	$ >foo
>         $ git add foo
>         $ echo 1 >foo
>         $ git mv foo bar
> 
> makes "foo" disappear from the index and adds "bar", but it makes the
> local change added to the index at the same time.
> 
> 	Side note. It actually is a lot worse than that.  When you move
> 	something to another existing entry, the code does not even add
> 	the object name of moved entry recorded in the index, nor rehashes
> 	the moved file.  This is totally inconsistent!
> 
> I personally think these buggy behaviours you are trying to inherit are
> making this patch more complex than necessary.  Perhaps this needs to be
> fixed up even further (you did some fix with the first patch) before
> adding new features?  For example, I think it is a bug that the
> "overwrite" codepath does not work with symlinks.

  I agree that it would be much cleaner to fix this; I got puzzled about
this behaviour a bit, but I was afraid to break the traditional
behaviour. However, if you are feeling this brave, patch to follow up
shortly. :-)

>  * Then, only for case (1), you do not call add_file_to_cache() -- because
>    you know you do not have anything you can rehash; instead, factor out
>    the codepath "git-update-index --cacheinfo" uses and call that.

  This is excellent hint, sort of what I hoped for, thanks! I forgot
about --cacheinfo completely, which is truly a shame especially when I
look at the history of this switch. ;-) (BTW, curiously, the commit
lists Linus as an author even though the patch is yours. Maybe this was
merely some imperfection of the early scripts around Git, though.)
I really did not touch git internals for way too long.

-- 
				Petr "Pasky" Baudis
GNU, n. An animal of South Africa, which in its domesticated state
resembles a horse, a buffalo and a stag. In its wild condition it is
something like a thunderbolt, an earthquake and a cyclone. -- A. Pierce

  reply	other threads:[~2008-07-17 13:07 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-16 19:11 [RFC][PATCH 0/7] Submodule support in git mv, git rm Petr Baudis
2008-07-16 19:11 ` [PATCH 1/7] git-mv: Remove dead code branch Petr Baudis
2008-07-16 19:11 ` [PATCH 2/7] t7400: Add short "git submodule add" testsuite Petr Baudis
2008-07-16 19:11 ` [PATCH 3/7] git submodule add: Fix naming clash handling Petr Baudis
2008-07-16 19:11 ` [PATCH 4/7] submodule.*: Introduce simple C interface for submodule lookup by path Petr Baudis
2008-07-16 19:11 ` [PATCH 5/7] git mv: Support moving submodules Petr Baudis
2008-07-17  2:37   ` Junio C Hamano
2008-07-17 13:06     ` Petr Baudis [this message]
2008-07-17 22:31       ` [PATCH] git-mv: Keep moved index entries inact Petr Baudis
2008-07-17 22:34         ` [PATCH] git mv: Support moving submodules Petr Baudis
2008-07-19 23:54         ` [PATCH] git-mv: Keep moved index entries inact Junio C Hamano
2008-07-21  0:23           ` Petr Baudis
2008-07-21  0:25             ` [PATCHv2] " Petr Baudis
2008-07-21  4:36               ` Junio C Hamano
2008-07-26  6:46               ` Junio C Hamano
2008-07-27 13:41                 ` Petr Baudis
2008-07-27 13:47                   ` [PATCH] t/t7001-mv.sh: Propose ability to use git-mv on conflicting entries Petr Baudis
2008-07-28  1:13                     ` Junio C Hamano
2008-07-28  1:21                       ` Junio C Hamano
2008-07-28 14:20                 ` [PATCHv2] git-mv: Keep moved index entries inact SZEDER Gábor
2008-07-28 15:06                   ` Johannes Schindelin
2008-07-28 15:14                     ` Johannes Schindelin
2008-07-28 18:24                       ` Johannes Schindelin
2008-07-28 19:19                     ` Junio C Hamano
2008-07-28 23:41                       ` Johannes Schindelin
2008-07-28 23:55                         ` Johannes Schindelin
2008-07-29  0:17                       ` Petr Baudis
2008-07-29  0:46                         ` Junio C Hamano
2008-07-29  5:23                           ` Junio C Hamano
2008-08-04  7:49                 ` Not going beyond symbolic links Junio C Hamano
2008-08-04  7:51                   ` [PATCH 1/2] update-index: refuse to add working tree items beyond symlinks Junio C Hamano
2008-08-04  7:52                   ` [PATCH 2/2] add: " Junio C Hamano
2008-08-05  0:21                   ` Not going beyond symbolic links Linus Torvalds
2008-08-05  0:54                     ` Junio C Hamano
2008-08-05  1:43                       ` Linus Torvalds
2008-08-05  1:59                         ` Johannes Schindelin
2008-08-05  2:28                           ` Linus Torvalds
2008-08-05  6:11                             ` Junio C Hamano
2008-08-05 12:54                               ` Dmitry Potapov
2008-08-05 23:57                                 ` Junio C Hamano
2008-08-05 17:15                               ` Linus Torvalds
2008-08-05  4:44                           ` Junio C Hamano
2008-08-05 11:23                             ` Johannes Schindelin
2008-08-05  3:01                         ` Junio C Hamano
2008-08-05  3:04                           ` david
2008-08-07  6:52                     ` Junio C Hamano
2008-08-08  1:45                       ` [PATCH] powerpc: Do not ignore arch/powerpc/include Junio C Hamano
2008-08-08  3:13                         ` Stephen Rothwell
2008-08-08 20:55                       ` Not going beyond symbolic links Junio C Hamano
2008-08-08 23:45                         ` Linus Torvalds
2008-07-21  1:20             ` [PATCH] git-mv: Keep moved index entries inact Johannes Schindelin
2008-07-21  7:18               ` Petr Baudis
2008-07-21  7:38                 ` Junio C Hamano
2008-07-16 19:11 ` [PATCH 6/7] git rm: Support for removing submodules Petr Baudis
2008-07-16 22:41   ` Johannes Schindelin
2008-07-17 12:35     ` Petr Baudis
2008-07-17 12:59       ` Johannes Schindelin
2008-07-16 19:11 ` [PATCH 7/7] t7403: Submodule git mv, git rm testsuite Petr Baudis

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=20080717130651.GU32184@machine.or.cz \
    --to=pasky@suse.cz \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.