Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bug: links are relative to linkdir, not rootdir #848

Merged
merged 3 commits into from
Dec 9, 2023
Merged

Bug: links are relative to linkdir, not rootdir #848

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
022a2cb
Log worktree path
thockin Dec 8, 2023
4564d4c
Bug: links are relative to linkdir, not rootdir
thockin Dec 8, 2023
4b5b1bf
Make abspath Split return an abspath
thockin Dec 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions abspath.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ func (abs absPath) Canonical() (absPath, error) {
return absPath(result), nil
}

// Join appends more path elements to abs, like filepath.Join.
// Join appends more path elements to abs, like filepath.Join. This will clean
// the final path (e.g. resolve ".." elements).
func (abs absPath) Join(elems ...string) absPath {
all := make([]string, 0, 1+len(elems))
all = append(all, abs.String())
Expand All @@ -59,7 +60,7 @@ func (abs absPath) Join(elems ...string) absPath {
// Split breaks abs into stem and leaf parts (often directory and file, but not
// necessarily), similar to filepath.Split. Unlike filepath.Split, the
// resulting stem part does not have any trailing path separators.
func (abs absPath) Split() (string, string) {
func (abs absPath) Split() (absPath, string) {
if abs == "" {
return "", ""
}
Expand All @@ -73,13 +74,13 @@ func (abs absPath) Split() (string, string) {
dir = string(os.PathSeparator)
}

return dir, base
return absPath(dir), base
}

// Dir returns the stem part of abs without the leaf, like filepath.Dir.
func (abs absPath) Dir() string {
dir, _ := abs.Split()
return dir
return string(dir)
}

// Base returns the leaf part of abs without the stem, like filepath.Base.
Expand Down
2 changes: 1 addition & 1 deletion abspath_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func TestAbsPathJoin(t *testing.T) {
func TestAbsPathSplit(t *testing.T) {
testCases := []struct {
in absPath
expDir string
expDir absPath
expBase string
}{{
in: "",
Expand Down
17 changes: 9 additions & 8 deletions main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ func main() {
}
log := func() *logging.Logger {
dir, file := makeAbsPath(*flErrorFile, absRoot).Split()
return logging.New(dir, file, *flVerbose)
return logging.New(dir.String(), file, *flVerbose)
}()
cmdRunner := cmd.NewRunner(log)

Expand Down Expand Up @@ -1273,25 +1273,25 @@ func (git *repoSync) publishSymlink(ctx context.Context, worktree worktree) erro
linkDir, linkFile := git.link.Split()

// Make sure the link directory exists.
if err := os.MkdirAll(linkDir, defaultDirMode); err != nil {
if err := os.MkdirAll(linkDir.String(), defaultDirMode); err != nil {
return fmt.Errorf("error making symlink dir: %w", err)
}

// newDir is absolute, so we need to change it to a relative path. This is
// linkDir is absolute, so we need to change it to a relative path. This is
// so it can be volume-mounted at another path and the symlink still works.
targetRelative, err := filepath.Rel(linkDir, targetPath.String())
targetRelative, err := filepath.Rel(linkDir.String(), targetPath.String())
if err != nil {
return fmt.Errorf("error converting to relative path: %w", err)
}

const tmplink = "tmp-link"
git.log.V(2).Info("creating tmp symlink", "dir", linkDir, "link", tmplink, "target", targetRelative)
if err := os.Symlink(targetRelative, filepath.Join(linkDir, tmplink)); err != nil {
if err := os.Symlink(targetRelative, filepath.Join(linkDir.String(), tmplink)); err != nil {
return fmt.Errorf("error creating symlink: %w", err)
}

git.log.V(2).Info("renaming symlink", "root", linkDir, "oldName", tmplink, "newName", linkFile)
if err := os.Rename(filepath.Join(linkDir, tmplink), git.link.String()); err != nil {
if err := os.Rename(filepath.Join(linkDir.String(), tmplink), git.link.String()); err != nil {
return fmt.Errorf("error replacing symlink: %w", err)
}

Expand Down Expand Up @@ -1573,7 +1573,8 @@ func (git *repoSync) currentWorktree() (worktree, error) {
if filepath.IsAbs(target) {
return worktree(target), nil
}
return worktree(git.root.Join(target)), nil
linkDir, _ := git.link.Split()
return worktree(linkDir.Join(target)), nil
}

// SyncRepo syncs the repository to the desired ref, publishes it via the link,
Expand Down Expand Up @@ -1619,7 +1620,7 @@ func (git *repoSync) SyncRepo(ctx context.Context, refreshCreds func(context.Con
currentWorktree = wt
}
currentHash := currentWorktree.Hash()
git.log.V(3).Info("current hash", "hash", currentHash)
git.log.V(3).Info("current state", "hash", currentHash, "worktree", currentWorktree)

if currentHash == remoteHash {
// We seem to have the right hash already. Let's be sure it's good.
Expand Down
120 changes: 90 additions & 30 deletions test_e2e.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,36 +411,6 @@ function e2e::init_root_fails_sanity() {
assert_file_eq "$ROOT/link/file" "$FUNCNAME"
}

##############################################
# Test init with an absolute-path link
##############################################
function e2e::sync_absolute_link() {
GIT_SYNC \
--one-time \
--repo="file://$REPO" \
--root="$ROOT/root" \
--link="$ROOT/other/dir/link"
assert_file_absent "$ROOT/root/link"
assert_link_exists "$ROOT/other/dir/link"
assert_file_exists "$ROOT/other/dir/link/file"
assert_file_eq "$ROOT/other/dir/link/file" "$FUNCNAME"
}

##############################################
# Test init with a subdir-path link
##############################################
function e2e::sync_subdir_link() {
GIT_SYNC \
--one-time \
--repo="file://$REPO" \
--root="$ROOT" \
--link="other/dir/link"
assert_file_absent "$ROOT/link"
assert_link_exists "$ROOT/other/dir/link"
assert_file_exists "$ROOT/other/dir/link/file"
assert_file_eq "$ROOT/other/dir/link/file" "$FUNCNAME"
}

##############################################
# Test non-zero exit with a bad ref
##############################################
Expand Down Expand Up @@ -539,6 +509,96 @@ function e2e::sync_head() {
assert_metric_eq "${METRIC_FETCH_COUNT}" 3
}

##############################################
# Test sync with an absolute-path link
##############################################
function e2e::sync_head_absolute_link() {
# First sync
echo "$FUNCNAME 1" > "$REPO/file"
git -C "$REPO" commit -qam "$FUNCNAME 1"

GIT_SYNC \
--period=100ms \
--repo="file://$REPO" \
--ref=HEAD \
--root="$ROOT/root" \
--link="$ROOT/other/dir/link" \
&
wait_for_sync "${MAXWAIT}"
assert_file_absent "$ROOT/root/link"
assert_link_exists "$ROOT/other/dir/link"
assert_file_exists "$ROOT/other/dir/link/file"
assert_file_eq "$ROOT/other/dir/link/file" "$FUNCNAME 1"
assert_metric_eq "${METRIC_GOOD_SYNC_COUNT}" 1
assert_metric_eq "${METRIC_FETCH_COUNT}" 1

# Move HEAD forward
echo "$FUNCNAME 2" > "$REPO/file"
git -C "$REPO" commit -qam "$FUNCNAME 2"
wait_for_sync "${MAXWAIT}"
assert_file_absent "$ROOT/root/link"
assert_link_exists "$ROOT/other/dir/link"
assert_file_exists "$ROOT/other/dir/link/file"
assert_file_eq "$ROOT/other/dir/link/file" "$FUNCNAME 2"
assert_metric_eq "${METRIC_GOOD_SYNC_COUNT}" 2
assert_metric_eq "${METRIC_FETCH_COUNT}" 2

# Move HEAD backward
git -C "$REPO" reset -q --hard HEAD^
wait_for_sync "${MAXWAIT}"
assert_file_absent "$ROOT/root/link"
assert_link_exists "$ROOT/other/dir/link"
assert_file_exists "$ROOT/other/dir/link/file"
assert_file_eq "$ROOT/other/dir/link/file" "$FUNCNAME 1"
assert_metric_eq "${METRIC_GOOD_SYNC_COUNT}" 3
assert_metric_eq "${METRIC_FETCH_COUNT}" 3
}

##############################################
# Test sync with a subdir-path link
##############################################
function e2e::sync_head_subdir_link() {
# First sync
echo "$FUNCNAME 1" > "$REPO/file"
git -C "$REPO" commit -qam "$FUNCNAME 1"

GIT_SYNC \
--period=100ms \
--repo="file://$REPO" \
--ref=HEAD \
--root="$ROOT" \
--link="other/dir/link" \
&
wait_for_sync "${MAXWAIT}"
assert_file_absent "$ROOT/link"
assert_link_exists "$ROOT/other/dir/link"
assert_file_exists "$ROOT/other/dir/link/file"
assert_file_eq "$ROOT/other/dir/link/file" "$FUNCNAME 1"
assert_metric_eq "${METRIC_GOOD_SYNC_COUNT}" 1
assert_metric_eq "${METRIC_FETCH_COUNT}" 1

# Move HEAD forward
echo "$FUNCNAME 2" > "$REPO/file"
git -C "$REPO" commit -qam "$FUNCNAME 2"
wait_for_sync "${MAXWAIT}"
assert_file_absent "$ROOT/link"
assert_link_exists "$ROOT/other/dir/link"
assert_file_exists "$ROOT/other/dir/link/file"
assert_file_eq "$ROOT/other/dir/link/file" "$FUNCNAME 2"
assert_metric_eq "${METRIC_GOOD_SYNC_COUNT}" 2
assert_metric_eq "${METRIC_FETCH_COUNT}" 2

# Move HEAD backward
git -C "$REPO" reset -q --hard HEAD^
wait_for_sync "${MAXWAIT}"
assert_file_absent "$ROOT/link"
assert_link_exists "$ROOT/other/dir/link"
assert_file_exists "$ROOT/other/dir/link/file"
assert_file_eq "$ROOT/other/dir/link/file" "$FUNCNAME 1"
assert_metric_eq "${METRIC_GOOD_SYNC_COUNT}" 3
assert_metric_eq "${METRIC_FETCH_COUNT}" 3
}

##############################################
# Test worktree-cleanup
##############################################
Expand Down
Loading