Skip to content

Commit 65f6a9e

Browse files
vdyegitster
vdye
authored and
gitster
committed
scalar: constrain enlistment search
Make the search for repository and enlistment root in 'setup_enlistment_directory()' more constrained to simplify behavior and adhere to 'GIT_CEILING_DIRECTORIES'. Previously, 'setup_enlistment_directory()' would check whether the provided path (or current working directory) '<dir>' or its subdirectory '<dir>/src' was a repository root. If not, the process would repeat on the parent of '<dir>' until the repository was found or it reached the root of the filesystem. This meant that a user could specify a path *anywhere* inside an enlistment (including paths not in the repository contained within the enlistment) and it would be found. The downside to this process is that the search would not account for 'GIT_CEILING_DIRECTORIES', so the upward search could result in modifying repository contents past 'GIT_CEILING_DIRECTORIES'. Similarly, operations like 'scalar delete' could end up unintentionally deleting the parent of a repo if its root was named 'src'. To make this 'setup_enlistment_directory()' both adhere to 'GIT_CEILING_DIRECTORIES' and avoid unwanted deletions, the search for an enlistment directory is simplified to: - if '<dir>/src' is a repository root, '<dir>' is the enlistment root - if '<dir>' is either the repository root or contained within a repository, the repository root is the enlistment root Now, only 'setup_git_directory()' (called by 'setup_enlistment_directory()') searches upwards from the 'scalar' specified path, enforcing 'GIT_CEILING_DIRECTORIES' in the process. Additionally, 'scalar delete <dir>/src' will not delete '<dir>' (if users would like to delete it, they can still specify the enlistment root with 'scalar delete <dir>'). This is true of any 'scalar' operation; users can invoke 'scalar' on the enlistment root, but paths must otherwise be inside the repository to be valid. To help clarify the updated behavior, new tests are added to 't9099-scalar.sh'. Finally, this change leaves 'strbuf_parent_directory()' with only a single, WIN32-specific caller in 'delete_enlistment()'. Rather than wrap 'strbuf_parent_directory()' in '#ifdef WIN32' to avoid the "unused function" compiler error, move the contents of 'strbuf_parent_directory()' into 'delete_enlistment()' and remove the function. Helped-by: Derrick Stolee <[email protected]> Signed-off-by: Victoria Dye <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 9bf691b commit 65f6a9e

File tree

2 files changed

+113
-54
lines changed

2 files changed

+113
-54
lines changed

‎contrib/scalar/scalar.c‎

Lines changed: 28 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -14,29 +14,14 @@
1414
#include "archive.h"
1515
#include "object-store.h"
1616

17-
/*
18-
* Remove the deepest subdirectory in the provided path string. Path must not
19-
* include a trailing path separator. Returns 1 if parent directory found,
20-
* otherwise 0.
21-
*/
22-
static int strbuf_parent_directory(struct strbuf *buf)
23-
{
24-
size_t len = buf->len;
25-
size_t offset = offset_1st_component(buf->buf);
26-
char *path_sep = find_last_dir_sep(buf->buf + offset);
27-
strbuf_setlen(buf, path_sep ? path_sep - buf->buf : offset);
28-
29-
return buf->len < len;
30-
}
31-
3217
static void setup_enlistment_directory(int argc, const char **argv,
3318
const char * const *usagestr,
3419
const struct option *options,
3520
struct strbuf *enlistment_root)
3621
{
3722
struct strbuf path = STRBUF_INIT;
38-
char *root;
39-
int enlistment_found = 0;
23+
int enlistment_is_repo_parent = 0;
24+
size_t len;
4025

4126
if (startup_info->have_repository)
4227
BUG("gitdir already set up?!?");
@@ -49,51 +34,36 @@ static void setup_enlistment_directory(int argc, const char **argv,
4934
strbuf_add_absolute_path(&path, argv[0]);
5035
if (!is_directory(path.buf))
5136
die(_("'%s' does not exist"), path.buf);
37+
if (chdir(path.buf) < 0)
38+
die_errno(_("could not switch to '%s'"), path.buf);
5239
} else if (strbuf_getcwd(&path) < 0)
5340
die(_("need a working directory"));
5441

5542
strbuf_trim_trailing_dir_sep(&path);
56-
do {
57-
const size_t len = path.len;
58-
59-
/* check if currently in enlistment root with src/ workdir */
60-
strbuf_addstr(&path, "/src");
61-
if (is_nonbare_repository_dir(&path)) {
62-
if (enlistment_root)
63-
strbuf_add(enlistment_root, path.buf, len);
64-
65-
enlistment_found = 1;
66-
break;
67-
}
6843

69-
/* reset to original path */
70-
strbuf_setlen(&path, len);
71-
72-
/* check if currently in workdir */
73-
if (is_nonbare_repository_dir(&path)) {
74-
if (enlistment_root) {
75-
/*
76-
* If the worktree's directory's name is `src`, the enlistment is the
77-
* parent directory, otherwise it is identical to the worktree.
78-
*/
79-
root = strip_path_suffix(path.buf, "src");
80-
strbuf_addstr(enlistment_root, root ? root : path.buf);
81-
free(root);
82-
}
44+
/* check if currently in enlistment root with src/ workdir */
45+
len = path.len;
46+
strbuf_addstr(&path, "/src");
47+
if (is_nonbare_repository_dir(&path)) {
48+
enlistment_is_repo_parent = 1;
49+
if (chdir(path.buf) < 0)
50+
die_errno(_("could not switch to '%s'"), path.buf);
51+
}
52+
strbuf_setlen(&path, len);
8353

84-
enlistment_found = 1;
85-
break;
86-
}
87-
} while (strbuf_parent_directory(&path));
54+
setup_git_directory();
8855

89-
if (!enlistment_found)
90-
die(_("could not find enlistment root"));
56+
if (!the_repository->worktree)
57+
die(_("Scalar enlistments require a worktree"));
9158

92-
if (chdir(path.buf) < 0)
93-
die_errno(_("could not switch to '%s'"), path.buf);
59+
if (enlistment_root) {
60+
if (enlistment_is_repo_parent)
61+
strbuf_addbuf(enlistment_root, &path);
62+
else
63+
strbuf_addstr(enlistment_root, the_repository->worktree);
64+
}
9465

9566
strbuf_release(&path);
96-
setup_git_directory();
9767
}
9868

9969
static int run_git(const char *arg, ...)
@@ -431,6 +401,8 @@ static int delete_enlistment(struct strbuf *enlistment)
431401
{
432402
#ifdef WIN32
433403
struct strbuf parent = STRBUF_INIT;
404+
size_t offset;
405+
char *path_sep;
434406
#endif
435407

436408
if (unregister_dir())
@@ -441,8 +413,10 @@ static int delete_enlistment(struct strbuf *enlistment)
441413
* Change the current directory to one outside of the enlistment so
442414
* that we may delete everything underneath it.
443415
*/
444-
strbuf_addbuf(&parent, enlistment);
445-
strbuf_parent_directory(&parent);
416+
offset = offset_1st_component(enlistment->buf);
417+
path_sep = find_last_dir_sep(enlistment->buf + offset);
418+
strbuf_add(&parent, enlistment->buf,
419+
path_sep ? path_sep - enlistment->buf : offset);
446420
if (chdir(parent.buf) < 0)
447421
die_errno(_("could not switch to '%s'"), parent.buf);
448422
strbuf_release(&parent);

‎contrib/scalar/t/t9099-scalar.sh‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,91 @@ test_expect_success 'scalar shows a usage' '
1717
test_expect_code 129 scalar -h
1818
'
1919

20+
test_expect_success 'scalar invoked on enlistment root' '
21+
test_when_finished rm -rf test src deeper &&
22+
23+
for enlistment_root in test src deeper/test
24+
do
25+
git init ${enlistment_root}/src &&
26+
27+
# Register
28+
scalar register ${enlistment_root} &&
29+
scalar list >out &&
30+
grep "$(pwd)/${enlistment_root}/src\$" out &&
31+
32+
# Delete (including enlistment root)
33+
scalar delete $enlistment_root &&
34+
test_path_is_missing $enlistment_root &&
35+
scalar list >out &&
36+
! grep "^$(pwd)/${enlistment_root}/src\$" out || return 1
37+
done
38+
'
39+
40+
test_expect_success 'scalar invoked on enlistment src repo' '
41+
test_when_finished rm -rf test src deeper &&
42+
43+
for enlistment_root in test src deeper/test
44+
do
45+
git init ${enlistment_root}/src &&
46+
47+
# Register
48+
scalar register ${enlistment_root}/src &&
49+
scalar list >out &&
50+
grep "$(pwd)/${enlistment_root}/src\$" out &&
51+
52+
# Delete (will not include enlistment root)
53+
scalar delete ${enlistment_root}/src &&
54+
test_path_is_dir $enlistment_root &&
55+
scalar list >out &&
56+
! grep "^$(pwd)/${enlistment_root}/src\$" out || return 1
57+
done
58+
'
59+
60+
test_expect_success 'scalar invoked when enlistment root and repo are the same' '
61+
test_when_finished rm -rf test src deeper &&
62+
63+
for enlistment_root in test src deeper/test
64+
do
65+
git init ${enlistment_root} &&
66+
67+
# Register
68+
scalar register ${enlistment_root} &&
69+
scalar list >out &&
70+
grep "$(pwd)/${enlistment_root}\$" out &&
71+
72+
# Delete (will not include enlistment root)
73+
scalar delete ${enlistment_root} &&
74+
test_path_is_missing $enlistment_root &&
75+
scalar list >out &&
76+
! grep "^$(pwd)/${enlistment_root}\$" out &&
77+
78+
# Make sure we did not accidentally delete the trash dir
79+
test_path_is_dir "$TRASH_DIRECTORY" || return 1
80+
done
81+
'
82+
83+
test_expect_success 'scalar repo search respects GIT_CEILING_DIRECTORIES' '
84+
test_when_finished rm -rf test &&
85+
86+
git init test/src &&
87+
mkdir -p test/src/deep &&
88+
GIT_CEILING_DIRECTORIES="$(pwd)/test/src" &&
89+
! scalar register test/src/deep 2>err &&
90+
grep "not a git repository" err
91+
'
92+
93+
test_expect_success 'scalar enlistments need a worktree' '
94+
test_when_finished rm -rf bare test &&
95+
96+
git init --bare bare/src &&
97+
! scalar register bare/src 2>err &&
98+
grep "Scalar enlistments require a worktree" err &&
99+
100+
git init test/src &&
101+
! scalar register test/src/.git 2>err &&
102+
grep "Scalar enlistments require a worktree" err
103+
'
104+
20105
test_expect_success 'scalar unregister' '
21106
git init vanish/src &&
22107
scalar register vanish/src &&

0 commit comments

Comments
 (0)