Re: [PATCH 2/2] checkout: fix attribute handling in checkout all

12 messages, 6 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 2/2] checkout: fix attribute handling in checkout all

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:29

Junio C Hamano [off-list ref] writes:
Steffen Prohaska [off-list ref] writes:
...
quoted
This works only together with the commit

'attr: fix attribute handling if .gitattributes is involved'
While I think it is _one_ good approach to make things two-pass,
I do not know if this is enough.  A logic similar to this should
be made available to the codepath that switches branches,
shouldn't it?
Ok, let's step back a bit and I'll suggest an alternative
approach to your 1/2.  This would hopefully solve 2/2 without
any code change your patch 2/2 has.

I think this approach is very much in line with how the git
plumbing works, but you would need to know how the world is
designed to work in order to appreciate it fully.  Let's have a
few paragraphs to give the readers some background.

The work tree side of git is primarily about the index, and what
is on the work tree is more or less secondary.  At the lower
level, often we deliberately treat not having a working tree
file as equivalent to having an unmodified work tree file.  We
can apply the same principle to this "missing .gitattributes
file" case.

People who only know modern git may not be aware of this, but
you can apply patches and perform a merge in a work tree that
does not have any file checked out, as long as your index is
fully populated.  For example, you can do something like this:

    $ git clone -n git://.../git.git v.git
    $ cd v.git
    $ git update-ref --no-deref HEAD $(git rev-parse v1.5.3-rc4^0)
    $ git read-tree HEAD
    $ git apply --index patch.txt

You will have the files that are patched in the resulting work
tree, so that you can inspect the result.  If you like the
result, you can even make a commit in such a sparsely populated
tree:

    $ git commit

Of course, "git commit -a" and "git add -u" Porcelain options
are more recent inventions, and they would not work with such a
sparsely populated work tree.  But the above demonstration shows
that at the plumbing level the index is the king and the work
tree is secondary, and this is very much as designed.  The merge
operation has similar characteristics:

    $ git merge master

... will check out the paths that need file-level 3-way merge,
so that you can inspect the result, but what you will have is a
sparsely populated work tree, and this is as designed.

Currently, the attr_stack code reads only from the work tree
and work tree alone.  We could change it to:

 - If the directory on the work tree has .gitattributes, use it
   (this is what the current code does);

 - Otherwise if the index has .gitattributes at the
   corresponding path, use that instead.

This essentially treats not having .gitattributes files checked
out as equivalent to having these files checked out unmodified,
which is very much in line with how the world is designed to
work.

Re: [PATCH 2/2] checkout: fix attribute handling in checkout all

From: Marius Storm-Olsen <hidden>
Date: 2016-06-15 22:43:29

Junio C Hamano said the following on 13.08.2007 08:14:
Ok, let's step back a bit and I'll suggest an alternative
approach to your 1/2.  This would hopefully solve 2/2 without
any code change your patch 2/2 has.
(..snip..)
I think this approach is very much in line with how the git
plumbing works, but you would need to know how the world is
designed to work in order to appreciate it fully.  Let's have a
few paragraphs to give the readers some background.
(..snip..)
Currently, the attr_stack code reads only from the work tree
and work tree alone.  We could change it to:

 - If the directory on the work tree has .gitattributes, use it
   (this is what the current code does);

 - Otherwise if the index has .gitattributes at the
   corresponding path, use that instead.

This essentially treats not having .gitattributes files checked
out as equivalent to having these files checked out unmodified,
which is very much in line with how the world is designed to
work.
ACK! We really need this! :-)

In msysgit.git/etc/.gitattributes we have 'termcap -crlf', to avoid 
the termcaps being checked out with Windows EOL, if the user happens 
to have 'autocrlf = true'. However, when you checkout the working dir 
the first time it still has Windows EOL due to exactly this problem.

The above algorithm would alleviate this issue.

-- 
.marius

Re: [PATCH 2/2] checkout: fix attribute handling in checkout all

From: Steffen Prohaska <hidden>
Date: 2016-06-15 22:43:29

On Aug 13, 2007, at 8:14 AM, Junio C Hamano wrote:
Junio C Hamano [off-list ref] writes:
quoted
Steffen Prohaska [off-list ref] writes:
...
quoted
This works only together with the commit

'attr: fix attribute handling if .gitattributes is involved'
While I think it is _one_ good approach to make things two-pass,
I do not know if this is enough.  A logic similar to this should
be made available to the codepath that switches branches,
shouldn't it?
Ok, let's step back a bit and I'll suggest an alternative
approach to your 1/2.  This would hopefully solve 2/2 without
any code change your patch 2/2 has.
That would be great.

I think this approach is very much in line with how the git
plumbing works, but you would need to know how the world is
designed to work in order to appreciate it fully.  Let's have a
few paragraphs to give the readers some background.

The work tree side of git is primarily about the index, and what
is on the work tree is more or less secondary.  At the lower
level, often we deliberately treat not having a working tree
file as equivalent to having an unmodified work tree file.  We
can apply the same principle to this "missing .gitattributes
file" case.

People who only know modern git may not be aware of this, but
you can apply patches and perform a merge in a work tree that
does not have any file checked out, as long as your index is
fully populated.  For example, you can do something like this:

    $ git clone -n git://.../git.git v.git
    $ cd v.git
    $ git update-ref --no-deref HEAD $(git rev-parse v1.5.3-rc4^0)
    $ git read-tree HEAD
    $ git apply --index patch.txt

You will have the files that are patched in the resulting work
tree, so that you can inspect the result.  If you like the
result, you can even make a commit in such a sparsely populated
tree:

    $ git commit

Of course, "git commit -a" and "git add -u" Porcelain options
are more recent inventions, and they would not work with such a
sparsely populated work tree.  But the above demonstration shows
that at the plumbing level the index is the king and the work
tree is secondary, and this is very much as designed.  The merge
operation has similar characteristics:

    $ git merge master

... will check out the paths that need file-level 3-way merge,
so that you can inspect the result, but what you will have is a
sparsely populated work tree, and this is as designed.
Ah, merge ...

Currently, the attr_stack code reads only from the work tree
and work tree alone.  We could change it to:

 - If the directory on the work tree has .gitattributes, use it
   (this is what the current code does);

 - Otherwise if the index has .gitattributes at the
   corresponding path, use that instead.

This essentially treats not having .gitattributes files checked
out as equivalent to having these files checked out unmodified,
which is very much in line with how the world is designed to
work.
We may have conflicts in the .gitattributes file during a merge.
.gitattributes may be present in different stages, and with
conflict markers in the work tree.

Could we drop reading the file in the work tree completely?
.gitattributes would be a property of the index alone. To control
attributes you first need to add them to the index, before adding
the file that has attributes set in .gitattributes.

If we have .gitattributes in different stages, the right one
should be chosen to checkout corresponding files in the same stage.

	Steffen

Re: [PATCH 2/2] checkout: fix attribute handling in checkout all

From: Steffen Prohaska <hidden>
Date: 2016-06-15 22:43:29

On Aug 13, 2007, at 8:32 AM, Marius Storm-Olsen wrote:
Junio C Hamano said the following on 13.08.2007 08:14:
quoted
Ok, let's step back a bit and I'll suggest an alternative
approach to your 1/2.  This would hopefully solve 2/2 without
any code change your patch 2/2 has.
(..snip..)
quoted
I think this approach is very much in line with how the git
plumbing works, but you would need to know how the world is
designed to work in order to appreciate it fully.  Let's have a
few paragraphs to give the readers some background.
(..snip..)
quoted
Currently, the attr_stack code reads only from the work tree
and work tree alone.  We could change it to:
 - If the directory on the work tree has .gitattributes, use it
   (this is what the current code does);
 - Otherwise if the index has .gitattributes at the
   corresponding path, use that instead.
This essentially treats not having .gitattributes files checked
out as equivalent to having these files checked out unmodified,
which is very much in line with how the world is designed to
work.
ACK! We really need this! :-)

In msysgit.git/etc/.gitattributes we have 'termcap -crlf', to avoid  
the termcaps being checked out with Windows EOL, if the user  
happens to have 'autocrlf = true'. However, when you checkout the  
working dir the first time it still has Windows EOL due to exactly  
this problem.
And exactly this is where I recognized the issue.

msysgit devs,
We should really make autocrlf = true the default for us and fix all
problems that we'll encounter. There may be more tricky stuff ahead,
like merges, cherry-picks, ...

	Steffen

Re: [PATCH 2/2] checkout: fix attribute handling in checkout all

From: Marius Storm-Olsen <hidden>
Date: 2016-06-15 22:43:29

Steffen Prohaska said the following on 13.08.2007 08:50:
On Aug 13, 2007, at 8:32 AM, Marius Storm-Olsen wrote:
quoted
In msysgit.git/etc/.gitattributes we have 'termcap -crlf', to avoid  
the termcaps being checked out with Windows EOL, if the user  
happens to have 'autocrlf = true'. However, when you checkout the  
working dir the first time it still has Windows EOL due to exactly  
this problem.
And exactly this is where I recognized the issue.

msysgit devs,
We should really make autocrlf = true the default for us and fix
all problems that we'll encounter. There may be more tricky stuff
ahead, like merges, cherry-picks, ...
I'm more leaning towards having the installer give you the option to 
choose what kind of line-endings you want Git to work with; just like 
the Cygwin installer.

-- 
.marius

Re: [PATCH 2/2] checkout: fix attribute handling in checkout all

From: David Kastrup <hidden>
Date: 2016-06-15 22:43:29

Junio C Hamano [off-list ref] writes:
Junio C Hamano [off-list ref] writes:

    $ git update-ref --no-deref HEAD $(git rev-parse v1.5.3-rc4^0)
Is there a fundamental difference to using

git-symbolic-ref HEAD $(git rev-parse v1.5.3-rc4^0)

here?

-- 
David Kastrup

Re: [PATCH 2/2] checkout: fix attribute handling in checkout all

From: Steffen Prohaska <hidden>
Date: 2016-06-15 22:43:29

On Aug 13, 2007, at 9:15 AM, Marius Storm-Olsen wrote:
Steffen Prohaska said the following on 13.08.2007 08:50:
quoted
On Aug 13, 2007, at 8:32 AM, Marius Storm-Olsen wrote:
quoted
In msysgit.git/etc/.gitattributes we have 'termcap -crlf', to  
avoid  the termcaps being checked out with Windows EOL, if the  
user  happens to have 'autocrlf = true'. However, when you  
checkout the  working dir the first time it still has Windows EOL  
due to exactly  this problem.
And exactly this is where I recognized the issue.
msysgit devs,
We should really make autocrlf = true the default for us and fix
all problems that we'll encounter. There may be more tricky stuff
ahead, like merges, cherry-picks, ...
I'm more leaning towards having the installer give you the option  
to choose what kind of line-endings you want Git to work with; just  
like the Cygwin installer.
Which is the root of much trouble with Cygwin. People now say,
git works perfectly in Cygwin but forget to mention that they
mean Cygwin A (in binmode) but not Cygwin B (in textmode).

Better choose the right default and work hard to make the
default choice work perfectly. I am strongly against an option
in the installer. An option _will_ cause confusion. Better give
people a hint how they can override the default for a single
user, or for a single repo. Then they recognize that they move
to a non-default configuration and hopefully think twice. And we
never need to talk about msysgit A vs. msysgit B, but only about
msysgit with repo specific or user specific options.

For me, the question comes down to the following: What would the
average Windows user (real Windows user, not Linux user who was
forced to work in Cygwin!) expect git to do with line endings?
The answer to this question should be the default.

	Steffen

Re: [PATCH 2/2] checkout: fix attribute handling in checkout all

From: Marius Storm-Olsen <hidden>
Date: 2016-06-15 22:43:29

Steffen Prohaska said the following on 13.08.2007 09:32:
On Aug 13, 2007, at 9:15 AM, Marius Storm-Olsen wrote:
quoted
Steffen Prohaska said the following on 13.08.2007 08:50:
quoted
We should really make autocrlf = true the default for us and
fix all problems that we'll encounter. There may be more tricky
stuff ahead, like merges, cherry-picks, ...
I'm more leaning towards having the installer give you the option
 to choose what kind of line-endings you want Git to work with;
just like the Cygwin installer.
Which is the root of much trouble with Cygwin. People now say, git
works perfectly in Cygwin but forget to mention that they mean
Cygwin A (in binmode) but not Cygwin B (in textmode).

Better choose the right default and work hard to make the default
choice work perfectly. I am strongly against an option in the
installer. An option _will_ cause confusion. Better give people a
hint how they can override the default for a single user, or for a
single repo. Then they recognize that they move to a non-default
configuration and hopefully think twice. And we never need to talk
about msysgit A vs. msysgit B, but only about msysgit with repo
specific or user specific options.

For me, the question comes down to the following: What would the 
average Windows user (real Windows user, not Linux user who was 
forced to work in Cygwin!) expect git to do with line endings? The
answer to this question should be the default.
If we were talking about a huge amount (real) Windows users I would 
agree with you. However, currently most of the users using Git on 
Windows are Unix users which for some reason have to work on Windows 
every now and then. And changing the default option to autocrlf=true 
would be stepping on their toes, which we probably don't want to do :-)

I'm a Windows developer myself, so I naturally have autocrlf=true in 
my global settings. I don't think having the option in the installer 
(together with other things, like setting the global username, and 
email for example) would be such a bad thing. The problem with the way 
the Cygwin installer presents it is that it doesn't explain the pros 
and cons of the two options; it just recommends Linux EOL, which leads 
to confusion with some Windows developers. If we properly explain the 
issue in the installer, and say we recommend Windows EOL for Windows 
developers, I think it's OK. It would in any case be better than the 
current state where you have no option, or stepping on all the current 
msysgit/mingw-git maintainers toes.

-- 
.marius

Re: [PATCH 2/2] checkout: fix attribute handling in checkout all

From: Steffen Prohaska <hidden>
Date: 2016-06-15 22:43:29

On Aug 13, 2007, at 10:39 AM, Marius Storm-Olsen wrote:
Steffen Prohaska said the following on 13.08.2007 09:32:
quoted
On Aug 13, 2007, at 9:15 AM, Marius Storm-Olsen wrote:
quoted
Steffen Prohaska said the following on 13.08.2007 08:50:
quoted
We should really make autocrlf = true the default for us and
fix all problems that we'll encounter. There may be more tricky
stuff ahead, like merges, cherry-picks, ...
I'm more leaning towards having the installer give you the option
 to choose what kind of line-endings you want Git to work with;
just like the Cygwin installer.
Which is the root of much trouble with Cygwin. People now say, git
works perfectly in Cygwin but forget to mention that they mean
Cygwin A (in binmode) but not Cygwin B (in textmode).
Better choose the right default and work hard to make the default
choice work perfectly. I am strongly against an option in the
installer. An option _will_ cause confusion. Better give people a
hint how they can override the default for a single user, or for a
single repo. Then they recognize that they move to a non-default
configuration and hopefully think twice. And we never need to talk
about msysgit A vs. msysgit B, but only about msysgit with repo
specific or user specific options.
For me, the question comes down to the following: What would the  
average Windows user (real Windows user, not Linux user who was  
forced to work in Cygwin!) expect git to do with line endings? The
answer to this question should be the default.
If we were talking about a huge amount (real) Windows users I would  
agree with you. However, currently most of the users using Git on  
Windows are Unix users which for some reason have to work on  
Windows every now and then. And changing the default option to  
autocrlf=true would be stepping on their toes, which we probably  
don't want to do :-)
My target audience of Git on Windows are Windows users and, frankly,
that is the only reasonable way to think about Windows. Why else should
I boot Windows, if I don't have real Windows users in mind? I mean,
Windows is not the superior platform to build Unix on top. The reason
to boot Windows is Windows itself, including its real users.

I'm a Windows developer myself, so I naturally have autocrlf=true  
in my global settings. I don't think having the option in the  
installer (together with other things, like setting the global  
username, and email for example) would be such a bad thing. The  
problem with the way the Cygwin installer presents it is that it  
doesn't explain the pros and cons of the two options; it just  
recommends Linux EOL, which leads to confusion with some Windows  
developers.
The problem is that Cygwin doesn't really support textmode. It
offers a choice, where there is no choice. After selecting textmode,
I still can install git. But git doesn't work.

If we properly explain the issue in the installer, and say we  
recommend Windows EOL for Windows developers, I think it's OK. It  
would in any case be better than the current state where you have  
no option, or stepping on all the current msysgit/mingw-git  
maintainers toes.
Maybe I don't fully understand what msysgit is about. I thought it would
be about real Windows support, which I think requires to accept what
Windows users expect to be the right thing: Windows EOL.

In the long run it will also be easier for us, because other Windows
tools expect Windows EOL. I'm pretty sure that git plays better on
Windows if it has Window EOL on by default.

	Steffen

Re: [PATCH 2/2] checkout: fix attribute handling in checkout all

From: Dmitry Kakurin <hidden>
Date: 2016-06-15 22:43:29

On 8/13/07, Steffen Prohaska [off-list ref] wrote:
My target audience of Git on Windows are Windows users and, frankly,
that is the only reasonable way to think about Windows. Why else should
I boot Windows, if I don't have real Windows users in mind? I mean,
Windows is not the superior platform to build Unix on top. The reason
to boot Windows is Windows itself, including its real users.
I agree with this approach.
Maybe I don't fully understand what msysgit is about. I thought it would
be about real Windows support, which I think requires to accept what
Windows users expect to be the right thing: Windows EOL.
msysgit is Build Environment for Git on Windows. It's purpose is to
facilitate Git development. This is not what end-user wants.
Another installer (WinGit) is targeting end users. It is just so
happens that msysgit is in better shape right now and more useful. But
long term it will not be the case.

Here is another consideration: let's say I've started a new git repo
under Windows with no autocrlf set. Then my repo will contain crlf
line endings.
Now let's say that someone else checks out this repo with
autocrlf=true. What would happen then? Will they get cr cr lf?
-- 
- Dmitry

git-update-ref bug? (was: [PATCH 2/2] checkout: fix attribute handling in checkout all)

From: David Kastrup <hidden>
Date: 2016-06-15 22:43:29

David Kastrup [off-list ref] writes:
Junio C Hamano [off-list ref] writes:
quoted
Junio C Hamano [off-list ref] writes:

    $ git update-ref --no-deref HEAD $(git rev-parse v1.5.3-rc4^0)
Is there a fundamental difference to using

git-symbolic-ref HEAD $(git rev-parse v1.5.3-rc4^0)

here?
Apart from the fact that the latter works, and the former doesn't
because "--no-deref" is actually ignored?

-- 
David Kastrup

Re: [PATCH 2/2] checkout: fix attribute handling in checkout all

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:29

Hi,

On Mon, 13 Aug 2007, Steffen Prohaska wrote:
Could we drop reading the file [.gitattributes] in the work tree 
completely?
NACK.

It is not good to hide things away from the working tree.  It is much 
easier to just edit a file than to edit it and put it into the index.

Ciao,
Dscho
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help