{"thread":{"id":"52107","subject":"[Outreachy][PATCH] abspath: reconcile `dir_exists()` and `is_directory()`","startedAt":"2019-10-24T09:27:56Z","lastAt":"2019-10-26T18:42:48Z","messageCount":15,"participants":["Miriam Rubio","SZEDER Gábor","Jeff King","Emily Shaffer","Miriam R.","Junio C Hamano","Christian Couder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"384752","messageId":"20191024092745.97035-1-mirucam@gmail.com","threadId":"52107","inReplyTo":null,"subject":"[Outreachy][PATCH] abspath: reconcile `dir_exists()` and `is_directory()`","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2019-10-24T09:27:45Z","receivedAt":"2019-10-24T09:27:56Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"The dir_exists() function in builtin/clone.c is marked as static, so\nnobody can use it outside builtin/clone.c.\n\nThere is also is_directory() which obviously tries to do the very same, but it uses a name that few developers will think of when they see file_exists() and look for the equivalent function to see whether a given directory exists.\n\nLet's reconcile these functions by renaming is_directory() to dir_exists() and using it also in builtin/clone.c.\n\nSigned-off-by: Miriam Rubio <mirucam@gmail.com>\n---\n abspath.c                   |  2 +-\n builtin/am.c                |  2 +-\n builtin/clone.c             |  6 ------\n builtin/mv.c                |  2 +-\n builtin/rebase.c            | 10 +++++-----\n builtin/submodule--helper.c |  4 ++--\n builtin/worktree.c          |  6 +++---\n cache.h                     |  2 +-\n compat/mingw.c              |  2 +-\n daemon.c                    |  2 +-\n diff-no-index.c             |  4 ++--\n dir.c                       |  2 +-\n gettext.c                   |  2 +-\n rerere.c                    |  2 +-\n sequencer.c                 |  2 +-\n sha1-file.c                 |  8 ++++----\n submodule.c                 |  4 ++--\n trace2/tr2_dst.c            |  2 +-\n worktree.c                  |  2 +-\n 19 files changed, 30 insertions(+), 36 deletions(-)\n\ndiff --git a/abspath.c b/abspath.c\nindex 9857985329..13bd92eca5 100644\n--- a/abspath.c\n+++ b/abspath.c\n@@ -5,7 +5,7 @@\n  * symlink to a directory, we do not want to say it is a directory when\n  * dealing with tracked content in the working tree.\n  */\n-int is_directory(const char *path)\n+int dir_exists(const char *path)\n {\n \tstruct stat st;\n \treturn (!stat(path, &st) && S_ISDIR(st.st_mode));\ndiff --git a/builtin/am.c b/builtin/am.c\nindex 8181c2aef3..f872125fc7 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -576,7 +576,7 @@ static int detect_patch_format(const char **paths)\n \t/*\n \t * We default to mbox format if input is from stdin and for directories\n \t */\n-\tif (!*paths || !strcmp(*paths, \"-\") || is_directory(*paths))\n+\tif (!*paths || !strcmp(*paths, \"-\") || dir_exists(*paths))\n \t\treturn PATCH_FORMAT_MBOX;\n \n \t/*\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex c46ee29f0a..f89938bf94 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -899,12 +899,6 @@ static void dissociate_from_references(void)\n \tfree(alternates);\n }\n \n-static int dir_exists(const char *path)\n-{\n-\tstruct stat sb;\n-\treturn !stat(path, &sb);\n-}\n-\n int cmd_clone(int argc, const char **argv, const char *prefix)\n {\n \tint is_bundle = 0, is_local;\ndiff --git a/builtin/mv.c b/builtin/mv.c\nindex be15ba7044..194e1618a0 100644\n--- a/builtin/mv.c\n+++ b/builtin/mv.c\n@@ -152,7 +152,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t * \"git mv directory no-such-dir/\".\n \t */\n \tflags = KEEP_TRAILING_SLASH;\n-\tif (argc == 1 && is_directory(argv[0]) && !is_directory(argv[1]))\n+\tif (argc == 1 && dir_exists(argv[0]) && !dir_exists(argv[1]))\n \t\tflags = 0;\n \tdest_path = internal_prefix_pathspec(prefix, argv + argc, 1, flags);\n \tsubmodule_gitfile = xcalloc(argc, sizeof(char *));\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 4a20582e72..c66cdf729b 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -275,7 +275,7 @@ static int init_basic_state(struct replay_opts *opts, const char *head_name,\n {\n \tFILE *interactive;\n \n-\tif (!is_directory(merge_dir()) && mkdir_in_gitdir(merge_dir()))\n+\tif (!dir_exists(merge_dir()) && mkdir_in_gitdir(merge_dir()))\n \t\treturn error_errno(_(\"could not create temporary %s\"), merge_dir());\n \n \tdelete_reflog(\"REBASE_HEAD\");\n@@ -1068,7 +1068,7 @@ static int run_am(struct rebase_options *opts)\n \t\treturn move_to_original_branch(opts);\n \t}\n \n-\tif (is_directory(opts->state_dir))\n+\tif (dir_exists(opts->state_dir))\n \t\trebase_write_basic_state(opts);\n \n \treturn status;\n@@ -1529,13 +1529,13 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \tif(file_exists(buf.buf))\n \t\tdie(_(\"It looks like 'git am' is in progress. Cannot rebase.\"));\n \n-\tif (is_directory(apply_dir())) {\n+\tif (dir_exists(apply_dir())) {\n \t\toptions.type = REBASE_AM;\n \t\toptions.state_dir = apply_dir();\n-\t} else if (is_directory(merge_dir())) {\n+\t} else if (dir_exists(merge_dir())) {\n \t\tstrbuf_reset(&buf);\n \t\tstrbuf_addf(&buf, \"%s/rewritten\", merge_dir());\n-\t\tif (is_directory(buf.buf)) {\n+\t\tif (dir_exists(buf.buf)) {\n \t\t\toptions.type = REBASE_PRESERVE_MERGES;\n \t\t\toptions.flags |= REBASE_INTERACTIVE_EXPLICIT;\n \t\t} else {\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 2c2395a620..8df36e06b4 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1096,7 +1096,7 @@ static void deinit_submodule(const char *path, const char *prefix,\n \tdisplaypath = get_submodule_displaypath(path, prefix);\n \n \t/* remove the submodule work tree (unless the user already did it) */\n-\tif (is_directory(path)) {\n+\tif (dir_exists(path)) {\n \t\tstruct strbuf sb_rm = STRBUF_INIT;\n \t\tconst char *format;\n \n@@ -1105,7 +1105,7 @@ static void deinit_submodule(const char *path, const char *prefix,\n \t\t * NEEDSWORK: instead of dying, automatically call\n \t\t * absorbgitdirs and (possibly) warn.\n \t\t */\n-\t\tif (is_directory(sub_git_dir))\n+\t\tif (dir_exists(sub_git_dir))\n \t\t\tdie(_(\"Submodule work tree '%s' contains a .git \"\n \t\t\t      \"directory (use 'rm -rf' if you really want \"\n \t\t\t      \"to remove it including all of its history)\"),\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 4de44f579a..a69a1e5612 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -75,7 +75,7 @@ static int prune_worktree(const char *id, struct strbuf *reason)\n \tsize_t len;\n \tssize_t read_result;\n \n-\tif (!is_directory(git_path(\"worktrees/%s\", id))) {\n+\tif (!dir_exists(git_path(\"worktrees/%s\", id))) {\n \t\tstrbuf_addf(reason, _(\"Removing worktrees/%s: not a valid directory\"), id);\n \t\treturn 1;\n \t}\n@@ -738,7 +738,7 @@ static void validate_no_submodules(const struct worktree *wt)\n \tstruct strbuf path = STRBUF_INIT;\n \tint i, found_submodules = 0;\n \n-\tif (is_directory(worktree_git_path(wt, \"modules\"))) {\n+\tif (dir_exists(worktree_git_path(wt, \"modules\"))) {\n \t\t/*\n \t\t * There could be false positives, e.g. the \"modules\"\n \t\t * directory exists but is empty. But it's a rare case and\n@@ -799,7 +799,7 @@ static int move_worktree(int ac, const char **av, const char *prefix)\n \t\tdie(_(\"'%s' is not a working tree\"), av[0]);\n \tif (is_main_worktree(wt))\n \t\tdie(_(\"'%s' is a main working tree\"), av[0]);\n-\tif (is_directory(dst.buf)) {\n+\tif (dir_exists(dst.buf)) {\n \t\tconst char *sep = find_last_dir_sep(wt->path);\n \n \t\tif (!sep)\ndiff --git a/cache.h b/cache.h\nindex 04cabaac11..596de2db38 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1274,7 +1274,7 @@ static inline int is_absolute_path(const char *path)\n {\n \treturn is_dir_sep(path[0]) || has_dos_drive_prefix(path);\n }\n-int is_directory(const char *);\n+int dir_exists(const char *);\n char *strbuf_realpath(struct strbuf *resolved, const char *path,\n \t\t      int die_on_error);\n const char *real_path(const char *path);\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 6b765d936c..9e391a2a74 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -2352,7 +2352,7 @@ static void setup_windows_environment(void)\n \t\t\tstrbuf_addstr(&buf, tmp);\n \t\t\tif ((tmp = getenv(\"HOMEPATH\"))) {\n \t\t\t\tstrbuf_addstr(&buf, tmp);\n-\t\t\t\tif (is_directory(buf.buf))\n+\t\t\t\tif (dir_exists(buf.buf))\n \t\t\t\t\tsetenv(\"HOME\", buf.buf, 1);\n \t\t\t\telse\n \t\t\t\t\ttmp = NULL; /* use $USERPROFILE */\ndiff --git a/daemon.c b/daemon.c\nindex 9d2e0d20ef..050c1ffacf 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -1455,7 +1455,7 @@ int cmd_main(int argc, const char **argv)\n \tif (strict_paths && (!ok_paths || !*ok_paths))\n \t\tdie(\"option --strict-paths requires a whitelist\");\n \n-\tif (base_path && !is_directory(base_path))\n+\tif (base_path && !dir_exists(base_path))\n \t\tdie(\"base-path '%s' does not exist or is not a directory\",\n \t\t    base_path);\n \ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex 7814eabfe0..7f6e17fc76 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -221,8 +221,8 @@ static void fixup_paths(const char **path, struct strbuf *replacement)\n \tif (path[0] == file_from_standard_input ||\n \t    path[1] == file_from_standard_input)\n \t\treturn;\n-\tisdir0 = is_directory(path[0]);\n-\tisdir1 = is_directory(path[1]);\n+\tisdir0 = dir_exists(path[0]);\n+\tisdir1 = dir_exists(path[1]);\n \tif (isdir0 == isdir1)\n \t\treturn;\n \tif (isdir0) {\ndiff --git a/dir.c b/dir.c\nindex 61f559f980..fd22bc1866 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -2100,7 +2100,7 @@ static int treat_leading_path(struct dir_struct *dir,\n \t\t\tbaselen = cp - path;\n \t\tstrbuf_setlen(&sb, 0);\n \t\tstrbuf_add(&sb, path, baselen);\n-\t\tif (!is_directory(sb.buf))\n+\t\tif (!dir_exists(sb.buf))\n \t\t\tbreak;\n \t\tif (simplify_away(sb.buf, sb.len, pathspec))\n \t\t\tbreak;\ndiff --git a/gettext.c b/gettext.c\nindex 35d2c1218d..c02f6675fa 100644\n--- a/gettext.c\n+++ b/gettext.c\n@@ -183,7 +183,7 @@ void git_setup_gettext(void)\n \n \tuse_gettext_poison(); /* getenv() reentrancy paranoia */\n \n-\tif (!is_directory(podir)) {\n+\tif (!dir_exists(podir)) {\n \t\tfree(p);\n \t\treturn;\n \t}\ndiff --git a/rerere.c b/rerere.c\nindex 3e51fdfe58..1d6a4b8df2 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -873,7 +873,7 @@ static int is_rerere_enabled(void)\n \tif (!rerere_enabled)\n \t\treturn 0;\n \n-\trr_cache_exists = is_directory(git_path_rr_cache());\n+\trr_cache_exists = dir_exists(git_path_rr_cache());\n \tif (rerere_enabled < 0)\n \t\treturn rr_cache_exists;\n \ndiff --git a/sequencer.c b/sequencer.c\nindex 9d5964fd81..ee27b7b1cd 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2813,7 +2813,7 @@ int sequencer_skip(struct repository *r, struct replay_opts *opts)\n \n \tif (skip_single_pick())\n \t\treturn error(_(\"failed to skip the commit\"));\n-\tif (!is_directory(git_path_seq_dir()))\n+\tif (!dir_exists(git_path_seq_dir()))\n \t\treturn 0;\n \n \treturn sequencer_continue(r, opts);\ndiff --git a/sha1-file.c b/sha1-file.c\nindex 188de57634..7fcf89d431 100644\n--- a/sha1-file.c\n+++ b/sha1-file.c\n@@ -448,7 +448,7 @@ static int alt_odb_usable(struct raw_object_store *o,\n \tstruct object_directory *odb;\n \n \t/* Detect cases where alternate disappeared */\n-\tif (!is_directory(path->buf)) {\n+\tif (!dir_exists(path->buf)) {\n \t\terror(_(\"object directory %s does not exist; \"\n \t\t\t\"check .git/objects/info/alternates\"),\n \t\t      path->buf);\n@@ -699,11 +699,11 @@ char *compute_alternate_path(const char *path, struct strbuf *err)\n \t\tref_git = xstrdup(repo);\n \t}\n \n-\tif (!repo && is_directory(mkpath(\"%s/.git/objects\", ref_git))) {\n+\tif (!repo && dir_exists(mkpath(\"%s/.git/objects\", ref_git))) {\n \t\tchar *ref_git_git = mkpathdup(\"%s/.git\", ref_git);\n \t\tfree(ref_git);\n \t\tref_git = ref_git_git;\n-\t} else if (!is_directory(mkpath(\"%s/objects\", ref_git))) {\n+\t} else if (!dir_exists(mkpath(\"%s/objects\", ref_git))) {\n \t\tstruct strbuf sb = STRBUF_INIT;\n \t\tseen_error = 1;\n \t\tif (get_common_dir(&sb, ref_git)) {\n@@ -821,7 +821,7 @@ static int refs_from_alternate_cb(struct object_directory *e,\n \n \t/* Is this a git repository with refs? */\n \tstrbuf_addstr(&path, \"/refs\");\n-\tif (!is_directory(path.buf))\n+\tif (!dir_exists(path.buf))\n \t\tgoto out;\n \tstrbuf_setlen(&path, base_len);\n \ndiff --git a/submodule.c b/submodule.c\nindex 0f199c5137..870f35cd56 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -174,7 +174,7 @@ int add_submodule_odb(const char *path)\n \tret = strbuf_git_path_submodule(&objects_directory, path, \"objects/\");\n \tif (ret)\n \t\tgoto done;\n-\tif (!is_directory(objects_directory.buf)) {\n+\tif (!dir_exists(objects_directory.buf)) {\n \t\tret = -1;\n \t\tgoto done;\n \t}\n@@ -1647,7 +1647,7 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked)\n \tif (!git_dir)\n \t\tgit_dir = buf.buf;\n \tif (!is_git_directory(git_dir)) {\n-\t\tif (is_directory(git_dir))\n+\t\tif (dir_exists(git_dir))\n \t\t\tdie(_(\"'%s' not recognized as a git repository\"), git_dir);\n \t\tstrbuf_release(&buf);\n \t\t/* The submodule is not checked out, so it is not modified */\ndiff --git a/trace2/tr2_dst.c b/trace2/tr2_dst.c\nindex ae052a07fe..fe0b4d94ff 100644\n--- a/trace2/tr2_dst.c\n+++ b/trace2/tr2_dst.c\n@@ -337,7 +337,7 @@ int tr2_dst_get_trace_fd(struct tr2_dst *dst)\n \t}\n \n \tif (is_absolute_path(tgt_value)) {\n-\t\tif (is_directory(tgt_value))\n+\t\tif (dir_exists(tgt_value))\n \t\t\treturn tr2_dst_try_auto_path(dst, tgt_value);\n \t\telse\n \t\t\treturn tr2_dst_try_path(dst, tgt_value);\ndiff --git a/worktree.c b/worktree.c\nindex 5b4793caa3..e2d2e2dbf1 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -290,7 +290,7 @@ int validate_worktree(const struct worktree *wt, struct strbuf *errmsg,\n \tstrbuf_addf(&wt_path, \"%s/.git\", wt->path);\n \n \tif (is_main_worktree(wt)) {\n-\t\tif (is_directory(wt_path.buf)) {\n+\t\tif (dir_exists(wt_path.buf)) {\n \t\t\tret = 0;\n \t\t\tgoto done;\n \t\t}\n-- \n2.21.0 (Apple Git-122)\n\n"},{"id":"384763","messageId":"20191024114148.GK4348@szeder.dev","threadId":"52107","inReplyTo":"20191024092745.97035-1-mirucam@gmail.com","subject":"Re: [Outreachy][PATCH] abspath: reconcile `dir_exists()` and `is_directory()`","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-10-24T11:41:48Z","receivedAt":"2019-10-24T11:41:57Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Thu, Oct 24, 2019 at 11:27:45AM +0200, Miriam Rubio wrote:\n> The dir_exists() function in builtin/clone.c is marked as static, so\n> nobody can use it outside builtin/clone.c.\n> \n> There is also is_directory() which obviously tries to do the very same, but it uses a name that few developers will think of when they see file_exists() and look for the equivalent function to see whether a given directory exists.\n> \n> Let's reconcile these functions by renaming is_directory() to dir_exists() and using it also in builtin/clone.c.\n\nPlease wrap the proposed log message at about 70 characters width;\nthat way it will look much better in 'git log' in a standard 80 char\nwide terminal.\n\nI think this is a cleanup worth doing, but...\n\n> diff --git a/abspath.c b/abspath.c\n> index 9857985329..13bd92eca5 100644\n> --- a/abspath.c\n> +++ b/abspath.c\n> @@ -5,7 +5,7 @@\n>   * symlink to a directory, we do not want to say it is a directory when\n>   * dealing with tracked content in the working tree.\n>   */\n> -int is_directory(const char *path)\n> +int dir_exists(const char *path)\n>  {\n>  \tstruct stat st;\n>  \treturn (!stat(path, &st) && S_ISDIR(st.st_mode));\n\nNote the '&& S_ISDIR(st.st_mode)', making sure that the given path is\nin fact a directory.  Good.\n\n> diff --git a/builtin/clone.c b/builtin/clone.c\n> index c46ee29f0a..f89938bf94 100644\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -899,12 +899,6 @@ static void dissociate_from_references(void)\n>  \tfree(alternates);\n>  }\n>  \n> -static int dir_exists(const char *path)\n> -{\n> -\tstruct stat sb;\n> -\treturn !stat(path, &sb);\n\nBut look at this, it only checks that the given path exists, but it\ncould be a regular file or any other kind of path other than a\ndirectory as well!\n\nSo this function clearly doesn't do what it's name suggests.  That's\nbad.\n\nUnfortunately, it gets worse: some of its callsites in\n'builtin/clone.c' do expect it to check the existence of _any_ path,\nnot just a directory.\n\nThe first callsite is:\n\n    dest_exists = dir_exists(dir);\n    if (dest_exists && !is_empty_dir(dir))\n            die(_(\"destination path '%s' already exists and is not \"\n                    \"an empty directory.\"), dir);\n\nI think this actually means path_exists(): if a file, or any other\nkind of path with the given name were to exist, then we should die()\nshowing this error message, but after changing dir_exists() to make\nsure that the path is indeed a directory we won't:\n\n  # create a 'git' _file_\n  $ >git\n  # current git master:\n  $ git clone https://github.com/git/git\n  fatal: destination path 'git' already exists and is not an empty directory.\n  # with this patch:\n  $ ~/src/git/git clone https://github.com/git/git\n  fatal: could not create work tree dir 'git': File exists\n\nSo the command still fails, which is good, but with a different error\nmessage.  The test suite doesn't catch this, because the test case\nlooking at this scenario ('clone to an existing path' in\n'./t5601-clone.sh') only checks that 'git clone' fails, but it doesn't\ncheck whether it failed with the right error message.\n\nNow, that other error message comes after a failed mkdir() call later\non which should have created the work tree.  So it begs the question\nwhat would happen when a file is in the way of a bare clone:\n\n  $ >git.git\n  $ git clone --bare https://github.com/git/git\n  fatal: destination path 'git.git' already exists and is not an empty directory.\n  $ ~/src/git/git clone --bare https://github.com/git/git\n  Cloning into bare repository 'git.git'...\n  fatal: invalid gitfile format: /home/szeder/src/git/tmp/git.git\n\nThen the next callsite looks like it meant path_exists() as well, but\nI didn't try to make it fail or show different behavior:\n\n    work_tree = getenv(\"GIT_WORK_TREE\");\n    if (work_tree && dir_exists(work_tree))\n            die(_(\"working tree '%s' already exists.\"), work_tree);\n\nAnd there is a third callsite, but I'm not sure what it is about, and,\nconsequently, what is really meant with dir_exists() here:\n\n    if (real_git_dir) {\n            if (dir_exists(real_git_dir))\n                    junk_git_dir_flags |= REMOVE_DIR_KEEP_TOPLEVEL;\n\n\n"},{"id":"384782","messageId":"20191024181344.GD12892@sigill.intra.peff.net","threadId":"52107","inReplyTo":"20191024114148.GK4348@szeder.dev","subject":"Re: [Outreachy][PATCH] abspath: reconcile `dir_exists()` and `is_directory()`","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-10-24T18:13:45Z","receivedAt":"2019-10-24T18:13:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 24, 2019 at 01:41:48PM +0200, SZEDER Gábor wrote:\n\n> > diff --git a/builtin/clone.c b/builtin/clone.c\n> > index c46ee29f0a..f89938bf94 100644\n> > --- a/builtin/clone.c\n> > +++ b/builtin/clone.c\n> > @@ -899,12 +899,6 @@ static void dissociate_from_references(void)\n> >  \tfree(alternates);\n> >  }\n> >  \n> > -static int dir_exists(const char *path)\n> > -{\n> > -\tstruct stat sb;\n> > -\treturn !stat(path, &sb);\n> \n> But look at this, it only checks that the given path exists, but it\n> could be a regular file or any other kind of path other than a\n> directory as well!\n> \n> So this function clearly doesn't do what it's name suggests.  That's\n> bad.\n> \n> Unfortunately, it gets worse: some of its callsites in\n> 'builtin/clone.c' do expect it to check the existence of _any_ path,\n> not just a directory.\n\nYes, that's the reason for the funny name (and the fact that it was\nnever re-factored to use is_directory() in the first place). There's\nsome more discussion in:\n\n  https://public-inbox.org/git/xmqqbmi9dw55.fsf@gitster.mtv.corp.google.com/\n\nand its subthread.\n\n-Peff\n"},{"id":"384791","messageId":"20191024204500.GG9323@google.com","threadId":"52107","inReplyTo":"20191024181344.GD12892@sigill.intra.peff.net","subject":"Re: [Outreachy][PATCH] abspath: reconcile `dir_exists()` and `is_directory()`","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-10-24T20:45:00Z","receivedAt":"2019-10-24T20:45:07Z","isPatch":true,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Thu, Oct 24, 2019 at 02:13:45PM -0400, Jeff King wrote:\n> On Thu, Oct 24, 2019 at 01:41:48PM +0200, SZEDER Gábor wrote:\n> \n> > > diff --git a/builtin/clone.c b/builtin/clone.c\n> > > index c46ee29f0a..f89938bf94 100644\n> > > --- a/builtin/clone.c\n> > > +++ b/builtin/clone.c\n> > > @@ -899,12 +899,6 @@ static void dissociate_from_references(void)\n> > >  \tfree(alternates);\n> > >  }\n> > >  \n> > > -static int dir_exists(const char *path)\n> > > -{\n> > > -\tstruct stat sb;\n> > > -\treturn !stat(path, &sb);\n> > \n> > But look at this, it only checks that the given path exists, but it\n> > could be a regular file or any other kind of path other than a\n> > directory as well!\n> > \n> > So this function clearly doesn't do what it's name suggests.  That's\n> > bad.\n> > \n> > Unfortunately, it gets worse: some of its callsites in\n> > 'builtin/clone.c' do expect it to check the existence of _any_ path,\n> > not just a directory.\n> \n> Yes, that's the reason for the funny name (and the fact that it was\n> never re-factored to use is_directory() in the first place). There's\n> some more discussion in:\n> \n>   https://public-inbox.org/git/xmqqbmi9dw55.fsf@gitster.mtv.corp.google.com/\n> \n> and its subthread.\n\nHm. Then, is the solution to use dir_exists() for \"a directory exists\nhere\" and also add path_exists() for \"literally anything exists here\"?\nThat seems like it's still a pretty minor change. It'd be nice to\nun-stick our Outreachy applicant :)\n\n - Emily\n"},{"id":"384793","messageId":"20191024205100.GB30715@sigill.intra.peff.net","threadId":"52107","inReplyTo":"20191024204500.GG9323@google.com","subject":"Re: [Outreachy][PATCH] abspath: reconcile `dir_exists()` and `is_directory()`","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-10-24T20:51:00Z","receivedAt":"2019-10-24T20:51:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 24, 2019 at 01:45:00PM -0700, Emily Shaffer wrote:\n\n> > Yes, that's the reason for the funny name (and the fact that it was\n> > never re-factored to use is_directory() in the first place). There's\n> > some more discussion in:\n> > \n> >   https://public-inbox.org/git/xmqqbmi9dw55.fsf@gitster.mtv.corp.google.com/\n> > \n> > and its subthread.\n> \n> Hm. Then, is the solution to use dir_exists() for \"a directory exists\n> here\" and also add path_exists() for \"literally anything exists here\"?\n> That seems like it's still a pretty minor change. It'd be nice to\n> un-stick our Outreachy applicant :)\n\nYeah, I think one path forward could be:\n\n  - add path_exists(); this will work the same as file_exists(), but is\n    a better name. Keep file_exists() for now, but put a comment that\n    new calls should use path_exists().\n\n  - use path_exists() in builtin/clone.c, ditching its custom\n    dir_exists()\n\n  - (optional) start converting file_exists() calls to path_exists(),\n    after confirming what each call wants (just files, or any path)\n\n    I one really does want to check for a regular file, then we'd need\n    to figure out what the \"does this regular file exist\" function is\n    called. I have a suspicion that there won't be any such callers, so\n    we can punt on it until then.\n\n-Peff\n"},{"id":"384794","messageId":"CAN7CjDAavVx5n3jwBiwHOt51pNA8u4=Yrc+BWh5aJez0t38Lhg@mail.gmail.com","threadId":"52107","inReplyTo":"20191024114148.GK4348@szeder.dev","subject":"Re: [Outreachy][PATCH] abspath: reconcile `dir_exists()` and `is_directory()`","fromName":"Miriam R.","fromEmail":"mirucam@gmail.com","sentAt":"2019-10-24T20:57:50Z","receivedAt":"2019-10-24T20:58:03Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"El jue., 24 oct. 2019 a las 13:41, SZEDER Gábor\n(<szeder.dev@gmail.com>) escribió:\n>\n> On Thu, Oct 24, 2019 at 11:27:45AM +0200, Miriam Rubio wrote:\n> > The dir_exists() function in builtin/clone.c is marked as static, so\n> > nobody can use it outside builtin/clone.c.\n> >\n> > There is also is_directory() which obviously tries to do the very same, but it uses a name that few developers will think of when they see file_exists() and look for the equivalent function to see whether a given directory exists.\n> >\n> > Let's reconcile these functions by renaming is_directory() to dir_exists() and using it also in builtin/clone.c.\n>\n> Please wrap the proposed log message at about 70 characters width;\n> that way it will look much better in 'git log' in a standard 80 char\n> wide terminal.\n>\n> I think this is a cleanup worth doing, but...\n\nThanks for the guidelines!\n\n>\n> > diff --git a/abspath.c b/abspath.c\n> > index 9857985329..13bd92eca5 100644\n> > --- a/abspath.c\n> > +++ b/abspath.c\n> > @@ -5,7 +5,7 @@\n> >   * symlink to a directory, we do not want to say it is a directory when\n> >   * dealing with tracked content in the working tree.\n> >   */\n> > -int is_directory(const char *path)\n> > +int dir_exists(const char *path)\n> >  {\n> >       struct stat st;\n> >       return (!stat(path, &st) && S_ISDIR(st.st_mode));\n>\n> Note the '&& S_ISDIR(st.st_mode)', making sure that the given path is\n> in fact a directory.  Good.\n>\n> > diff --git a/builtin/clone.c b/builtin/clone.c\n> > index c46ee29f0a..f89938bf94 100644\n> > --- a/builtin/clone.c\n> > +++ b/builtin/clone.c\n> > @@ -899,12 +899,6 @@ static void dissociate_from_references(void)\n> >       free(alternates);\n> >  }\n> >\n> > -static int dir_exists(const char *path)\n> > -{\n> > -     struct stat sb;\n> > -     return !stat(path, &sb);\n>\n> But look at this, it only checks that the given path exists, but it\n> could be a regular file or any other kind of path other than a\n> directory as well!\n>\n> So this function clearly doesn't do what it's name suggests.  That's\n> bad.\n>\n> Unfortunately, it gets worse: some of its callsites in\n> 'builtin/clone.c' do expect it to check the existence of _any_ path,\n> not just a directory.\n>\n> The first callsite is:\n>\n>     dest_exists = dir_exists(dir);\n>     if (dest_exists && !is_empty_dir(dir))\n>             die(_(\"destination path '%s' already exists and is not \"\n>                     \"an empty directory.\"), dir);\n>\n> I think this actually means path_exists(): if a file, or any other\n> kind of path with the given name were to exist, then we should die()\n> showing this error message, but after changing dir_exists() to make\n> sure that the path is indeed a directory we won't:\n>\n>   # create a 'git' _file_\n>   $ >git\n>   # current git master:\n>   $ git clone https://github.com/git/git\n>   fatal: destination path 'git' already exists and is not an empty directory.\n>   # with this patch:\n>   $ ~/src/git/git clone https://github.com/git/git\n>   fatal: could not create work tree dir 'git': File exists\n>\n> So the command still fails, which is good, but with a different error\n> message.  The test suite doesn't catch this, because the test case\n> looking at this scenario ('clone to an existing path' in\n> './t5601-clone.sh') only checks that 'git clone' fails, but it doesn't\n> check whether it failed with the right error message.\n>\n> Now, that other error message comes after a failed mkdir() call later\n> on which should have created the work tree.  So it begs the question\n> what would happen when a file is in the way of a bare clone:\n>\n>   $ >git.git\n>   $ git clone --bare https://github.com/git/git\n>   fatal: destination path 'git.git' already exists and is not an empty directory.\n>   $ ~/src/git/git clone --bare https://github.com/git/git\n>   Cloning into bare repository 'git.git'...\n>   fatal: invalid gitfile format: /home/szeder/src/git/tmp/git.git\n>\n> Then the next callsite looks like it meant path_exists() as well, but\n> I didn't try to make it fail or show different behavior:\n>\n>     work_tree = getenv(\"GIT_WORK_TREE\");\n>     if (work_tree && dir_exists(work_tree))\n>             die(_(\"working tree '%s' already exists.\"), work_tree);\n>\n> And there is a third callsite, but I'm not sure what it is about, and,\n> consequently, what is really meant with dir_exists() here:\n>\n>     if (real_git_dir) {\n>             if (dir_exists(real_git_dir))\n>                     junk_git_dir_flags |= REMOVE_DIR_KEEP_TOPLEVEL;\n>\n>\n\nHere is my proposal:\n\n- Rename is_directory() to dir_exists(), as it is the equivalent to\nfile_exists().\n- Rename dir_exists() to path_exists() so it describes its real\nbehavior, and either leave it as static inside clone.c or extract it\nto a common header such as that containing dir_exists().\n\nWhat do you think?\n"},{"id":"384824","messageId":"xmqqr231eedi.fsf@gitster-ct.c.googlers.com","threadId":"52107","inReplyTo":"20191024205100.GB30715@sigill.intra.peff.net","subject":"Re: [Outreachy][PATCH] abspath: reconcile `dir_exists()` and `is_directory()`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-25T02:40:57Z","receivedAt":"2019-10-25T02:41:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Yeah, I think one path forward could be:\n>\n>   - add path_exists(); this will work the same as file_exists(), but is\n>     a better name. Keep file_exists() for now, but put a comment that\n>     new calls should use path_exists().\n>\n>   - use path_exists() in builtin/clone.c, ditching its custom\n>     dir_exists()\n\nBoth are of immediate value ;-)\n\n>   - (optional) start converting file_exists() calls to path_exists(),\n>     after confirming what each call wants (just files, or any path)\n\nThat is of lessor urgency but the result has a good documentation\nvalue.\n\n"},{"id":"384825","messageId":"xmqqmudpee57.fsf@gitster-ct.c.googlers.com","threadId":"52107","inReplyTo":"20191024114148.GK4348@szeder.dev","subject":"Re: [Outreachy][PATCH] abspath: reconcile `dir_exists()` and `is_directory()`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-25T02:45:56Z","receivedAt":"2019-10-25T02:46:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor <szeder.dev@gmail.com> writes:\n\n> The first callsite is:\n>\n>     dest_exists = dir_exists(dir);\n>     if (dest_exists && !is_empty_dir(dir))\n>             die(_(\"destination path '%s' already exists and is not \"\n>                     \"an empty directory.\"), dir);\n\nYup.  The primary/original reason why the helper exists is to see if\nwe can create directory there, so the function is asking \"is this\npath taken?\"  It might have been cleaner to do all of these without\nusing such a helper function and instead take the safer approach to\n\"try mkdir, and if we fail, complian\", which is race-free.  But the\nabove is what we have now X-<.\n\n"},{"id":"384849","messageId":"CAN7CjDB9mRTNRKRoE8XfLz4in5gV6pxrKrqcjLPfthDHaf20nA@mail.gmail.com","threadId":"52107","inReplyTo":"xmqqmudpee57.fsf@gitster-ct.c.googlers.com","subject":"Re: [Outreachy][PATCH] abspath: reconcile `dir_exists()` and `is_directory()`","fromName":"Miriam R.","fromEmail":"mirucam@gmail.com","sentAt":"2019-10-25T08:59:42Z","receivedAt":"2019-10-25T08:59:55Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"Ok, then after discussion, finally the issue tasks would be:\n\n- Add path_exists() that will work same as file_exists(), keeping for\nnow the latter.\n- Use path_exists() instead of dir_exists() in builtin/clone.c.\n\nAnd also:\n- Rename is_directory() to dir_exists(), as it is the equivalent to\npath_exists()/file_exists(), isn't it?\n\nBest,\nMiriam\n\n\nEl vie., 25 oct. 2019 a las 4:46, Junio C Hamano (<gitster@pobox.com>) escribió:\n>\n> SZEDER Gábor <szeder.dev@gmail.com> writes:\n>\n> > The first callsite is:\n> >\n> >     dest_exists = dir_exists(dir);\n> >     if (dest_exists && !is_empty_dir(dir))\n> >             die(_(\"destination path '%s' already exists and is not \"\n> >                     \"an empty directory.\"), dir);\n>\n> Yup.  The primary/original reason why the helper exists is to see if\n> we can create directory there, so the function is asking \"is this\n> path taken?\"  It might have been cleaner to do all of these without\n> using such a helper function and instead take the safer approach to\n> \"try mkdir, and if we fail, complian\", which is race-free.  But the\n> above is what we have now X-<.\n>\n"},{"id":"384850","messageId":"xmqqzhhpb1nx.fsf@gitster-ct.c.googlers.com","threadId":"52107","inReplyTo":"CAN7CjDB9mRTNRKRoE8XfLz4in5gV6pxrKrqcjLPfthDHaf20nA@mail.gmail.com","subject":"Re: [Outreachy][PATCH] abspath: reconcile `dir_exists()` and `is_directory()`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-25T09:43:46Z","receivedAt":"2019-10-25T09:43:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Miriam R.\" <mirucam@gmail.com> writes:\n\n> Ok, then after discussion, finally the issue tasks would be:\n>\n> - Add path_exists() that will work same as file_exists(), keeping for\n> now the latter.\n> - Use path_exists() instead of dir_exists() in builtin/clone.c.\n\nSounds about right.\n\n> And also:\n> - Rename is_directory() to dir_exists(), as it is the equivalent to\n> path_exists()/file_exists(), isn't it?\n\nI wouldn't go there in the same series, if I were doing it.  I'd\nexpect that such a patch would be more noisy than it is worth if\ndone in a single step.  In order to avoid becoming a hindrance to\nother topics in flight, an ideal series to do so would support the\nsame functionality with both old and new names, convert code that\nuse the old name to use the new name, possibly in multiple patches\nto avoid unnecessary textual conflicts (i.e. some of these patches\nmade to areas that are seeing active development will be discarded\nand need to be retried later when the area is more quiet) and then\nfinally the function wither the old name gets removed.\n\nYou would not want to mix the first two bullet points that are\nrelatively isolated with such a long transition.\n"},{"id":"384867","messageId":"CAP8UFD1_vnjApobt+aN3M12g8mLqOZJGyvr4oqqTax5=cmLhsg@mail.gmail.com","threadId":"52107","inReplyTo":"xmqqzhhpb1nx.fsf@gitster-ct.c.googlers.com","subject":"Re: [Outreachy][PATCH] abspath: reconcile `dir_exists()` and `is_directory()`","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2019-10-25T14:47:50Z","receivedAt":"2019-10-25T14:48:05Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Oct 25, 2019 at 11:43 AM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> \"Miriam R.\" <mirucam@gmail.com> writes:\n>\n> > Ok, then after discussion, finally the issue tasks would be:\n> >\n> > - Add path_exists() that will work same as file_exists(), keeping for\n> > now the latter.\n> > - Use path_exists() instead of dir_exists() in builtin/clone.c.\n>\n> Sounds about right.\n>\n> > And also:\n> > - Rename is_directory() to dir_exists(), as it is the equivalent to\n> > path_exists()/file_exists(), isn't it?\n>\n> I wouldn't go there in the same series, if I were doing it.  I'd\n> expect that such a patch would be more noisy than it is worth if\n> done in a single step.  In order to avoid becoming a hindrance to\n> other topics in flight, an ideal series to do so would support the\n> same functionality with both old and new names, convert code that\n> use the old name to use the new name, possibly in multiple patches\n> to avoid unnecessary textual conflicts (i.e. some of these patches\n> made to areas that are seeing active development will be discarded\n> and need to be retried later when the area is more quiet) and then\n> finally the function wither the old name gets removed.\n>\n> You would not want to mix the first two bullet points that are\n> relatively isolated with such a long transition.\n\nYeah, and for a micro-project it is more than enough if you only work\non the first two bullet points.\n"},{"id":"384872","messageId":"CAN7CjDC169rv8p9ZJcoLMeisXh7eMVcE4_-bpz8XFiYUsWAakQ@mail.gmail.com","threadId":"52107","inReplyTo":"CAP8UFD1_vnjApobt+aN3M12g8mLqOZJGyvr4oqqTax5=cmLhsg@mail.gmail.com","subject":"Re: [Outreachy][PATCH] abspath: reconcile `dir_exists()` and `is_directory()`","fromName":"Miriam R.","fromEmail":"mirucam@gmail.com","sentAt":"2019-10-25T15:23:21Z","receivedAt":"2019-10-25T15:23:34Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"Ok! Thanks to everyone.\n\nBest,\nMiriam\n\nEl vie., 25 oct. 2019 a las 16:48, Christian Couder\n(<christian.couder@gmail.com>) escribió:\n>\n> On Fri, Oct 25, 2019 at 11:43 AM Junio C Hamano <gitster@pobox.com> wrote:\n> >\n> > \"Miriam R.\" <mirucam@gmail.com> writes:\n> >\n> > > Ok, then after discussion, finally the issue tasks would be:\n> > >\n> > > - Add path_exists() that will work same as file_exists(), keeping for\n> > > now the latter.\n> > > - Use path_exists() instead of dir_exists() in builtin/clone.c.\n> >\n> > Sounds about right.\n> >\n> > > And also:\n> > > - Rename is_directory() to dir_exists(), as it is the equivalent to\n> > > path_exists()/file_exists(), isn't it?\n> >\n> > I wouldn't go there in the same series, if I were doing it.  I'd\n> > expect that such a patch would be more noisy than it is worth if\n> > done in a single step.  In order to avoid becoming a hindrance to\n> > other topics in flight, an ideal series to do so would support the\n> > same functionality with both old and new names, convert code that\n> > use the old name to use the new name, possibly in multiple patches\n> > to avoid unnecessary textual conflicts (i.e. some of these patches\n> > made to areas that are seeing active development will be discarded\n> > and need to be retried later when the area is more quiet) and then\n> > finally the function wither the old name gets removed.\n> >\n> > You would not want to mix the first two bullet points that are\n> > relatively isolated with such a long transition.\n>\n> Yeah, and for a micro-project it is more than enough if you only work\n> on the first two bullet points.\n"},{"id":"384913","messageId":"CAN7CjDDr0vDBDi+RKA0BMTHSaVQofc=GTCEuy1mAOaQmVqhJXA@mail.gmail.com","threadId":"52107","inReplyTo":"CAN7CjDC169rv8p9ZJcoLMeisXh7eMVcE4_-bpz8XFiYUsWAakQ@mail.gmail.com","subject":"Re: [Outreachy][PATCH] abspath: reconcile `dir_exists()` and `is_directory()`","fromName":"Miriam R.","fromEmail":"mirucam@gmail.com","sentAt":"2019-10-26T15:30:51Z","receivedAt":"2019-10-26T15:31:05Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"Dear all,\nthere is already a static function called path_exists() in archive.c\nso project does not compile.\n\nMaybe we could change the name of this static function and its\nreference in archive.c like archive_path_exists() for example, or some\nother you find more suitable.\n\nBest,\nMiriam\n\nEl vie., 25 oct. 2019 a las 17:23, Miriam R. (<mirucam@gmail.com>) escribió:\n>\n> Ok! Thanks to everyone.\n>\n> Best,\n> Miriam\n>\n> El vie., 25 oct. 2019 a las 16:48, Christian Couder\n> (<christian.couder@gmail.com>) escribió:\n> >\n> > On Fri, Oct 25, 2019 at 11:43 AM Junio C Hamano <gitster@pobox.com> wrote:\n> > >\n> > > \"Miriam R.\" <mirucam@gmail.com> writes:\n> > >\n> > > > Ok, then after discussion, finally the issue tasks would be:\n> > > >\n> > > > - Add path_exists() that will work same as file_exists(), keeping for\n> > > > now the latter.\n> > > > - Use path_exists() instead of dir_exists() in builtin/clone.c.\n> > >\n> > > Sounds about right.\n> > >\n> > > > And also:\n> > > > - Rename is_directory() to dir_exists(), as it is the equivalent to\n> > > > path_exists()/file_exists(), isn't it?\n> > >\n> > > I wouldn't go there in the same series, if I were doing it.  I'd\n> > > expect that such a patch would be more noisy than it is worth if\n> > > done in a single step.  In order to avoid becoming a hindrance to\n> > > other topics in flight, an ideal series to do so would support the\n> > > same functionality with both old and new names, convert code that\n> > > use the old name to use the new name, possibly in multiple patches\n> > > to avoid unnecessary textual conflicts (i.e. some of these patches\n> > > made to areas that are seeing active development will be discarded\n> > > and need to be retried later when the area is more quiet) and then\n> > > finally the function wither the old name gets removed.\n> > >\n> > > You would not want to mix the first two bullet points that are\n> > > relatively isolated with such a long transition.\n> >\n> > Yeah, and for a micro-project it is more than enough if you only work\n> > on the first two bullet points.\n"},{"id":"384915","messageId":"CAP8UFD3pOL27VEO_42mXP_mfM689hRWLBU3KR0zJLgQKrX9ZPA@mail.gmail.com","threadId":"52107","inReplyTo":"CAN7CjDDr0vDBDi+RKA0BMTHSaVQofc=GTCEuy1mAOaQmVqhJXA@mail.gmail.com","subject":"Re: [Outreachy][PATCH] abspath: reconcile `dir_exists()` and `is_directory()`","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2019-10-26T18:05:34Z","receivedAt":"2019-10-26T18:05:48Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Hi Miriam,\n\nOn Sat, Oct 26, 2019 at 5:31 PM Miriam R. <mirucam@gmail.com> wrote:\n>\n> Dear all,\n> there is already a static function called path_exists() in archive.c\n> so project does not compile.\n>\n> Maybe we could change the name of this static function and its\n> reference in archive.c like archive_path_exists() for example, or some\n> other you find more suitable.\n\nYeah, I think renaming the function archive_path_exists() in a\npreparatory patch would be a good way to move forward on this.\n\nBest,\nChristian.\n"},{"id":"384917","messageId":"CAN7CjDCS3MNgdFd8NBkNEt3E+hfNgWiKizpEkQ1xjPbEkNrC3g@mail.gmail.com","threadId":"52107","inReplyTo":"CAP8UFD3pOL27VEO_42mXP_mfM689hRWLBU3KR0zJLgQKrX9ZPA@mail.gmail.com","subject":"Re: [Outreachy][PATCH] abspath: reconcile `dir_exists()` and `is_directory()`","fromName":"Miriam R.","fromEmail":"mirucam@gmail.com","sentAt":"2019-10-26T18:42:34Z","receivedAt":"2019-10-26T18:42:48Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"El sáb., 26 oct. 2019 a las 20:05, Christian Couder\n(<christian.couder@gmail.com>) escribió:\n>\n> Hi Miriam,\n>\n> On Sat, Oct 26, 2019 at 5:31 PM Miriam R. <mirucam@gmail.com> wrote:\n> >\n> > Dear all,\n> > there is already a static function called path_exists() in archive.c\n> > so project does not compile.\n> >\n> > Maybe we could change the name of this static function and its\n> > reference in archive.c like archive_path_exists() for example, or some\n> > other you find more suitable.\n>\n> Yeah, I think renaming the function archive_path_exists() in a\n> preparatory patch would be a good way to move forward on this.\n>\n> Best,\n> Christian.\n\nGreat!\nThank you, Christian.\n"}]}