On Tue, Apr 8, 2025 at 3:37 AM Patrick Steinhardt <ps@xxxxxx> wrote:
>
> The `safe_create_leading_directories()` function and its relatives
How is mkdir_in_gitdir() a relative of safe_create_leading_directories()?
I assumed the relation was "called by", but there is no such
relationship. The rest of the patch looked fine, but I was puzzled
for a while trying to figure out what this relationship is.
> are
> located in "object-file.c", which is not a good fit as they provide
> generic functionality not related to objects at all. Move them into
> "dir.c".
Makes sense.
>
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> ---
> builtin/bugreport.c | 2 +-
> builtin/credential-cache--daemon.c | 2 +-
> builtin/diagnose.c | 2 +-
> builtin/fsck.c | 1 +
> builtin/gc.c | 2 +-
> builtin/init-db.c | 2 +-
> builtin/log.c | 2 +-
> commit-graph.c | 1 +
> dir.c | 107 ++++++++++++++++++++++++++++++++++++-
> dir.h | 35 ++++++++++++
> midx-write.c | 1 +
> object-file.c | 106 ------------------------------------
> object-file.h | 35 ------------
> 13 files changed, 150 insertions(+), 148 deletions(-)
>
> diff --git a/builtin/bugreport.c b/builtin/bugreport.c
> index 66d64bfd5ae..d07fa91c247 100644
> --- a/builtin/bugreport.c
> +++ b/builtin/bugreport.c
> @@ -1,6 +1,7 @@
> #define USE_THE_REPOSITORY_VARIABLE
> #include "builtin.h"
> #include "abspath.h"
> +#include "dir.h"
> #include "editor.h"
> #include "gettext.h"
> #include "parse-options.h"
> @@ -10,7 +11,6 @@
> #include "hook.h"
> #include "hook-list.h"
> #include "diagnose.h"
> -#include "object-file.h"
> #include "setup.h"
> #include "version.h"
>
> diff --git a/builtin/credential-cache--daemon.c b/builtin/credential-cache--daemon.c
> index e707618e743..80d29b4f5c0 100644
> --- a/builtin/credential-cache--daemon.c
> +++ b/builtin/credential-cache--daemon.c
> @@ -1,8 +1,8 @@
> #define USE_THE_REPOSITORY_VARIABLE
> #include "builtin.h"
> #include "abspath.h"
> +#include "dir.h"
> #include "gettext.h"
> -#include "object-file.h"
> #include "parse-options.h"
>
> #ifndef NO_UNIX_SOCKETS
> diff --git a/builtin/diagnose.c b/builtin/diagnose.c
> index 33c39bd5981..d5dadd6a48b 100644
> --- a/builtin/diagnose.c
> +++ b/builtin/diagnose.c
> @@ -2,8 +2,8 @@
>
> #include "builtin.h"
> #include "abspath.h"
> +#include "dir.h"
> #include "gettext.h"
> -#include "object-file.h"
> #include "parse-options.h"
> #include "diagnose.h"
>
> diff --git a/builtin/fsck.c b/builtin/fsck.c
> index 9c8a6d6a8df..32d40d8f9fc 100644
> --- a/builtin/fsck.c
> +++ b/builtin/fsck.c
> @@ -1,5 +1,6 @@
> #define USE_THE_REPOSITORY_VARIABLE
> #include "builtin.h"
> +#include "dir.h"
> #include "gettext.h"
> #include "hex.h"
> #include "config.h"
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 99431fd4674..b069629676c 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -16,6 +16,7 @@
> #include "builtin.h"
> #include "abspath.h"
> #include "date.h"
> +#include "dir.h"
> #include "environment.h"
> #include "hex.h"
> #include "config.h"
> @@ -28,7 +29,6 @@
> #include "commit.h"
> #include "commit-graph.h"
> #include "packfile.h"
> -#include "object-file.h"
> #include "object-store-ll.h"
> #include "pack.h"
> #include "pack-objects.h"
> diff --git a/builtin/init-db.c b/builtin/init-db.c
> index 196dccdd77a..39730c1b0ce 100644
> --- a/builtin/init-db.c
> +++ b/builtin/init-db.c
> @@ -6,9 +6,9 @@
> #define USE_THE_REPOSITORY_VARIABLE
> #include "builtin.h"
> #include "abspath.h"
> +#include "dir.h"
> #include "environment.h"
> #include "gettext.h"
> -#include "object-file.h"
> #include "parse-options.h"
> #include "path.h"
> #include "refs.h"
> diff --git a/builtin/log.c b/builtin/log.c
> index 0d4c579dad7..06ffaa93e86 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -10,11 +10,11 @@
> #include "builtin.h"
> #include "abspath.h"
> #include "config.h"
> +#include "dir.h"
> #include "environment.h"
> #include "gettext.h"
> #include "hex.h"
> #include "refs.h"
> -#include "object-file.h"
> #include "object-name.h"
> #include "object-store-ll.h"
> #include "pager.h"
> diff --git a/commit-graph.c b/commit-graph.c
> index 8286d5dda24..3fae20dc21b 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -4,6 +4,7 @@
> #include "git-compat-util.h"
> #include "config.h"
> #include "csum-file.h"
> +#include "dir.h"
> #include "gettext.h"
> #include "hex.h"
> #include "lockfile.h"
> diff --git a/dir.c b/dir.c
> index 28b0e03feb4..16ae3b5169d 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -17,7 +17,6 @@
> #include "environment.h"
> #include "gettext.h"
> #include "name-hash.h"
> -#include "object-file.h"
> #include "object-store-ll.h"
> #include "path.h"
> #include "refs.h"
> @@ -4132,3 +4131,109 @@ int path_match_flags(const char *const str, const enum path_match_flags flags)
> return is_xplatform_dir_sep(*p);
> BUG("unreachable");
> }
> +
> +int mkdir_in_gitdir(const char *path)
> +{
> + if (mkdir(path, 0777)) {
> + int saved_errno = errno;
> + struct stat st;
> + struct strbuf sb = STRBUF_INIT;
> +
> + if (errno != EEXIST)
> + return -1;
> + /*
> + * Are we looking at a path in a symlinked worktree
> + * whose original repository does not yet have it?
> + * e.g. .git/rr-cache pointing at its original
> + * repository in which the user hasn't performed any
> + * conflict resolution yet?
> + */
> + if (lstat(path, &st) || !S_ISLNK(st.st_mode) ||
> + strbuf_readlink(&sb, path, st.st_size) ||
> + !is_absolute_path(sb.buf) ||
> + mkdir(sb.buf, 0777)) {
> + strbuf_release(&sb);
> + errno = saved_errno;
> + return -1;
> + }
> + strbuf_release(&sb);
> + }
> + return adjust_shared_perm(the_repository, path);
> +}
> +
> +static enum scld_error safe_create_leading_directories_1(char *path, int share)
> +{
> + char *next_component = path + offset_1st_component(path);
> + enum scld_error ret = SCLD_OK;
> +
> + while (ret == SCLD_OK && next_component) {
> + struct stat st;
> + char *slash = next_component, slash_character;
> +
> + while (*slash && !is_dir_sep(*slash))
> + slash++;
> +
> + if (!*slash)
> + break;
> +
> + next_component = slash + 1;
> + while (is_dir_sep(*next_component))
> + next_component++;
> + if (!*next_component)
> + break;
> +
> + slash_character = *slash;
> + *slash = '\0';
> + if (!stat(path, &st)) {
> + /* path exists */
> + if (!S_ISDIR(st.st_mode)) {
> + errno = ENOTDIR;
> + ret = SCLD_EXISTS;
> + }
> + } else if (mkdir(path, 0777)) {
> + if (errno == EEXIST &&
> + !stat(path, &st) && S_ISDIR(st.st_mode))
> + ; /* somebody created it since we checked */
> + else if (errno == ENOENT)
> + /*
> + * Either mkdir() failed because
> + * somebody just pruned the containing
> + * directory, or stat() failed because
> + * the file that was in our way was
> + * just removed. Either way, inform
> + * the caller that it might be worth
> + * trying again:
> + */
> + ret = SCLD_VANISHED;
> + else
> + ret = SCLD_FAILED;
> + } else if (share && adjust_shared_perm(the_repository, path)) {
> + ret = SCLD_PERMS;
> + }
> + *slash = slash_character;
> + }
> + return ret;
> +}
> +
> +enum scld_error safe_create_leading_directories(char *path)
> +{
> + return safe_create_leading_directories_1(path, 1);
> +}
> +
> +enum scld_error safe_create_leading_directories_no_share(char *path)
> +{
> + return safe_create_leading_directories_1(path, 0);
> +}
> +
> +enum scld_error safe_create_leading_directories_const(const char *path)
> +{
> + int save_errno;
> + /* path points to cache entries, so xstrdup before messing with it */
> + char *buf = xstrdup(path);
> + enum scld_error result = safe_create_leading_directories(buf);
> +
> + save_errno = errno;
> + free(buf);
> + errno = save_errno;
> + return result;
> +}
> diff --git a/dir.h b/dir.h
> index d7e71aa8daa..02c1f9420b0 100644
> --- a/dir.h
> +++ b/dir.h
> @@ -676,4 +676,39 @@ static inline int starts_with_dot_dot_slash_native(const char *const path)
> return path_match_flags(path, what | PATH_MATCH_NATIVE);
> }
>
> +/*
> + * Create the directory containing the named path, using care to be
> + * somewhat safe against races. Return one of the scld_error values to
> + * indicate success/failure. On error, set errno to describe the
> + * problem.
> + *
> + * SCLD_VANISHED indicates that one of the ancestor directories of the
> + * path existed at one point during the function call and then
> + * suddenly vanished, probably because another process pruned the
> + * directory while we were working. To be robust against this kind of
> + * race, callers might want to try invoking the function again when it
> + * returns SCLD_VANISHED.
> + *
> + * safe_create_leading_directories() temporarily changes path while it
> + * is working but restores it before returning.
> + * safe_create_leading_directories_const() doesn't modify path, even
> + * temporarily. Both these variants adjust the permissions of the
> + * created directories to honor core.sharedRepository, so they are best
> + * suited for files inside the git dir. For working tree files, use
> + * safe_create_leading_directories_no_share() instead, as it ignores
> + * the core.sharedRepository setting.
> + */
> +enum scld_error {
> + SCLD_OK = 0,
> + SCLD_FAILED = -1,
> + SCLD_PERMS = -2,
> + SCLD_EXISTS = -3,
> + SCLD_VANISHED = -4
> +};
> +enum scld_error safe_create_leading_directories(char *path);
> +enum scld_error safe_create_leading_directories_const(const char *path);
> +enum scld_error safe_create_leading_directories_no_share(char *path);
> +
> +int mkdir_in_gitdir(const char *path);
> +
> #endif
> diff --git a/midx-write.c b/midx-write.c
> index a628ac24dcb..e01a867c583 100644
> --- a/midx-write.c
> +++ b/midx-write.c
> @@ -3,6 +3,7 @@
> #include "git-compat-util.h"
> #include "abspath.h"
> #include "config.h"
> +#include "dir.h"
> #include "hex.h"
> #include "lockfile.h"
> #include "packfile.h"
> diff --git a/object-file.c b/object-file.c
> index 772c311f188..23b2c8560be 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -91,112 +91,6 @@ static int get_conv_flags(unsigned flags)
> }
>
>
> -int mkdir_in_gitdir(const char *path)
> -{
> - if (mkdir(path, 0777)) {
> - int saved_errno = errno;
> - struct stat st;
> - struct strbuf sb = STRBUF_INIT;
> -
> - if (errno != EEXIST)
> - return -1;
> - /*
> - * Are we looking at a path in a symlinked worktree
> - * whose original repository does not yet have it?
> - * e.g. .git/rr-cache pointing at its original
> - * repository in which the user hasn't performed any
> - * conflict resolution yet?
> - */
> - if (lstat(path, &st) || !S_ISLNK(st.st_mode) ||
> - strbuf_readlink(&sb, path, st.st_size) ||
> - !is_absolute_path(sb.buf) ||
> - mkdir(sb.buf, 0777)) {
> - strbuf_release(&sb);
> - errno = saved_errno;
> - return -1;
> - }
> - strbuf_release(&sb);
> - }
> - return adjust_shared_perm(the_repository, path);
> -}
> -
> -static enum scld_error safe_create_leading_directories_1(char *path, int share)
> -{
> - char *next_component = path + offset_1st_component(path);
> - enum scld_error ret = SCLD_OK;
> -
> - while (ret == SCLD_OK && next_component) {
> - struct stat st;
> - char *slash = next_component, slash_character;
> -
> - while (*slash && !is_dir_sep(*slash))
> - slash++;
> -
> - if (!*slash)
> - break;
> -
> - next_component = slash + 1;
> - while (is_dir_sep(*next_component))
> - next_component++;
> - if (!*next_component)
> - break;
> -
> - slash_character = *slash;
> - *slash = '\0';
> - if (!stat(path, &st)) {
> - /* path exists */
> - if (!S_ISDIR(st.st_mode)) {
> - errno = ENOTDIR;
> - ret = SCLD_EXISTS;
> - }
> - } else if (mkdir(path, 0777)) {
> - if (errno == EEXIST &&
> - !stat(path, &st) && S_ISDIR(st.st_mode))
> - ; /* somebody created it since we checked */
> - else if (errno == ENOENT)
> - /*
> - * Either mkdir() failed because
> - * somebody just pruned the containing
> - * directory, or stat() failed because
> - * the file that was in our way was
> - * just removed. Either way, inform
> - * the caller that it might be worth
> - * trying again:
> - */
> - ret = SCLD_VANISHED;
> - else
> - ret = SCLD_FAILED;
> - } else if (share && adjust_shared_perm(the_repository, path)) {
> - ret = SCLD_PERMS;
> - }
> - *slash = slash_character;
> - }
> - return ret;
> -}
> -
> -enum scld_error safe_create_leading_directories(char *path)
> -{
> - return safe_create_leading_directories_1(path, 1);
> -}
> -
> -enum scld_error safe_create_leading_directories_no_share(char *path)
> -{
> - return safe_create_leading_directories_1(path, 0);
> -}
> -
> -enum scld_error safe_create_leading_directories_const(const char *path)
> -{
> - int save_errno;
> - /* path points to cache entries, so xstrdup before messing with it */
> - char *buf = xstrdup(path);
> - enum scld_error result = safe_create_leading_directories(buf);
> -
> - save_errno = errno;
> - free(buf);
> - errno = save_errno;
> - return result;
> -}
> -
> int odb_mkstemp(struct strbuf *temp_filename, const char *pattern)
> {
> int fd;
> diff --git a/object-file.h b/object-file.h
> index 81b30d269c8..922f2bba8c9 100644
> --- a/object-file.h
> +++ b/object-file.h
> @@ -21,41 +21,6 @@ extern int fetch_if_missing;
> int index_fd(struct index_state *istate, struct object_id *oid, int fd, struct stat *st, enum object_type type, const char *path, unsigned flags);
> int index_path(struct index_state *istate, struct object_id *oid, const char *path, struct stat *st, unsigned flags);
>
> -/*
> - * Create the directory containing the named path, using care to be
> - * somewhat safe against races. Return one of the scld_error values to
> - * indicate success/failure. On error, set errno to describe the
> - * problem.
> - *
> - * SCLD_VANISHED indicates that one of the ancestor directories of the
> - * path existed at one point during the function call and then
> - * suddenly vanished, probably because another process pruned the
> - * directory while we were working. To be robust against this kind of
> - * race, callers might want to try invoking the function again when it
> - * returns SCLD_VANISHED.
> - *
> - * safe_create_leading_directories() temporarily changes path while it
> - * is working but restores it before returning.
> - * safe_create_leading_directories_const() doesn't modify path, even
> - * temporarily. Both these variants adjust the permissions of the
> - * created directories to honor core.sharedRepository, so they are best
> - * suited for files inside the git dir. For working tree files, use
> - * safe_create_leading_directories_no_share() instead, as it ignores
> - * the core.sharedRepository setting.
> - */
> -enum scld_error {
> - SCLD_OK = 0,
> - SCLD_FAILED = -1,
> - SCLD_PERMS = -2,
> - SCLD_EXISTS = -3,
> - SCLD_VANISHED = -4
> -};
> -enum scld_error safe_create_leading_directories(char *path);
> -enum scld_error safe_create_leading_directories_const(const char *path);
> -enum scld_error safe_create_leading_directories_no_share(char *path);
> -
> -int mkdir_in_gitdir(const char *path);
> -
> int git_open_cloexec(const char *name, int flags);
> #define git_open(name) git_open_cloexec(name, O_RDONLY)
>
>
> --
> 2.49.0.682.gc9b6a7b2b0.dirty
>
>
