diff mbox series

[(Apple,Git),01/13] .gitignore: Remove *.s as it matches *.S on case insensitive filesystem

Message ID 20190129193818.8645-2-jeremyhu@apple.com (mailing list archive)
State New, archived
Headers show
Series Differences between git-2.20.1 and Apple Git-116 | expand

Commit Message

Jeremy Sequoia Jan. 29, 2019, 7:38 p.m. UTC
This was causing problems with ppc/sha1ppc.S

Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
---
 .gitignore | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Eric Sunshine Jan. 30, 2019, 11:33 a.m. UTC | #1
On Tue, Jan 29, 2019 at 3:11 PM Jeremy Huddleston Sequoia
<jeremyhu@apple.com> wrote:
> This was causing problems with ppc/sha1ppc.S

What problems, exactly?

> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
> ---
> diff --git a/.gitignore b/.gitignore
> @@ -195,7 +195,7 @@
> -*.[aos]
> +*.[ao]
Jeremy Sequoia Jan. 30, 2019, 11:37 a.m. UTC | #2
> On Jan 30, 2019, at 03:33, Eric Sunshine <sunshine@sunshineco.com> wrote:
> 
> On Tue, Jan 29, 2019 at 3:11 PM Jeremy Huddleston Sequoia
> <jeremyhu@apple.com> wrote:
>> This was causing problems with ppc/sha1ppc.S
> 
> What problems, exactly?

The file is ignored, but it shouldn't be.

> 
>> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
>> ---
>> diff --git a/.gitignore b/.gitignore
>> @@ -195,7 +195,7 @@
>> -*.[aos]
>> +*.[ao]
Eric Sunshine Jan. 30, 2019, 12:29 p.m. UTC | #3
On Wed, Jan 30, 2019 at 6:37 AM Jeremy Huddleston Sequoia
<jeremyhu@apple.com> wrote:
> > On Jan 30, 2019, at 03:33, Eric Sunshine <sunshine@sunshineco.com> wrote:
> > On Tue, Jan 29, 2019 at 3:11 PM Jeremy Huddleston Sequoia
> > <jeremyhu@apple.com> wrote:
> >> This was causing problems with ppc/sha1ppc.S
> >
> > What problems, exactly?
>
> The file is ignored, but it shouldn't be.

But what problem are you experiencing, exactly? .gitignore rules do
not impact tracked files such as ppc/sha1ppc.S, even if the name
matches an ignore-rule, so it's not clear what problem you're trying
to solve.
Eric Sunshine Jan. 30, 2019, 12:32 p.m. UTC | #4
On Wed, Jan 30, 2019 at 7:29 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Wed, Jan 30, 2019 at 6:37 AM Jeremy Huddleston Sequoia
> <jeremyhu@apple.com> wrote:
> > > On Jan 30, 2019, at 03:33, Eric Sunshine <sunshine@sunshineco.com> wrote:
> > > On Tue, Jan 29, 2019 at 3:11 PM Jeremy Huddleston Sequoia
> > > <jeremyhu@apple.com> wrote:
> > >> This was causing problems with ppc/sha1ppc.S
> > >
> > > What problems, exactly?
> >
> > The file is ignored, but it shouldn't be.
>
> But what problem are you experiencing, exactly? .gitignore rules do
> not impact tracked files such as ppc/sha1ppc.S, even if the name
> matches an ignore-rule, so it's not clear what problem you're trying
> to solve.

I'm guessing that this has something to do with HFS+ being
case-insensitive yet case-preserving, but an actual explanation of the
misbehavior experienced would be helpful.
Johannes Schindelin Jan. 30, 2019, 12:42 p.m. UTC | #5
Hi Jeremy,

On Wed, 30 Jan 2019, Jeremy Huddleston Sequoia wrote:

> > On Jan 30, 2019, at 03:33, Eric Sunshine <sunshine@sunshineco.com> wrote:
> > 
> > On Tue, Jan 29, 2019 at 3:11 PM Jeremy Huddleston Sequoia
> > <jeremyhu@apple.com> wrote:
> >> This was causing problems with ppc/sha1ppc.S
> > 
> > What problems, exactly?
> 
> The file is ignored, but it shouldn't be.

As somebody who sometimes (pretty rarely, but definitely more than once a
year) generates the assembler files to have a deeper look, I really
understand why *.s is ignored, and I think it should stay ignored.

What you probably want instead is

	# Accommodate for case-insensitive filesystems where *.s would catch
	!ppc/sha1ppc.S

after the `*.[aos]` line.

Ciao,
Johannes

> 
> > 
> >> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
> >> ---
> >> diff --git a/.gitignore b/.gitignore
> >> @@ -195,7 +195,7 @@
> >> -*.[aos]
> >> +*.[ao]
> 
>
Junio C Hamano Jan. 30, 2019, 4:47 p.m. UTC | #6
Jeremy Huddleston Sequoia <jeremyhu@apple.com> writes:

> This was causing problems with ppc/sha1ppc.S

"causing problems"?  How?  The source file is already tracked so
"git add" to record updates to it would work just fine, and "git
status" would report "modified" for it, with or without "*.S" in the
.gitignore file, no?

    To emulate, I just added ".*S" to .gitignore and modified
    ppc/sha1ppc.S to double check whta happens.

We still have the build rule for %.s in our Makefile, so ignoring
".s" is still the right thing to do to make sure "git add dir/"
won't add "dir/frotz.s" by accident.  I sense the downside of doing
so outweighs whatever benefit it would have on case incapable
filesystems (and it is unclear what problem this change is trying to
work around).

> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
> ---
>  .gitignore | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/.gitignore b/.gitignore
> index 0d77ea5894..a5db584576 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -195,7 +195,7 @@
>  *.deb
>  /git.spec
>  *.exe
> -*.[aos]
> +*.[ao]
>  *.py[co]
>  .depend/
>  *.gcda
Jeremy Sequoia Jan. 30, 2019, 7:13 p.m. UTC | #7
> On Jan 30, 2019, at 04:42, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> 
> Hi Jeremy,
> 
> On Wed, 30 Jan 2019, Jeremy Huddleston Sequoia wrote:
> 
>>> On Jan 30, 2019, at 03:33, Eric Sunshine <sunshine@sunshineco.com> wrote:
>>> 
>>> On Tue, Jan 29, 2019 at 3:11 PM Jeremy Huddleston Sequoia
>>> <jeremyhu@apple.com> wrote:
>>>> This was causing problems with ppc/sha1ppc.S
>>> 
>>> What problems, exactly?
>> 
>> The file is ignored, but it shouldn't be.
> 
> As somebody who sometimes (pretty rarely, but definitely more than once a
> year) generates the assembler files to have a deeper look, I really
> understand why *.s is ignored, and I think it should stay ignored.
> 
> What you probably want instead is
> 
> 	# Accommodate for case-insensitive filesystems where *.s would catch
> 	!ppc/sha1ppc.S
> 
> after the `*.[aos]` line.

Thanks for the suggestion.  I didn't know that was possible with .gitignore.  That's a much better solution.  I was expecting this to just stay in our series as upstreamable, but I'm glad you pointed this out.  I got to learn something new and offload a patch.  Thanks!

I'll now be able include this in my followup series of patches that I think can be upstreamed! =)

> Ciao,
> Johannes
> 
>> 
>>> 
>>>> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
>>>> ---
>>>> diff --git a/.gitignore b/.gitignore
>>>> @@ -195,7 +195,7 @@
>>>> -*.[aos]
>>>> +*.[ao]
>> 
>>
Junio C Hamano Jan. 31, 2019, 5:57 p.m. UTC | #8
Jeremy Huddleston Sequoia <jeremyhu@apple.com> writes:

>> What you probably want instead is
>> 
>> 	# Accommodate for case-insensitive filesystems where *.s would catch
>> 	!ppc/sha1ppc.S
>> 
>> after the `*.[aos]` line.
>
> Thanks for the suggestion.  I didn't know that was possible with
> .gitignore.  That's a much better solution.

I still do not see what problem you need a "solution" for in the
first place---I saw a few comments asking it in the thread, but saw
no answer.  ppc/sha1ppc.S is already tracked, so any modification
you make in the working tree can be added to the index with "git
add" and "git status" would report when you have modification to
that file in the working tree, without any such extra entry in
.gitignore, no?
Jeremy Sequoia Jan. 31, 2019, 6:17 p.m. UTC | #9
Sent from my iPhone...

> On Jan 31, 2019, at 09:57, Junio C Hamano <gitster@pobox.com> wrote:
> 
> Jeremy Huddleston Sequoia <jeremyhu@apple.com> writes:
> 
>>> What you probably want instead is
>>> 
>>>    # Accommodate for case-insensitive filesystems where *.s would catch
>>>    !ppc/sha1ppc.S
>>> 
>>> after the `*.[aos]` line.
>> 
>> Thanks for the suggestion.  I didn't know that was possible with
>> .gitignore.  That's a much better solution.
> 
> I still do not see what problem you need a "solution" for in the
> first place---I saw a few comments asking it in the thread, but saw
> no answer.  ppc/sha1ppc.S is already tracked, so any modification
> you make in the working tree can be added to the index with "git
> add" and "git status" would report when you have modification to
> that file in the working tree, without any such extra entry in
> .gitignore, no?

I originally saw this because the .gitignore was present in the tarball, and we would only update to new versions by pulling in the newer tarball.  This resulted in *.S not being included in our repo.

When I switched to having upstream be a subtree instead of an extracted tarball, I noticed this difference when comparing the source code difference between the tarball based approach and the git subtree approach.

This would have implications for anyone doing something similar or to anyone intending to add new assembly files to the tree (since they wouldn’t show up in status or get added with add -A),
Eric Sunshine Jan. 31, 2019, 7:48 p.m. UTC | #10
On Thu, Jan 31, 2019 at 1:17 PM Jeremy Sequoia <jeremyhu@apple.com> wrote:
> > On Jan 31, 2019, at 09:57, Junio C Hamano <gitster@pobox.com> wrote:
> > I still do not see what problem you need a "solution" for in the
> > first place---I saw a few comments asking it in the thread, but saw
> > no answer.  ppc/sha1ppc.S is already tracked, so any modification
> > you make in the working tree can be added to the index with "git
> > add" and "git status" would report when you have modification to
> > that file in the working tree, without any such extra entry in
> > .gitignore, no?
>
> This would have implications for anyone doing something similar or
> to anyone intending to add new assembly files to the tree (since
> they wouldn’t show up in status or get added with add -A),

This nugget finally gives readers an idea of the sort of issue this
patch wants to "fix", which happens to be related to HFS+ being
case-insensitive. As noted upstream, though, files which are already
tracked, such as ppc/sha1ppc.S, are not subject to .gitignore, so
Dscho's proposed modification to .gitignore:

    ...
    *.[aos]
    !ppc/sha1ppc.S
    ...

doesn't actually help. Moreover, this .gitignore change doesn't at all
help the case you describe about new assembly files not being noticed
by "git status" or "git add -A" since any new files won't be named
"ppc/sha1ppc.S".

As Junio said upstream, .gitignore ignoring "*.s" files is the right
thing to do for this project and, as adding new .S files is so rare,
it seems unlikely that a patch changing .gitignore to accommodate the
above use-case for case-insensitive filesystems would make sense to
the project as a whole.
diff mbox series

Patch

diff --git a/.gitignore b/.gitignore
index 0d77ea5894..a5db584576 100644
--- a/.gitignore
+++ b/.gitignore
@@ -195,7 +195,7 @@ 
 *.deb
 /git.spec
 *.exe
-*.[aos]
+*.[ao]
 *.py[co]
 .depend/
 *.gcda