Re: [PATCH v9 1/9] git-clean: refactor git-clean into two phases

2013/5/15 Junio C Hamano [off-list ref]:

quoted

@@ -242,11 +287,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
                              continue; /* Yup, this one exists unmerged */
              }

-             /*
-              * we might have removed this as part of earlier
-              * recursive directory removal, so lstat() here could
-              * fail with ENOENT.
-              */
              if (lstat(ent->name, &st))
                      continue;

I am guessing that the reason why you removed the comment is because
during this phase there is no way we "might have removed".  But if
that is the case, does it still make sense to run lstat() and ignore
errors from the call?

Run lstat() here is necessary, because we need to check whether
ent->name points to a file or a directory. If ent points to a directory,
only add to del_list when user provides '-x' option to git-clean.

                if (S_ISDIR(st.st_mode)) {
                        if (remove_directories || (matches ==
MATCHED_EXACTLY)) {
                                rel = path_relative(ent->name, prefix);
                                string_list_append(&del_list, rel);
                        }
                } ...


When start to do cleaning, there is a duplicate lstat() call, and the removed
comments above are moved here.

                /*
                 * we might have removed this as part of earlier
                 * recursive directory removal, so lstat() here could
                 * fail with ENOENT.
                 */
                if (lstat(abs_path.buf, &st))
                        continue;

                if (S_ISDIR(st.st_mode)) {
                        if (remove_dirs(&abs_path, prefix, rm_flags,
dry_run, quiet, &gone))
                                errors++;
                        if (gone && !quiet) {
                                qname =
quote_path_relative(item->string, -1, &buf, NULL);
                                printf(dry_run ? _(msg_would_remove) :
_(msg_remove), qname);
                        }
                }

But I am not clear how "earlier recursive directory removal" could
make lstat() failure here. If it is not the case, maybe we can test a
directory by ent->name (ends with '/' or '\\' ?).

-- 
Jiang Xin