All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marcus Comstedt <marcus@mc.pp.se>
To: git@vger.kernel.org
Cc: gitster@pobox.com
Subject: Fix for normalization of foreign idents
Date: Mon, 23 Aug 2010 23:30:31 +0200	[thread overview]
Message-ID: <1282599032-11369-1-git-send-email-marcus@mc.pp.se> (raw)

Hi.

The new behaviour that $Id$ tags containing expanded idents from
other version control systems, nice though it is, has a rather
serious problem.  This is because convert_to_git is no longer
the inverse operation of convert_to_working_tree.  For native
git idents, the transformations are

  $Id$ --(c_t_w_t)--> $Id: 123...$ --(c_t_g)--> $Id$

but for foreign idents, it becomes

  $Id: blah$ --(c_t_w_t)--> $Id: blah$ --(c_t_g)--> $Id$

The result of this is that git may consider even newly checked out
files as modified, even though neither the file contents nor its
attributes have been modified after the checkout.  I say _may_, because
it can also happen that it decides based on the time stamps that it
doesn't need to compare the actual contents, in which case the file
does not show as modified.

The following patch fixes this by preserving the foreign ident
also in convert_to_git, meaning that convert_to_git is again the
inverse operation of convert_to_working_tree, with the following
transformation series:

  $Id: blah$ --(c_t_w_t)--> $Id: blah$ --(c_t_g)--> $Id: blah$

This restores correct and deterministic operation of status and
diff, meaning that if the file hasn't actually been modified, no
modifications are shown.

As you might suggest, always keeping the foreign ident would mean it
is never updated when you commit new versions of the file, which isn't
really what we want.  Keeping the foreign ident as long as the last
modification to the file was made in the previous version control
system makes perfect sense, but once we make a commit to the file
within git, it should be replaced with a git ident.  The patch is
therefore slightly more complex, adding an extra parameter to control
whether foreign idents are collapsed or not.  This parameter is set
to true only in the case when index_mem is called with write_object
set to true, which is to say when we create a new blob from the
working tree (i.e. we are committing the file).

I hope we can agree that this is a sound and unintrusive way of
handling the problem.  :-)

Incidentally, should one want to create a commit to replace a foreign
ident with a native git one without making any other changes to the
file, this is still simple to do.  All that is needed is to change any
character inside the expanded ident in the working tree, and the file
will show as modified, and will have the foreign ident removed on
commit.


  // Marcus

             reply	other threads:[~2010-08-23 21:30 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-23 21:30 Marcus Comstedt [this message]
2010-08-23 21:30 ` [PATCH] convert: fix normalization of foreign idents Marcus Comstedt
2010-08-23 21:35 ` Fix for " Jonathan Nieder
2010-08-23 21:44   ` Marcus Comstedt
2010-08-23 22:33     ` Jonathan Nieder
2010-08-23 23:46       ` Junio C Hamano
2010-08-24  7:23         ` Marcus Comstedt
2010-09-06  9:42     ` Marcus Comstedt
2010-09-06 21:07       ` Jonathan Nieder
2010-09-07 19:37         ` Marcus Comstedt
2010-08-23  7:05           ` [PATCH v2 1/2] convert: fix " Marcus Comstedt
2010-09-10  0:26             ` Junio C Hamano
2010-09-12 21:01               ` Marcus Comstedt
2010-09-12 21:44                 ` Junio C Hamano
2010-09-12 22:06                   ` Marcus Comstedt
2010-09-13 22:00                     ` [PATCH] convert: generalize checksafe parameter Marcus Comstedt
2010-09-07 19:16           ` [PATCH v2 2/2] t0021: test checkout and commit of foreign idents Marcus Comstedt
2010-09-07 20:00           ` [PATCH v2 0/2] fix normalization of foreign idents (now with test cases) Marcus Comstedt
2010-09-08  4:32           ` Fix for normalization of foreign idents Jonathan Nieder

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=1282599032-11369-1-git-send-email-marcus@mc.pp.se \
    --to=marcus@mc.pp.se \
    --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.