Dear Junio,
Thank you for reviewing my patch. I completely agree with you, that
add_to_index() should not be called with undefined data.
I will amend patch now with proposed changes.
Thank you!
Best regards,
Qusielle
On 28.10.2019 03:03, Junio C Hamano wrote:
"qusielle via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
From: qusielle <31454380+qusielle@xxxxxxxxxxxxxxxxxxxxxxxx>
"git add --ignore-errors" command fails immediately when lstat returns
an error, despite the ignore errors' flag is enabled.
...
diff --git a/read-cache.c b/read-cache.c
index 133f790fa4..67237ecd29 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -801,7 +801,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
int add_file_to_index(struct index_state *istate, const char *path, int flags)
{
struct stat st;
- if (lstat(path, &st))
+ if (lstat(path, &st) && !(flags & ADD_CACHE_IGNORE_ERRORS))
die_errno(_("unable to stat '%s'"), path);
return add_to_index(istate, path, &st, flags);
}
The only callers of this function that matter calls it and then
responds to an error return like so:
(in builtin/add.c::update_callback())
if (add_file_to_index(&the_index, path, data->flags)) {
if (!(data->flags & ADD_CACHE_IGNORE_ERRORS))
die(_("updating files failed"));
(in builtin/add.c::add_files(), where ignore_add_errors was used to
set the ADD_CACHE_IGNORE_ERRORS to flags in its caller)
if (add_file_to_index(&the_index, dir->entries[i]->name, flags)) {
if (!ignore_add_errors)
die(_("adding files failed"));
So you correctly identified what is the right place to fix. We
should not "die_errno()"; we should give the control back to the
caller instead.
But after a failed stat, the code with your patch still calls
add_to_index() using the now undefined stat data, which would
contaminate the in-core index with wrong data.
I think we should instead return without touching the index for the
path we had trouble lstat()ing.
IOW
if (lstat(path, &st)) {
if (flags & ADD_CACHE_IGNORE_ERRORS)
return -1;
else
die_errno(_("unable to ..."));
}
return add_to_index(...);
