Re: [PATCH/RFC] add: listen to --ignore-errors for submodule-errors

From: Erik Faye-Lund <kusmabite@gmail.com>
Date: Jun 26, 2012, 08:25 UTC
Message-ID: <CABPQNSZmQOiwOo_dV43BidT3TRp63mOJeJWe4pK-9z0ryB6_QQ@mail.gmail.com>
In-Reply-To: <7vfw9ieqvh.fsf@alter.siamese.dyndns.org>

On Tue, Jun 26, 2012 at 4:41 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 27 quoted lines

> Erik Faye-Lund <kusmabite@gmail.com> writes:
>
>> I recently tried to do the following in the msysGit-repo:
>>
>>  $ git add --ignore-errors -- */.gitignore
>>  fatal: Path 'src/git-cheetah/.gitignore' is in submodule 'src/git-cheetah'
>>
>> I was a bit puzzled by this; I explicitly specified --ignore-errors
>> because I did not want to be stopped due to src/git-cheetah/.gitignore
>> being located in a submodule.
>
> If I recall correctly, originally --ignore-errors was added was by
> those who (arguably misguidedly) wanted to randomly run "git add"
> that can potentially race with ongoing working tree updates
> (i.e. think of a poor-man's unreliable snapshotting filesystem), to
> which "git add" will notice that the working tree file it was asked
> to index changed while it was reading and error out.  Also on some
> systems, "git add" on files that are currently open may not be able
> to read from them, which would also cause a run-time error.  The
> kind of errors the option was meant to ignore were "these paths are
> perfectly OK to add, but for some reason, adding them fails at this
> moment, and for the purpose of poor-man's unreliable snapshot, it is
> OK not to pick the exact current state up, as we will pick it up the
> next round", not your kind of request that will lead to an error of
> the "adding this path will break the structural integrity of the
> repository and git should error out" kind.
>

Right. I guess my confusion was due to the documentation wording "If some files could not be added *because of errors indexing them*, do not abort the operation". It's not obvious to me that "because if errors indexing them" means what you describe above.

Show 10 quoted lines

>> The documentation seems to suggest that this is what is supposed to
>> happen, and this seems like the most likely behavior that the user
>> wanted. After all, there's no good reason submodules are special
>> in this regard, no?
>
> How does "git add .git" or "git add .git/config" behave with your
> patch applied?  It is exactly the same kind of error that breaks the
> structural integrity of the repository as adding src/cheetah/.gitignore
> to the top-level project repository, and there is no good reason to
> special case submodules, either.

Both "git add .git" and "git add --ignore-errors .git" returned silently without error even before my patch.

"git add .git/config" outputs "error: Invalid path '.git/config' error: unable to add .git/config to index fatal: adding files failed"

...and "git add --ignore-errors .git/config" outputs "error: Invalid path '.git/config' error: unable to add .git/config to index"

...this is both before and after my patch. I didn't touch that part of the code-path, so this already treats these errors as non-fatal when --ignore-errors are specified.

I simply found a new code-path where errors weren't ignored, but there might of course be more. However, the lack of a "fatal: adding files failed" for files inside .git seems to suggest to me that even though the intent might be what you describe above, that is not actually what the code does.

Of course, silently ignoring "git add .git" seems odd to me.