Skip to content

Commit 4a02ef0

Browse files
aledbfclaude
andcommitted
fix(config): port upstream #1213 (colon defaults) and #1261 (worktree custom mount)
#1213: a ${localEnv:VAR:default} default value may itself contain colons (URLs, image:tag, host:port). resolveEnvTag now rejoins the parts after the variable name instead of dropping everything past the first colon. #1261: when the config sets a custom workspaceMount, the Git worktree common-dir mount is now resolved relative to that (substituted) mount target instead of the auto-computed container folder, so git works in the container for monorepo setups with a custom workspaceMount/workspaceFolder. Both are the same bugs the Go port inherited from the pre-fix reference. Tests added. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 73d7299 commit 4a02ef0

4 files changed

Lines changed: 144 additions & 5 deletions

File tree

internal/config/loader.go

Lines changed: 69 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,15 @@ func computeWorkspaceConfig(workspace *Workspace, config *DevContainer, mountWor
343343
containerMountFolder := "/workspaces/" + filepath.Base(sourceFolder)
344344
var additionalMounts []string
345345
if mountWorkspaceGitRoot && mountGitWorktreeCommonDir && !config.IsComposeConfig() {
346-
if remapped, commonDirMount, ok := gitWorktreeCommonDirMount(sourceFolder, consistency); ok {
346+
// A custom workspaceMount defines where the worktree is actually mounted in
347+
// the container; resolve the common dir relative to that (substituted)
348+
// target so the relative gitdir still resolves. Otherwise fall back to the
349+
// computed container mount folder (#1261).
350+
customTarget := ""
351+
if config.WorkspaceMount != "" {
352+
customTarget = substituteHostString(workspace, mountTarget(config.WorkspaceMount))
353+
}
354+
if remapped, commonDirMount, ok := gitWorktreeCommonDirMount(sourceFolder, customTarget, consistency); ok {
347355
containerMountFolder = remapped
348356
additionalMounts = append(additionalMounts, commonDirMount)
349357
}
@@ -402,7 +410,7 @@ func bindMount(source, target, consistency string) string {
402410
// the shared common dir (the main repo's `.git`) into the container. ok is false
403411
// for a normal clone (`.git` is a directory), an absolute gitdir, or a missing
404412
// gitlink — all cases where no extra mount is needed.
405-
func gitWorktreeCommonDirMount(hostMountFolder, consistency string) (containerMountFolder, additionalMount string, ok bool) {
413+
func gitWorktreeCommonDirMount(hostMountFolder, customContainerTarget, consistency string) (containerMountFolder, additionalMount string, ok bool) {
406414
info, err := os.Stat(filepath.Join(hostMountFolder, ".git"))
407415
if err != nil || !info.Mode().IsRegular() {
408416
return "", "", false
@@ -429,11 +437,68 @@ func gitWorktreeCommonDirMount(hostMountFolder, consistency string) (containerMo
429437
}
430438
containerMountFolder = path.Join(append([]string{"/workspaces"}, segments...)...)
431439

432-
// The common dir lands at the same relative offset inside the container.
433-
containerGitCommonDir := path.Clean(path.Join(containerMountFolder, filepath.ToSlash(gitdir), "..", ".."))
440+
// The common dir lands at the same relative offset from wherever the worktree
441+
// is mounted: a custom workspaceMount target when the config sets one, else the
442+
// computed container mount folder.
443+
worktreeContainerFolder := containerMountFolder
444+
if customContainerTarget != "" {
445+
worktreeContainerFolder = customContainerTarget
446+
}
447+
containerGitCommonDir := path.Clean(path.Join(worktreeContainerFolder, filepath.ToSlash(gitdir), "..", ".."))
434448
return containerMountFolder, bindMount(gitCommonDir, containerGitCommonDir, consistency), true
435449
}
436450

451+
// mountTarget extracts the target= value from a `type=bind,...` mount spec,
452+
// tolerating a quoted source/target that itself contains commas.
453+
func mountTarget(spec string) string {
454+
for _, f := range splitMountFields(spec) {
455+
if t, ok := strings.CutPrefix(f, "target="); ok {
456+
return t
457+
}
458+
}
459+
return ""
460+
}
461+
462+
// splitMountFields splits a docker mount spec on commas that are not inside
463+
// double quotes, stripping the quotes.
464+
func splitMountFields(spec string) []string {
465+
var fields []string
466+
var b strings.Builder
467+
inQuote := false
468+
for _, r := range spec {
469+
switch {
470+
case r == '"':
471+
inQuote = !inQuote
472+
case r == ',' && !inQuote:
473+
fields = append(fields, b.String())
474+
b.Reset()
475+
default:
476+
b.WriteRune(r)
477+
}
478+
}
479+
return append(fields, b.String())
480+
}
481+
482+
// substituteHostString applies host-phase variable substitution to s using a
483+
// minimal host context (platform, local workspace folder, env). Used for the
484+
// custom workspaceMount target, which is resolved before the main substitution
485+
// pass. Returns s unchanged on error or when it has no ${...} tags.
486+
func substituteHostString(workspace *Workspace, s string) string {
487+
if !strings.Contains(s, "${") {
488+
return s
489+
}
490+
ctx := HostSubContext{
491+
Platform: currentPlatform(),
492+
LocalWorkspaceFolder: strings.TrimRight(workspace.RootFolderPath, "/\\"),
493+
Env: envFromOS(),
494+
}
495+
out, err := NewVariableResolver().resolveString(SubstitutionContext{HostSubContext: ctx}, PhaseHost, s)
496+
if err != nil {
497+
return s
498+
}
499+
return out
500+
}
501+
437502
// parseGitdir extracts the target of a `gitdir: <path>` line from a `.git`
438503
// gitlink file. No regexp, matching the rest of the package.
439504
func parseGitdir(content string) (string, bool) {

internal/config/varsub.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,10 @@ func resolveEnvTag(isWin bool, env map[string]string, args []string, tag, config
243243
return value, true, nil
244244
}
245245
if len(args) > 1 {
246-
return args[1], true, nil
246+
// Rejoin the remaining parts so a default value may itself contain colons
247+
// (e.g. ${localEnv:REG:my.registry.io:5000/img}); only the first colon
248+
// separates the variable name from its default.
249+
return strings.Join(args[1:], ":"), true, nil
247250
}
248251
return "", true, nil
249252
}

internal/config/varsub_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,10 @@ func TestVariableResolverHostPhase(t *testing.T) {
9595
{"localEnv with suffix", localEnvCtx, "${localEnv:HOME}/project", "/home/user/project"},
9696
{"localEnv missing", localEnvCtx, "${localEnv:MISSING}", ""},
9797
{"localEnv missing with fallback", localEnvCtx, "${localEnv:MISSING:fallback}", "fallback"},
98+
// A default value may itself contain colons (URLs, image:tag, host:port) —
99+
// only the first colon separates the var name from its default (#1213).
100+
{"localEnv fallback with colons", localEnvCtx, "${localEnv:MISSING:my.registry.io:5000/img:tag}", "my.registry.io:5000/img:tag"},
101+
{"localEnv present ignores colon fallback", localEnvCtx, "${localEnv:HOME:a:b:c}", "/home/user"},
98102
{"no vars", localEnvCtx, "no vars here", "no vars here"},
99103
{"multiple localEnv", localEnvCtx, "${localEnv:USER}@${localEnv:HOME}", "test@/home/user"},
100104

internal/config/worktree_test.go

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,3 +196,70 @@ func TestLoadDevContainerConfigWithMountsWorktree(t *testing.T) {
196196
t.Errorf("default load AdditionalMounts = %q, want none", res2.WorkspaceConfig.AdditionalMounts)
197197
}
198198
}
199+
200+
// TestWorktreeCommonDirCustomWorkspaceMount ports upstream #1261: when the config
201+
// sets a custom workspaceMount/workspaceFolder, the common-dir mount is still
202+
// added, with its target resolved relative to the custom mount target.
203+
func TestWorktreeCommonDirCustomWorkspaceMount(t *testing.T) {
204+
if runtime.GOOS != "linux" {
205+
t.Skip("oracle pins the linux consistency behavior (empty suffix)")
206+
}
207+
208+
t.Run("common dir target relative to custom mount", func(t *testing.T) {
209+
base := t.TempDir()
210+
wt := writeGitlink(t, base, "worktrees/feature", "gitdir: ../main/.git/worktrees/feature")
211+
mainGit := filepath.Join(base, "worktrees", "main", ".git")
212+
213+
wc := computeWorkspaceConfig(&Workspace{RootFolderPath: wt}, &DevContainer{
214+
WorkspaceMount: "type=bind,source=/host/wt,target=/workspace",
215+
WorkspaceFolder: "/workspace",
216+
}, true, true)
217+
218+
// Custom mount/folder respected verbatim; common dir mounted, target
219+
// resolved relative to /workspace → /main/.git.
220+
if wc.WorkspaceMount != "type=bind,source=/host/wt,target=/workspace" {
221+
t.Errorf("workspaceMount = %q", wc.WorkspaceMount)
222+
}
223+
if wc.WorkspaceFolder != "/workspace" {
224+
t.Errorf("workspaceFolder = %q", wc.WorkspaceFolder)
225+
}
226+
want := "type=bind,source=" + mainGit + ",target=/main/.git"
227+
if len(wc.AdditionalMounts) != 1 || wc.AdditionalMounts[0] != want {
228+
t.Errorf("additionalMounts = %q, want [%q]", wc.AdditionalMounts, want)
229+
}
230+
})
231+
232+
t.Run("variables in the custom mount target are substituted", func(t *testing.T) {
233+
base := t.TempDir()
234+
wt := writeGitlink(t, base, "worktrees/feature", "gitdir: ../main/.git/worktrees/feature")
235+
mainGit := filepath.Join(base, "worktrees", "main", ".git")
236+
237+
// target uses ${localWorkspaceFolderBasename} → "feature".
238+
wc := computeWorkspaceConfig(&Workspace{RootFolderPath: wt}, &DevContainer{
239+
WorkspaceMount: "type=bind,source=/host/wt,target=/src/${localWorkspaceFolderBasename}",
240+
WorkspaceFolder: "/src/feature",
241+
}, true, true)
242+
243+
// common dir resolved relative to the substituted target /src/feature →
244+
// /src/main/.git.
245+
want := "type=bind,source=" + mainGit + ",target=/src/main/.git"
246+
if len(wc.AdditionalMounts) != 1 || wc.AdditionalMounts[0] != want {
247+
t.Errorf("additionalMounts = %q, want [%q]", wc.AdditionalMounts, want)
248+
}
249+
})
250+
}
251+
252+
func TestMountTarget(t *testing.T) {
253+
cases := map[string]string{
254+
"type=bind,source=/a,target=/b": "/b",
255+
"type=bind,source=/a,target=/b,consistency=cached": "/b",
256+
`type=bind,source="/a,x",target="/b,y"`: "/b,y",
257+
"type=volume,source=vol,target=/data": "/data",
258+
"type=bind,source=/a": "",
259+
}
260+
for spec, want := range cases {
261+
if got := mountTarget(spec); got != want {
262+
t.Errorf("mountTarget(%q) = %q, want %q", spec, got, want)
263+
}
264+
}
265+
}

0 commit comments

Comments
 (0)