git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Finn Arne Gangstad <finnag@pvv.org>
To: "Henrik Grubbström" <grubba@roxen.com>
Cc: Jonathan Nieder <jrnieder@gmail.com>,
	Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org
Subject: Re: [PATCH v4 0/5] Patches to avoid reporting conversion changes.
Date: Mon, 7 Jun 2010 10:59:47 +0200	[thread overview]
Message-ID: <20100607085947.GA3924@pvv.org> (raw)
In-Reply-To: <Pine.GSO.4.63.1006061143000.27465@shipon.roxen.com>

On Sun, Jun 06, 2010 at 12:50:08PM +0200, Henrik Grubbström wrote:
[...]
>
> Currently (as I believe you know), git has no detection of when the  
> conversion mode for a file has changed, and it might even take a while  
> before the users notice that the repository is not normalized. eg:
>
>   0) There's a repository with some files containing crlf line endings.
>
>   1) User A notices that git now has native support for crlf
>      line endings, and adds the attribute eol=crlf for the
>      affected files.
>
>   2) User A does a git status, sees that .gitattributes is
>      modified, and commits it.

I think it would be best if git at this time could decide that the
affected files also become dirty. The ideal commit is one that
both alters the .gitattributes _and_ the affected files at the same
time, and git should make it easy to create that commit.

> [...]
>   6) User C is new to the project and does an initial git clone,
>      and ends up with a dirty index.

And the reason for this is mostly that unless you perform some special
actions, you will commit attributes and contents that are mismatched.

In your suggested mode, whay would happen if you did this:

$ git clone ......  (which has files that are "wrong" wrt line endings and
attributes for some .c files)
$ touch *.c

Would it still believe all *.c files were clean? Does it require an
actual other change at the same time to allow you to normalize the
file? That would be detrimental I think.  Changing newlines is best
done as a separate commit, intermingling newline changes and real
changes in the same commmit is not where you want to go.

However, for your ID string you obviously want this behaviour. I'm
guessing that hook is alreasy set up so that if you just touch the
file, it will still be treated as unmodified?

>
> What my patch set achieves is that user C above also gets a clean index.
>
> What it seems you want is that user A above should have all files that 
> got denormalized by the attribute change marked dirty at 2 (and 3).

That would indeed be a very welcome change.

> As I believe both behaviours may be desireable a config option and/or  
> attribute is needed. Any suggestions for a name (and default value)?

I think the default behaviour should be to mark files dirty if there
are ANY attribute changes that could cause content changes done to
them at all. I'm not sure that is exactly what your patch series is
allowing us to track though?

Just to be clear:

If you add this to your .gitattributes

*.c eol=lf

I think it would be very helpful if git then would treat all .c files
as "stat-dirty" the next time it updates its index.

A for config variables, what about:

core.rereadOnAttributeChanges = [true]/false    (default = true)

Which makes some sense for detecting it in 2, but not so much for
ignoring it in 6.

- Finn Arne

  reply	other threads:[~2010-06-07  8:59 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-01 14:41 [PATCH v4 0/5] Patches to avoid reporting conversion changes Henrik Grubbström (Grubba)
2010-06-01 14:41 ` [PATCH v4 1/5] sha1_file: Add index_blob() Henrik Grubbström (Grubba)
2010-06-01 14:41 ` [PATCH v4 2/5] strbuf: Add strbuf_add_uint32() Henrik Grubbström (Grubba)
2010-06-01 14:41 ` [PATCH v4 3/5] cache: Keep track of conversion mode changes Henrik Grubbström (Grubba)
2010-06-01 14:41 ` [PATCH v4 4/5] cache: Add index extension "CONV" Henrik Grubbström (Grubba)
2010-06-01 14:41 ` [PATCH v4 5/5] t/t0021: Test that conversion changes are detected Henrik Grubbström (Grubba)
2010-06-02  4:40 ` [PATCH v4 0/5] Patches to avoid reporting conversion changes Junio C Hamano
2010-06-03 16:00   ` Henrik Grubbström
2010-06-04  0:56     ` Jonathan Nieder
2010-06-04 11:59       ` Henrik Grubbström
2010-06-04 19:42         ` Jonathan Nieder
2010-06-06 10:50           ` Henrik Grubbström
2010-06-07  8:59             ` Finn Arne Gangstad [this message]
2010-06-07 16:37               ` Henrik Grubbström
2010-06-07 19:50                 ` Finn Arne Gangstad
2010-06-08 15:52                   ` Henrik Grubbström
2010-06-09 14:03                     ` Finn Arne Gangstad
2010-06-09 18:04                       ` Henrik Grubbström
2010-06-10 19:55                         ` Finn Arne Gangstad

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=20100607085947.GA3924@pvv.org \
    --to=finnag@pvv.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=grubba@roxen.com \
    --cc=jrnieder@gmail.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 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).