From 0ac6671cc1fa31c40460b51db8061096b146833c Mon Sep 17 00:00:00 2001 From: James Ainslie <42301770+jamesainslie@users.noreply.github.com> Date: Tue, 28 Jul 2026 16:52:43 -0400 Subject: [PATCH] fix(clone): apply the cloned repository dot config to the same-invocation install A repo committing .config/dot/config.yaml had it ignored during clone: config resolution ran once at CLI startup, before the repo existed on disk, so a zero-touch bootstrap installed with the default layout and mangled the target. After cloning, read the repo config dotfile options and rebuild the manage service used for the install; a broken repo config errors loudly. dot config get/list now resolve with repository priority too, matching every other command. --- cmd/dot/config.go | 8 +- pkg/dot/client.go | 24 ++++++ pkg/dot/clone_repo_config_test.go | 124 ++++++++++++++++++++++++++++++ pkg/dot/clone_service.go | 61 +++++++++++++++ 4 files changed, 213 insertions(+), 4 deletions(-) create mode 100644 pkg/dot/clone_repo_config_test.go diff --git a/cmd/dot/config.go b/cmd/dot/config.go index ef381d5b..d7e48631 100644 --- a/cmd/dot/config.go +++ b/cmd/dot/config.go @@ -200,8 +200,8 @@ For example: directories.package, logging.level`, func runConfigGet(key string) error { configPath := getConfigFilePath() - loader := dot.NewConfigLoader("dot", configPath) - cfg, err := loader.LoadWithEnv() + // Repository config takes precedence, matching every other command. + cfg, err := loadConfigWithRepoPriority(GetCLIFlags().packageDir, configPath) if err != nil { return fmt.Errorf("load config: %w", err) } @@ -341,8 +341,8 @@ Shows the final merged configuration from all sources.`, func runConfigListCmd(cmd *cobra.Command, args []string) error { configPath := getConfigFilePath() - loader := dot.NewConfigLoader("dot", configPath) - cfg, err := loader.LoadWithEnv() + // Repository config takes precedence, matching every other command. + cfg, err := loadConfigWithRepoPriority(GetCLIFlags().packageDir, configPath) if err != nil { return fmt.Errorf("load config: %w", err) } diff --git a/pkg/dot/client.go b/pkg/dot/client.go index b6a3d112..bc7f3d36 100644 --- a/pkg/dot/client.go +++ b/pkg/dot/client.go @@ -134,6 +134,30 @@ func NewClient(cfg Config) (*Client, error) { // Repository info must land in the manifest every other command reads. cloneSvc := newCloneService(cfg.FS, cfg.Logger, manageSvc, gitCloner, packageSelector, manifestStore, cfg.PackageDir, cfg.TargetDir, cfg.DryRun) + // A cloned repository may commit its own dotfile options in + // .config/dot/config.yaml; the clone service rebuilds its manage service + // from them so the first install already uses the repo's layout. + cloneSvc.rebuildManageSvc = func(mapping *bool, translate *bool) *ManageService { + m := cfg.PackageNameMapping + if mapping != nil { + m = *mapping + } + tr := cfg.Translate + if translate != nil { + tr = translate + } + pipe := pipeline.NewManagePipeline(pipeline.ManagePipelineOpts{ + FS: cfg.FS, + IgnoreSet: ignoreSet, + ScanConfig: scanConfig, + Policies: policies, + BackupDir: cfg.BackupDir, + PackageNameMapping: m, + Translate: tr, + }) + return newManageService(cfg.FS, cfg.Logger, pipe, exec, manifestSvc, unmanageSvc, cfg.PackageDir, cfg.TargetDir, cfg.DryRun) + } + // Create bootstrap service bootstrapSvc := newBootstrapService(cfg.FS, cfg.Logger, cfg.PackageDir, cfg.TargetDir) diff --git a/pkg/dot/clone_repo_config_test.go b/pkg/dot/clone_repo_config_test.go new file mode 100644 index 00000000..b888d7f0 --- /dev/null +++ b/pkg/dot/clone_repo_config_test.go @@ -0,0 +1,124 @@ +package dot + +import ( + "context" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/yaklabco/dot/internal/adapters" + "github.com/yaklabco/dot/internal/executor" + "github.com/yaklabco/dot/internal/ignore" + "github.com/yaklabco/dot/internal/manifest" + "github.com/yaklabco/dot/internal/pipeline" + "github.com/yaklabco/dot/internal/planner" +) + +// newRepoConfigCloneService builds a clone service whose pre-clone config has +// package name mapping ENABLED (the shipped default), with a rebuild hook +// mirroring the one the client wires, so a cloned repo carrying +// .config/dot/config.yaml can override the dotfile options for the install. +func newRepoConfigCloneService(t *testing.T, fs FS, repoFiles map[string]string) *CloneService { + t.Helper() + + logger := adapters.NewNoopLogger() + exec := executor.New(executor.Opts{FS: fs, Logger: logger, Tracer: adapters.NewNoopTracer()}) + store := manifest.NewFSManifestStore(fs) + manifestSvc := newManifestService(fs, logger, store) + unmanageSvc := newUnmanageService(fs, logger, exec, manifestSvc, "/packages", "/home", false) + + buildManage := func(mapping bool, translate *bool) *ManageService { + pipe := pipeline.NewManagePipeline(pipeline.ManagePipelineOpts{ + FS: fs, + IgnoreSet: ignore.NewDefaultIgnoreSet(), + Policies: planner.ResolutionPolicies{OnFileExists: planner.PolicySkip}, + PackageNameMapping: mapping, + Translate: translate, + }) + return newManageService(fs, logger, pipe, exec, manifestSvc, unmanageSvc, "/packages", "/home", false) + } + + cloner := &mockGitCloner{ + cloneFn: func(ctx context.Context, url string, dest string, opts adapters.CloneOptions) error { + for path, content := range repoFiles { + full := dest + "/" + path + if err := fs.MkdirAll(ctx, filepath.Dir(full), 0755); err != nil { + return err + } + if err := fs.WriteFile(ctx, full, []byte(content), 0644); err != nil { + return err + } + } + return nil + }, + } + + // Pre-clone configuration: mapping enabled (the default on a machine with + // no user config). + svc := newCloneService(fs, logger, buildManage(true, nil), cloner, &mockPackageSelector{}, store, "/packages", "/home", false) + svc.rebuildManageSvc = func(mapping *bool, translate *bool) *ManageService { + m := true + if mapping != nil { + m = *mapping + } + return buildManage(m, translate) + } + return svc +} + +// TestCloneAppliesRepoDotfileConfig is the zero-touch bootstrap contract: a +// repo that commits .config/dot/config.yaml with package_name_mapping: false +// must install stow-style full-tree links in the SAME clone invocation, on a +// machine with no pre-existing user config. +func TestCloneAppliesRepoDotfileConfig(t *testing.T) { + fs := adapters.NewMemFS() + ctx := context.Background() + require.NoError(t, fs.MkdirAll(ctx, "/home", 0755)) + + svc := newRepoConfigCloneService(t, fs, map[string]string{ + ".config/dot/config.yaml": "dotfile:\n package_name_mapping: false\n", + "mypkg/dot-rc": "content", + }) + + require.NoError(t, svc.Clone(ctx, "https://example.com/dots.git", CloneOptions{})) + + assert.True(t, fs.Exists(ctx, "/home/.rc"), + "full-tree layout expected: /home/.rc must exist") + assert.False(t, fs.Exists(ctx, "/home/mypkg"), + "package-name-prefixed layout must not appear when the repo config disables mapping") +} + +// TestCloneWithoutRepoConfigKeepsDefaults: no repo config means the pre-clone +// configuration governs, unchanged behavior. +func TestCloneWithoutRepoConfigKeepsDefaults(t *testing.T) { + fs := adapters.NewMemFS() + ctx := context.Background() + require.NoError(t, fs.MkdirAll(ctx, "/home", 0755)) + + svc := newRepoConfigCloneService(t, fs, map[string]string{ + "mypkg/dot-rc": "content", + }) + + require.NoError(t, svc.Clone(ctx, "https://example.com/dots.git", CloneOptions{})) + + assert.True(t, fs.Exists(ctx, "/home/mypkg/.rc"), + "mapping enabled pre-clone config must still apply without a repo config") +} + +// TestCloneRejectsBrokenRepoConfig: a present but unparseable repo config is +// a loud error, matching the CLI loader's behavior, never a silent fallback. +func TestCloneRejectsBrokenRepoConfig(t *testing.T) { + fs := adapters.NewMemFS() + ctx := context.Background() + require.NoError(t, fs.MkdirAll(ctx, "/home", 0755)) + + svc := newRepoConfigCloneService(t, fs, map[string]string{ + ".config/dot/config.yaml": "dotfile: [broken", + "mypkg/dot-rc": "content", + }) + + err := svc.Clone(ctx, "https://example.com/dots.git", CloneOptions{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "repository config") +} diff --git a/pkg/dot/clone_service.go b/pkg/dot/clone_service.go index 30a14938..de78d72a 100644 --- a/pkg/dot/clone_service.go +++ b/pkg/dot/clone_service.go @@ -10,6 +10,8 @@ import ( "strings" "time" + "gopkg.in/yaml.v3" + "github.com/yaklabco/dot/internal/adapters" "github.com/yaklabco/dot/internal/bootstrap" "github.com/yaklabco/dot/internal/cli/selector" @@ -37,6 +39,14 @@ type CloneService struct { // hostname reports the machine hostname for machines resolution. // Defaults to os.Hostname; tests substitute their own. hostname func() (string, error) + + // rebuildManageSvc rebuilds the manage service with dotfile options taken + // from the just-cloned repository's .config/dot/config.yaml, so a repo + // that commits its own layout settings installs correctly in the same + // clone invocation, before any user-level config exists. A nil value for + // either option means "keep the pre-clone configuration". Wired by the + // client; nil disables the reload. + rebuildManageSvc func(packageNameMapping *bool, translate *bool) *ManageService } // newCloneService creates a new clone service. @@ -140,6 +150,12 @@ func (s *CloneService) Clone(ctx context.Context, repoURL string, opts CloneOpti s.logger.Info(ctx, "repository_cloned_successfully", "path", s.packageDir) + // The cloned repository may carry its own dot configuration; apply its + // dotfile options to the install that follows. + if err := s.applyRepoDotfileConfig(ctx); err != nil { + return err + } + // Load bootstrap configuration if present s.logger.Debug(ctx, "checking_for_bootstrap_config") bootstrapConfig, hasBootstrap, err := loadBootstrapConfig(ctx, s.fs, s.packageDir) @@ -209,6 +225,51 @@ func (s *CloneService) Clone(ctx context.Context, repoURL string, opts CloneOpti return nil } +// repoDotfileConfig is the subset of a repository-level config.yaml the +// clone install step cares about. +type repoDotfileConfig struct { + Dotfile struct { + PackageNameMapping *bool `yaml:"package_name_mapping"` + Translate *bool `yaml:"translate"` + } `yaml:"dotfile"` +} + +// applyRepoDotfileConfig reads .config/dot/config.yaml from the cloned +// repository and, when it sets dotfile options, rebuilds the manage service +// used for the install. Absent file: no-op. Present but unparseable: loud +// error, matching the CLI config loader's contract for repository configs. +func (s *CloneService) applyRepoDotfileConfig(ctx context.Context) error { + configPath := filepath.Join(s.packageDir, ".config", "dot", "config.yaml") + if !s.fs.Exists(ctx, configPath) { + return nil + } + + data, err := s.fs.ReadFile(ctx, configPath) + if err != nil { + return fmt.Errorf("read repository config %s: %w", configPath, err) + } + + var cfg repoDotfileConfig + if err := yaml.Unmarshal(data, &cfg); err != nil { + return fmt.Errorf("parse repository config %s: %w", configPath, err) + } + + if cfg.Dotfile.PackageNameMapping == nil && cfg.Dotfile.Translate == nil { + return nil + } + if s.rebuildManageSvc == nil { + s.logger.Warn(ctx, "repo_config_found_but_reload_unsupported", "path", configPath) + return nil + } + + s.manageSvc = s.rebuildManageSvc(cfg.Dotfile.PackageNameMapping, cfg.Dotfile.Translate) + s.logger.Info(ctx, "repo_config_applied", + "path", configPath, + "package_name_mapping", cfg.Dotfile.PackageNameMapping, + "translate", cfg.Dotfile.Translate) + return nil +} + // profileFromMachines resolves a profile for the current host from the // machines section of the bootstrap config. //