@matheusbsbs / matheus-bagliano / commits / 70a16ff8a1

path: refactor `repo_common_path()` family of functions

The functions provided by the "path" subsystem to derive repository paths for the commondir, gitdir, worktrees and submodules are quite inconsistent. Some functions have a `strbuf_` prefix, others have different return values, some don't provide a variant working on top of `strbuf`s. We're thus about to refactor all of these family of functions so that they follow a common pattern: - `repo_*_path()` returns an allocated string. - `repo_*_path_append()` appends the path to the caller-provided buffer while returning a constant pointer to the buffer. This clarifies whether the buffer is being appended to or rewritten, which otherwise wasn't immediately obvious. - `repo_*_path_replace()` replaces contents of the buffer with the computed path, again returning a pointer to the buffer contents. The returned constant pointer isn't being used anywhere yet, but it will be used in subsequent commits. Its intent is to allow calling patterns like the following somewhat contrived example: if (!stat(&st, repo_common_path_replace(repo, &buf, ...)) && !unlink(repo_common_path_replace(repo, &buf, ...))) ... Refactor the commondir family of functions accordingly and adapt all callers. Note that `repo_common_pathv()` is converted into an internal implementation detail. It is only used to implement `the_repository` compatibility shims and will eventually be removed from the public interface. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Feb 7, 2025 at 12:03 UTC 70a16ff8a162ad0b6a39d17a1699a2949e2a2674

6 files changed +54 -27

loose.c

+3 -3

@@ -75,7 +75,7 @@ static int load_one_loose_object_map(struct repository *repo, struct object_dire

75 insert_loose_map(dir, repo->hash_algo->empty_blob, repo->compat_hash_algo->empty_blob);

76 insert_loose_map(dir, repo->hash_algo->null_oid, repo->compat_hash_algo->null_oid);

78 - strbuf_git_common_path(&path, repo, "objects/loose-object-idx");

78 + repo_common_path_replace(repo, &path, "objects/loose-object-idx");

79 fp = fopen(path.buf, "rb");

80 if (!fp) {

81 strbuf_release(&path);

@@ -133,7 +133,7 @@ int repo_write_loose_object_map(struct repository *repo)

133 if (!should_use_loose_object_map(repo))

134 return 0;

135

136 - strbuf_git_common_path(&path, repo, "objects/loose-object-idx");

136 + repo_common_path_replace(repo, &path, "objects/loose-object-idx");

137 fd = hold_lock_file_for_update_timeout(&lock, path.buf, LOCK_DIE_ON_ERROR, -1);

138 iter = kh_begin(map);

139 if (write_in_full(fd, loose_object_header, strlen(loose_object_header)) < 0)

@@ -174,7 +174,7 @@ static int write_one_object(struct repository *repo, const struct object_id *oid

174 struct stat st;

175 struct strbuf buf = STRBUF_INIT, path = STRBUF_INIT;

176

177 - strbuf_git_common_path(&path, repo, "objects/loose-object-idx");

177 + repo_common_path_replace(repo, &path, "objects/loose-object-idx");

178 hold_lock_file_for_update_timeout(&lock, path.buf, LOCK_DIE_ON_ERROR, -1);

179

180 fd = open(path.buf, O_WRONLY | O_CREAT | O_APPEND, 0666);

path.c

+28 -4

@@ -414,7 +414,7 @@ static void strbuf_worktree_gitdir(struct strbuf *buf,

414 else if (!wt->id)

415 strbuf_addstr(buf, repo->commondir);

416 else

417 - strbuf_git_common_path(buf, repo, "worktrees/%s", wt->id);

417 + repo_common_path_append(repo, buf, "worktrees/%s", wt->id);

418 }

419

420 void repo_git_pathv(const struct repository *repo,

@@ -596,14 +596,38 @@ void repo_common_pathv(const struct repository *repo,

596 strbuf_cleanup_path(sb);

597 }

598

599 -void strbuf_git_common_path(struct strbuf *sb,

600 - const struct repository *repo,

601 - const char *fmt, ...)

599 +char *repo_common_path(const struct repository *repo,

600 + const char *fmt, ...)

601 +{

602 + struct strbuf sb = STRBUF_INIT;

603 + va_list args;

604 + va_start(args, fmt);

605 + repo_common_pathv(repo, &sb, fmt, args);

606 + va_end(args);

607 + return strbuf_detach(&sb, NULL);

608 +}

609 +

610 +const char *repo_common_path_append(const struct repository *repo,

611 + struct strbuf *sb,

612 + const char *fmt, ...)

613 +{

614 + va_list args;

615 + va_start(args, fmt);

616 + repo_common_pathv(repo, sb, fmt, args);

617 + va_end(args);

618 + return sb->buf;

619 +}

620 +

621 +const char *repo_common_path_replace(const struct repository *repo,

622 + struct strbuf *sb,

623 + const char *fmt, ...)

624 {

625 va_list args;

626 + strbuf_reset(sb);

627 va_start(args, fmt);

628 repo_common_pathv(repo, sb, fmt, args);

629 va_end(args);

630 + return sb->buf;

631 }

632

633 static struct passwd *getpw_str(const char *username, size_t len)

path.h

+17 -13

@@ -25,22 +25,20 @@ char *mkpathdup(const char *fmt, ...)

25 __attribute__((format (printf, 1, 2)));

27 /*

28 - * The `strbuf_git_common_path` family of functions will construct a path into a

28 + * The `repo_common_path` family of functions will construct a path into a

29 * repository's common git directory, which is shared by all worktrees.

30 */

31 -

32 -/*

33 - * Constructs a path into the common git directory of repository `repo` and

34 - * append it in the provided buffer `sb`.

35 - */

36 -void strbuf_git_common_path(struct strbuf *sb,

37 - const struct repository *repo,

38 - const char *fmt, ...)

31 +char *repo_common_path(const struct repository *repo,

32 + const char *fmt, ...)

33 + __attribute__((format (printf, 2, 3)));

34 +const char *repo_common_path_append(const struct repository *repo,

35 + struct strbuf *sb,

36 + const char *fmt, ...)

37 + __attribute__((format (printf, 3, 4)));

38 +const char *repo_common_path_replace(const struct repository *repo,

39 + struct strbuf *sb,

40 + const char *fmt, ...)

41 __attribute__((format (printf, 3, 4)));

40 -void repo_common_pathv(const struct repository *repo,

41 - struct strbuf *buf,

42 - const char *fmt,

43 - va_list args);

43 /*

44 * The `repo_git_path` family of functions will construct a path into a repository's

@@ -243,6 +241,12 @@ struct strbuf *get_pathname(void);

241 # include "strbuf.h"

242 # include "repository.h"

243

244 +/* Internal implementation detail that should not be used. */

245 +void repo_common_pathv(const struct repository *repo,

246 + struct strbuf *buf,

247 + const char *fmt,

248 + va_list args);

249 +

250 /*

251 * Return a statically allocated path into the main repository's

252 * (the_repository) common git directory.

refs.c

+2 -2

@@ -2184,8 +2184,8 @@ struct ref_store *get_worktree_ref_store(const struct worktree *wt)

2184

2185 if (wt->id) {

2186 struct strbuf common_path = STRBUF_INIT;

2187 - strbuf_git_common_path(&common_path, wt->repo,

2188 - "worktrees/%s", wt->id);

2187 + repo_common_path_append(wt->repo, &common_path,

2188 + "worktrees/%s", wt->id);

2189 refs = ref_store_init(wt->repo, wt->repo->ref_storage_format,

2190 common_path.buf, REF_STORE_ALL_CAPS);

2191 strbuf_release(&common_path);

setup.c

+2 -2

@@ -792,7 +792,7 @@ int upgrade_repository_format(int target_version)

792 struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;

793 int ret;

794

795 - strbuf_git_common_path(&sb, the_repository, "config");

795 + repo_common_path_append(the_repository, &sb, "config");

796 read_repository_format(&repo_fmt, sb.buf);

797 strbuf_release(&sb);

798

@@ -2242,7 +2242,7 @@ void initialize_repository_version(int hash_algo,

2242 struct strbuf config = STRBUF_INIT;

2243 struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;

2244

2245 - strbuf_git_common_path(&config, the_repository, "config");

2245 + repo_common_path_append(the_repository, &config, "config");

2246 read_repository_format(&repo_fmt, config.buf);

2247

2248 if (repo_fmt.v1_only_extensions.nr)

worktree.c

+2 -3

@@ -104,7 +104,7 @@ struct worktree *get_linked_worktree(const char *id,

104 if (!id)

105 die("Missing linked worktree name");

106

107 - strbuf_git_common_path(&path, the_repository, "worktrees/%s/gitdir", id);

107 + repo_common_path_append(the_repository, &path, "worktrees/%s/gitdir", id);

108 if (strbuf_read_file(&worktree_path, path.buf, 0) <= 0)

109 /* invalid gitdir file */

110 goto done;

@@ -731,8 +731,7 @@ static ssize_t infer_backlink(const char *gitfile, struct strbuf *inferred)

731 id++; /* advance past '/' to point at <id> */

732 if (!*id)

733 goto error;

734 - strbuf_reset(inferred);

735 - strbuf_git_common_path(inferred, the_repository, "worktrees/%s", id);

734 + repo_common_path_replace(the_repository, inferred, "worktrees/%s", id);

735 if (!is_directory(inferred->buf))

736 goto error;

737