From 75c1c852d5a64f41c43859005a3afb7e961601a5 Mon Sep 17 00:00:00 2001 From: Timor Gruber Date: Sun, 23 Nov 2025 18:23:40 +0200 Subject: [PATCH 1/3] remove support for multi-user systems It never relaly worked in practice, so this just adds complexity and noise at this stage. Now that our installer is fully compliant with the depracted shell installer, we have no need for the mutli-user support. --- .github/workflows/installer-ci.yml | 1 - installer/cmd/install.go | 25 +- installer/lib/brew/installer.go | 193 +------ installer/lib/brew/installer_test.go | 529 +----------------- installer/lib/dotfilesmanager/chezmoi/data.go | 2 - .../lib/dotfilesmanager/chezmoi/data_test.go | 12 +- installer/lib/dotfilesmanager/data.go | 2 - 7 files changed, 55 insertions(+), 709 deletions(-) diff --git a/.github/workflows/installer-ci.yml b/.github/workflows/installer-ci.yml index c2c4198..61ab7d0 100644 --- a/.github/workflows/installer-ci.yml +++ b/.github/workflows/installer-ci.yml @@ -216,7 +216,6 @@ jobs: --install-prerequisites=true \ --git-clone-protocol=https \ --work-env=false \ - --multi-user-system=false - name: Install expect tool run: | diff --git a/installer/cmd/install.go b/installer/cmd/install.go index b8e34b6..2b7cfe5 100644 --- a/installer/cmd/install.go +++ b/installer/cmd/install.go @@ -30,7 +30,7 @@ var ( shellName string installBrew bool installShellWithBrew bool - multiUserSystem bool + gitCloneProtocol string verbose bool installPrerequisites bool @@ -243,14 +243,13 @@ func installHomebrew(sysInfo compatibility.SystemInfo, log logger.Logger) (strin // Create BrewInstaller using the new API. installer := brew.NewBrewInstaller(brew.Options{ - SystemInfo: &sysInfo, - Logger: cliLogger, - Commander: globalCommander, - HTTPClient: globalHttpClient, - OsManager: globalOsManager, - Fs: globalFilesystem, - MultiUserSystem: multiUserSystem, - DisplayMode: GetDisplayMode(), + SystemInfo: &sysInfo, + Logger: cliLogger, + Commander: globalCommander, + HTTPClient: globalHttpClient, + OsManager: globalOsManager, + Fs: globalFilesystem, + DisplayMode: GetDisplayMode(), }) log.StartProgress("Checking Homebrew availability") @@ -466,9 +465,7 @@ func initDotfilesManagerData(dm dotfilesmanager.DotfilesManager) error { LastName: "Gruber", Email: "timor.gruber@gmail.com", SystemData: mo.Some(dotfilesmanager.DotfilesSystemData{ - Shell: shellName, - MultiUserSystem: multiUserSystem, - BrewMultiUser: "linuxbrew-manager", + Shell: shellName, }), } @@ -515,11 +512,8 @@ func init() { "Install brew if not already installed") installCmd.Flags().BoolVar(&installShellWithBrew, "install-shell-with-brew", true, "Install shell with brew if not already installed") - installCmd.Flags().BoolVar(&multiUserSystem, "multi-user-system", false, - "Treat this system as a multi-user system (affects some dotfiles)") installCmd.Flags().StringVar(&gitCloneProtocol, "git-clone-protocol", "https", "Use the given git clone protocol (ssh or https) for git operations") - installCmd.Flags().BoolVar(&installPrerequisites, "install-prerequisites", false, "Automatically install missing prerequisites") @@ -529,7 +523,6 @@ func init() { viper.BindPFlag("shell", installCmd.Flags().Lookup("shell")) viper.BindPFlag("install-brew", installCmd.Flags().Lookup("install-brew")) viper.BindPFlag("install-shell-with-brew", installCmd.Flags().Lookup("install-shell-with-brew")) - viper.BindPFlag("multi-user-system", installCmd.Flags().Lookup("multi-user-system")) viper.BindPFlag("git-clone-protocol", installCmd.Flags().Lookup("git-clone-protocol")) viper.BindPFlag("install-prerequisites", installCmd.Flags().Lookup("install-prerequisites")) diff --git a/installer/lib/brew/installer.go b/installer/lib/brew/installer.go index c1f5ed6..7a45dd3 100644 --- a/installer/lib/brew/installer.go +++ b/installer/lib/brew/installer.go @@ -18,9 +18,6 @@ import ( ) const ( - // BrewUserOnMultiUserSystem is the username used for Homebrew on multi-user systems. - BrewUserOnMultiUserSystem = "linuxbrew-manager" - // LinuxBrewPath is the default installation path for Homebrew on Linux. LinuxBrewPath = "/home/linuxbrew/.linuxbrew/bin/brew" @@ -100,18 +97,9 @@ type brewInstaller struct { var _ BrewInstaller = (*brewInstaller)(nil) -// MultiUserBrewInstaller implements BrewInstaller for multi-user systems. -// It composes a regular brewInstaller and adds multi-user logic. -type MultiUserBrewInstaller struct { - *brewInstaller - brewUser string -} - -var _ BrewInstaller = (*MultiUserBrewInstaller)(nil) - // NewBrewInstaller creates a new BrewInstaller with the given options. func NewBrewInstaller(opts Options) BrewInstaller { - base := &brewInstaller{ + return &brewInstaller{ logger: opts.Logger, systemInfo: opts.SystemInfo, commander: opts.Commander, @@ -121,15 +109,6 @@ func NewBrewInstaller(opts Options) BrewInstaller { brewPathOverride: opts.BrewPathOverride, displayMode: opts.DisplayMode, } - - if opts.MultiUserSystem { - return &MultiUserBrewInstaller{ - brewInstaller: base, - brewUser: BrewUserOnMultiUserSystem, - } - } - - return base } // DetectBrewPath returns the appropriate brew binary path based on the system information. @@ -154,7 +133,7 @@ func (b *brewInstaller) IsAvailable() (bool, error) { return exists, nil } -// Install installs Homebrew if not already installed (single-user). +// Install installs Homebrew if not already installed. func (b *brewInstaller) Install() error { isAvailable, err := b.IsAvailable() if err != nil { @@ -177,7 +156,7 @@ func (b *brewInstaller) Install() error { } b.logger.Debug("Installing Homebrew") - err = b.installHomebrew("") + err = b.installHomebrew() if err != nil { return err } @@ -230,124 +209,8 @@ func (b *brewInstaller) validateInstall() error { return nil } -// Install installs Homebrew if not already installed (multi-user). -func (m *MultiUserBrewInstaller) Install() error { - isAvailable, err := m.IsAvailable() - if err != nil { - return fmt.Errorf("failed checking Homebrew availability: %w", err) - } - - if isAvailable { - m.logger.Debug("Homebrew is already installed (multi-user)") - - // Update PATH to include brew binaries so installed tools can be found - brewPath, err := m.DetectBrewPath() - if err != nil { - return fmt.Errorf("failed to detect brew path for PATH update: %w", err) - } - if err := UpdatePathWithBrewBinaries(brewPath); err != nil { - m.logger.Warning("Failed to update PATH with brew binaries: %v", err) - } - - return nil - } - - m.logger.Debug("Installing Homebrew (multi-user)") - if m.systemInfo.OSName == "darwin" { - return fmt.Errorf("multi-user Homebrew installation is not supported on macOS, please install manually") - } - - err = m.installMultiUserLinux() - if err != nil { - return err - } - - // Self-validation: check that brew is available and works - if err := m.validateInstall(); err != nil { - return fmt.Errorf("brew self-validation failed: %w", err) - } - - // Update PATH to include brew binaries so installed tools can be found - brewPath, err := m.DetectBrewPath() - if err != nil { - return fmt.Errorf("failed to detect brew path for PATH update: %w", err) - } - if err := UpdatePathWithBrewBinaries(brewPath); err != nil { - m.logger.Warning("Failed to update PATH with brew binaries: %v", err) - } - - return nil -} - -// Multi-user overrides. -// IsAvailable checks if Homebrew is already installed and available (multi-user). -func (m *MultiUserBrewInstaller) IsAvailable() (bool, error) { - m.logger.Debug("Checking if Homebrew is available") - - brewPath, err := m.DetectBrewPath() - if err != nil { - return false, err - } - - exists, err := m.fs.PathExists(brewPath) - if err != nil { - return false, err - } - if !exists { - return false, nil - } - - // For Linux multi-user systems, check file ownership - if m.systemInfo.OSName == "linux" { - brewPathOwner, err := m.osManager.GetFileOwner(brewPath) - if err != nil { - return false, fmt.Errorf("failed to get owner of brew binary: %w", err) - } - - return brewPathOwner == m.brewUser, nil - } - - return true, nil -} - -// installMultiUserLinux installs Homebrew in a multi-user configuration on Linux. -func (m *MultiUserBrewInstaller) installMultiUserLinux() error { - brewUser := BrewUserOnMultiUserSystem - brewHome := "/home/linuxbrew" - - // 1. Check if user exists and create if needed - exists, err := m.osManager.UserExists(brewUser) - if err != nil { - return fmt.Errorf("error checking if user '%s' exists: %w", brewUser, err) - } - - if !exists { - if err := m.osManager.AddUser(brewUser); err != nil { - return fmt.Errorf("error creating user '%s': %w", brewUser, err) - } - } - - // 2. Add user to sudo group - if err := m.osManager.AddUserToGroup(brewUser, "sudo"); err != nil { - m.logger.Debug("Note: Failed to add user to sudo group, continuing anyway") - } - - // 3. Add passwordless sudo for brew user - if err := m.osManager.AddSudoAccess(brewUser); err != nil { - return fmt.Errorf("failed to add sudo access for user '%s': %w", brewUser, err) - } - - // 4. Set ownership of homebrew directory - if err := m.osManager.SetOwnership(brewHome, brewUser); err != nil { - return fmt.Errorf("failed to set ownership of '%s' to '%s': %w", brewHome, brewUser, err) - } - - // 5. Install Homebrew as the brew user - return m.installHomebrew(brewUser) -} - -// installHomebrew handles both regular and multi-user Homebrew installations. -func (b *brewInstaller) installHomebrew(asUser string) error { +// installHomebrew handles Homebrew installation. +func (b *brewInstaller) installHomebrew() error { // Download and prepare the installation script installScriptPath, cleanup, err := b.downloadAndPrepareInstallScript() if err != nil { @@ -366,37 +229,21 @@ func (b *brewInstaller) installHomebrew(asUser string) error { b.logger.Debug("Successfully downloaded Homebrew install script") b.logger.Trace("Homebrew install script downloaded to %s", installScriptPath) - // Execute the downloaded install script, optionally as a different user - if asUser != "" { - b.logger.Debug("Running Homebrew install script as %s", asUser) - - var discardOutputOption utils.Option = utils.EmptyOption() - if b.displayMode != utils.DisplayModePassthrough { - discardOutputOption = utils.WithDiscardOutput() - } + // Execute the downloaded install script + b.logger.Debug("Running Homebrew install script") - _, err := b.commander.RunCommand("sudo", []string{"-Hu", asUser, "bash", installScriptPath}, discardOutputOption) - if err != nil { - return fmt.Errorf("failed running Homebrew install script as %s: %w", asUser, err) - } - - b.logger.Debug("Successfully installed Homebrew for user %s", asUser) - } else { - b.logger.Debug("Running Homebrew install script") - - var discardOutputOption utils.Option = utils.EmptyOption() - if b.displayMode != utils.DisplayModePassthrough { - discardOutputOption = utils.WithDiscardOutput() - } - - _, err := b.commander.RunCommand("/bin/bash", []string{installScriptPath}, discardOutputOption, utils.WithEnv(map[string]string{"NONINTERACTIVE": "1"})) - if err != nil { - return err - } + var discardOutputOption utils.Option = utils.EmptyOption() + if b.displayMode != utils.DisplayModePassthrough { + discardOutputOption = utils.WithDiscardOutput() + } - b.logger.Debug("Homebrew installed successfully") + _, err = b.commander.RunCommand("/bin/bash", []string{installScriptPath}, discardOutputOption, utils.WithEnv(map[string]string{"NONINTERACTIVE": "1"})) + if err != nil { + return err } + b.logger.Debug("Homebrew installed successfully") + return nil } @@ -456,7 +303,6 @@ func (b *brewInstaller) downloadAndPrepareInstallScript() (string, func(), error // Options holds configuration options for Homebrew operations. type Options struct { Logger logger.Logger - MultiUserSystem bool SystemInfo *compatibility.SystemInfo Commander utils.Commander HTTPClient httpclient.HTTPClient @@ -469,7 +315,6 @@ type Options struct { // DefaultOptions returns the default options. func DefaultOptions() *Options { return &Options{ - MultiUserSystem: false, Logger: logger.DefaultLogger, SystemInfo: nil, Commander: utils.NewDefaultCommander(logger.DefaultLogger), @@ -492,12 +337,6 @@ func (o *Options) WithLogger(log logger.Logger) *Options { return o } -// WithMultiUserSystem configures for multi-user system operation. -func (o *Options) WithMultiUserSystem(multiUser bool) *Options { - o.MultiUserSystem = multiUser - return o -} - // WithSystemInfo sets system information for brew operations. func (o *Options) WithSystemInfo(sysInfo *compatibility.SystemInfo) *Options { o.SystemInfo = sysInfo diff --git a/installer/lib/brew/installer_test.go b/installer/lib/brew/installer_test.go index 45c0b52..651ba6a 100644 --- a/installer/lib/brew/installer_test.go +++ b/installer/lib/brew/installer_test.go @@ -65,32 +65,14 @@ func Test_DetectBrewPath_UsesOverride_WhenProvided(t *testing.T) { /* Installer Constructor Tests */ /* ------------------------------------------------------------------------------------------------------------------ */ -func Test_NewBrewInstaller_CreatesMultiUserImplementation_WhenOptionIsEnabled(t *testing.T) { - opts := brew.Options{ - MultiUserSystem: true, - Logger: logger.DefaultLogger, - SystemInfo: &compatibility.SystemInfo{OSName: "linux", Arch: "amd64"}, - Commander: nil, - } - installer := brew.NewBrewInstaller(opts) - require.NotNil(t, installer) - - _, isMultiUser := installer.(*brew.MultiUserBrewInstaller) - require.True(t, isMultiUser) -} - func Test_NewBrewInstaller_CreatesSingleUserImplementation_ByDefault(t *testing.T) { opts := brew.Options{ - MultiUserSystem: false, - Logger: logger.DefaultLogger, - SystemInfo: &compatibility.SystemInfo{OSName: "linux", Arch: "amd64"}, - Commander: nil, + Logger: logger.DefaultLogger, + SystemInfo: &compatibility.SystemInfo{OSName: "linux", Arch: "amd64"}, + Commander: nil, } installer := brew.NewBrewInstaller(opts) require.NotNil(t, installer) - - _, isMultiUser := installer.(*brew.MultiUserBrewInstaller) - require.False(t, isMultiUser) } /* ------------------------------------------------------------------------------------------------------------------ */ @@ -121,7 +103,6 @@ func Test_SingleUserBrew_ReportsAvailable_WhenPathExists(t *testing.T) { HTTPClient: mockHTTP, OsManager: mockOsManager, Fs: mockFS, - MultiUserSystem: false, BrewPathOverride: expectedBrewPath, } @@ -156,7 +137,6 @@ func Test_SingleUserBrew_ReportsUnavailable_WhenPathDoesNotExist(t *testing.T) { HTTPClient: mockHTTP, OsManager: mockOsManager, Fs: mockFS, - MultiUserSystem: false, BrewPathOverride: expectedBrewPath, } @@ -198,7 +178,6 @@ func Test_SingleUserBrew_IsNotReinstalled_WhenAvailable(t *testing.T) { HTTPClient: mockHTTP, OsManager: mockOsManager, Fs: mockFS, - MultiUserSystem: false, BrewPathOverride: expectedBrewPath, } @@ -290,7 +269,6 @@ func Test_SingleUserBrew_InstallsSuccessfully_WhenNotAvailable(t *testing.T) { HTTPClient: mockHTTP, OsManager: mockOsManager, Fs: mockFS, - MultiUserSystem: false, BrewPathOverride: expectedBrewPath, } @@ -368,7 +346,6 @@ func Test_SingleUserBrew_FailsInstallation_WhenDownloadingInstallationScriptFail HTTPClient: mockHTTP, OsManager: mockOsManager, Fs: mockFS, - MultiUserSystem: false, BrewPathOverride: "/opt/homebrew/bin/brew", } @@ -414,7 +391,6 @@ func Test_SingleUserBrew_FailsInstallation_WhenFailingToCreateTempFileHoldingDow HTTPClient: mockHTTP, OsManager: mockOsManager, Fs: mockFS, - MultiUserSystem: false, BrewPathOverride: "/opt/homebrew/bin/brew", } @@ -504,7 +480,6 @@ func Test_SingleUserBrew_FailsInstallation_WhenFailingToCopyDownloadedScriptFrom HTTPClient: mockHTTP, OsManager: mockOsManager, Fs: mockFS, - MultiUserSystem: false, BrewPathOverride: expectedBrewPath, } @@ -530,7 +505,7 @@ func Test_DefaultOptions_CreatesNonEmptyConfiguration(t *testing.T) { require.NotNil(t, opts.HTTPClient) require.NotNil(t, opts.OsManager) require.NotNil(t, opts.Fs) - require.False(t, opts.MultiUserSystem) + require.Nil(t, opts.SystemInfo) require.Empty(t, opts.BrewPathOverride) } @@ -539,26 +514,30 @@ func Test_OptionsBuilderPattern_ConfiguresAllOptions(t *testing.T) { customLogger := &logger.NoopLogger{} customSystemInfo := &compatibility.SystemInfo{OSName: "linux", Arch: "amd64"} customCommander := &utils.MoqCommander{} - customHTTP := &httpclient.MoqHTTPClient{} + customHTTPClient := &httpclient.MoqHTTPClient{} customOsManager := &osmanager.MoqOsManager{} customFS := &utils.MoqFileSystem{} + customBrewPathOverride := "/custom/path/to/brew" + customDisplayMode := utils.DisplayModePassthrough opts := brew.DefaultOptions(). WithLogger(customLogger). - WithMultiUserSystem(true). WithSystemInfo(customSystemInfo). WithCommander(customCommander). - WithHTTPClient(customHTTP). + WithHTTPClient(customHTTPClient). WithOsManager(customOsManager). - WithFileSystem(customFS) + WithFileSystem(customFS). + WithBrewPathOverride(customBrewPathOverride). + WithDisplayMode(customDisplayMode) require.Equal(t, customLogger, opts.Logger) - require.True(t, opts.MultiUserSystem) require.Equal(t, customSystemInfo, opts.SystemInfo) require.Equal(t, customCommander, opts.Commander) - require.Equal(t, customHTTP, opts.HTTPClient) + require.Equal(t, customHTTPClient, opts.HTTPClient) require.Equal(t, customOsManager, opts.OsManager) require.Equal(t, customFS, opts.Fs) + require.Equal(t, customBrewPathOverride, opts.BrewPathOverride) + require.Equal(t, customDisplayMode, opts.DisplayMode) } /* ------------------------------------------------------------------------------------------------------------------ */ @@ -612,7 +591,6 @@ func Test_SingleUserBrew_HandlesEmptyInstallScript(t *testing.T) { HTTPClient: mockHTTP, OsManager: mockOsManager, Fs: mockFS, - MultiUserSystem: false, BrewPathOverride: "/opt/homebrew/bin/brew", } @@ -698,7 +676,6 @@ func Test_SingleUserBrew_CanHandleLargeInstallScript(t *testing.T) { HTTPClient: mockHTTP, OsManager: mockOsManager, Fs: mockFS, - MultiUserSystem: false, BrewPathOverride: "/opt/homebrew/bin/brew", } @@ -780,7 +757,6 @@ func Test_SingleUserBrew_Install_UpdatesPath_AfterSuccessfulInstallation(t *test HTTPClient: mockHTTPClient, OsManager: mockOsManager, Fs: mockFS, - MultiUserSystem: false, BrewPathOverride: brewPath, } @@ -894,454 +870,9 @@ func Test_UpdatePathWithBrewBinaries_UsesPlatformPathSeparator(t *testing.T) { } /* ------------------------------------------------------------------------------------------------------------------ */ -/* Multi-User Brew Installer Tests */ +/* Display Mode Tests */ /* ------------------------------------------------------------------------------------------------------------------ */ -func Test_MultiUserBrew_ReportsAvailable_WhenBrewExistsForBrewUser(t *testing.T) { - expectedBrewPath := "/home/linuxbrew/.linuxbrew/bin/brew" - - // Create mock dependencies - mockLogger := &logger.NoopLogger{} - mockCommander := &utils.MoqCommander{} - mockFS := &utils.MoqFileSystem{ - PathExistsFunc: func(path string) (bool, error) { - if path == expectedBrewPath { - return true, nil // Simulate that brew exists for the brew user - } - return false, nil // Other paths do not exist - }, - } - mockHTTP := &httpclient.MoqHTTPClient{} - mockOsManager := &osmanager.MoqOsManager{ - GetFileOwnerFunc: func(path string) (string, error) { - if path == expectedBrewPath { - return brew.BrewUserOnMultiUserSystem, nil // Simulate that the brew user owns the brew binary - } - return "", fmt.Errorf("unexpected path: %s", path) - }, - } - - opts := brew.Options{ - Logger: mockLogger, - SystemInfo: &compatibility.SystemInfo{OSName: "linux", Arch: "amd64"}, - Commander: mockCommander, - HTTPClient: mockHTTP, - OsManager: mockOsManager, - Fs: mockFS, - MultiUserSystem: true, - BrewPathOverride: expectedBrewPath, - } - - installer := brew.NewBrewInstaller(opts) - available, err := installer.IsAvailable() - - require.NoError(t, err) - require.True(t, available) -} - -func Test_MultiUserBrew_ReportsUnavailable_WhenBrewDoesNotExistForBrewUser(t *testing.T) { - expectedBrewPath := "/home/linuxbrew/.linuxbrew/bin/brew" - - // Create mock dependencies - mockLogger := &logger.NoopLogger{} - mockCommander := &utils.MoqCommander{} - mockFS := &utils.MoqFileSystem{ - PathExistsFunc: func(path string) (bool, error) { - if path == expectedBrewPath { - return true, nil // Simulate that brew path exists - } - return true, nil // Other paths exist - }, - } - mockHTTP := &httpclient.MoqHTTPClient{} - mockOsManager := &osmanager.MoqOsManager{ - GetFileOwnerFunc: func(path string) (string, error) { - if path == expectedBrewPath { - return "someotheruser", nil // Simulate that the brew binary is owned by a different user - } - return "", fmt.Errorf("unexpected path: %s", path) - }, - } - - opts := brew.Options{ - Logger: mockLogger, - SystemInfo: &compatibility.SystemInfo{OSName: "linux", Arch: "amd64"}, - Commander: mockCommander, - HTTPClient: mockHTTP, - OsManager: mockOsManager, - Fs: mockFS, - MultiUserSystem: true, - BrewPathOverride: expectedBrewPath, - } - - installer := brew.NewBrewInstaller(opts) - available, err := installer.IsAvailable() - - require.NoError(t, err) - require.False(t, available) -} - -func Test_MultiUserBrew_ReportedUnavailable_WhenBrewPathDoesNotExist(t *testing.T) { - expectedBrewPath := "/nonexistent/brew" - - // Create mock dependencies - mockLogger := &logger.NoopLogger{} - mockCommander := &utils.MoqCommander{} - mockFS := &utils.MoqFileSystem{ - PathExistsFunc: func(path string) (bool, error) { - if path == expectedBrewPath { - return false, nil // Simulate that brew does not exist - } - return true, nil // Other paths exist - }, - } - mockHTTP := &httpclient.MoqHTTPClient{} - mockOsManager := &osmanager.MoqOsManager{} - - opts := brew.Options{ - Logger: mockLogger, - SystemInfo: &compatibility.SystemInfo{OSName: "linux", Arch: "amd64"}, - Commander: mockCommander, - HTTPClient: mockHTTP, - OsManager: mockOsManager, - Fs: mockFS, - MultiUserSystem: true, - BrewPathOverride: expectedBrewPath, - } - - installer := brew.NewBrewInstaller(opts) - available, err := installer.IsAvailable() - - require.NoError(t, err) // Multi-user installation should error on non-existent path - require.False(t, available) // Brew should be reported as unavailable -} - -func Test_MultiUserBrew_DoesNotReinstall_WhenAlreadyAvailable(t *testing.T) { - expectedBrewPath := "/home/linuxbrew/.linuxbrew/bin/brew" - - // Create mock dependencies - mockLogger := &logger.NoopLogger{} - mockCommander := &utils.MoqCommander{ - RunCommandFunc: func(name string, args []string, opts ...utils.Option) (*utils.Result, error) { - if name == expectedBrewPath && len(args) == 1 && args[0] == "--version" { - return &utils.Result{ExitCode: 0}, nil // Validation successful - } - return nil, fmt.Errorf("unexpected command: %s %v", name, args) - }, - } - mockFS := &utils.MoqFileSystem{ - PathExistsFunc: func(path string) (bool, error) { - if path == expectedBrewPath { - return true, nil // Simulate that brew exists for the brew user - } - return false, nil // Other paths do not exist - }, - } - mockHTTP := &httpclient.MoqHTTPClient{} - mockOsManager := &osmanager.MoqOsManager{ - AddUserFunc: func(username string) error { - if username == brew.BrewUserOnMultiUserSystem { - return nil // Simulate successful user addition - } - return fmt.Errorf("unexpected user: %s", username) - }, - UserExistsFunc: func(username string) (bool, error) { - if username == brew.BrewUserOnMultiUserSystem { - return true, nil // Simulate that the brew user exists - } - return false, nil // Other users do not exist - }, - GetFileOwnerFunc: func(path string) (string, error) { - if path == expectedBrewPath { - return brew.BrewUserOnMultiUserSystem, nil // Simulate that the brew user owns the brew binary - } - return "", fmt.Errorf("unexpected path: %s", path) - }, - } - - opts := brew.Options{ - Logger: mockLogger, - SystemInfo: &compatibility.SystemInfo{OSName: "linux", Arch: "amd64"}, - Commander: mockCommander, - HTTPClient: mockHTTP, - OsManager: mockOsManager, - Fs: mockFS, - MultiUserSystem: true, - BrewPathOverride: expectedBrewPath, - } - - installer := brew.NewBrewInstaller(opts) - err := installer.Install() - - require.NoError(t, err) - - // Verify no HTTP requests were made (no download should happen) - require.Empty(t, mockHTTP.GetCalls()) - // Verify no user management operations were performed - require.Empty(t, mockOsManager.UserExistsCalls()) - require.Empty(t, mockOsManager.AddUserCalls()) -} - -//gocognit:ignore -//nolint:cyclop // This test is complex due to the multi-user setup and various checks involved. -func Test_MultiUserBrew_InstallsFromScratch_WhenUserDoesNotExist(t *testing.T) { - expectedBrewPath := "/home/linuxbrew/.linuxbrew/bin/brew" - tempScriptPath := "/tmp/brew-install-12345.sh" - installScript := "#!/bin/bash\necho 'Installing Homebrew...'" - - mockCommander := &utils.MoqCommander{ - RunCommandFunc: func(name string, args []string, opts ...utils.Option) (*utils.Result, error) { - if name == "sudo" && len(args) == 4 && args[0] == "-Hu" && - args[1] == brew.BrewUserOnMultiUserSystem && - args[2] == "bash" && args[3] == tempScriptPath { - return &utils.Result{ExitCode: 0}, nil - } - if name == expectedBrewPath && len(args) == 1 && args[0] == "--version" { - return &utils.Result{ExitCode: 0}, nil - } - return nil, fmt.Errorf("unexpected command: %s %v", name, args) - }, - } - - pathExistsCalls := 0 - mockFS := &utils.MoqFileSystem{ - PathExistsFunc: func(path string) (bool, error) { - pathExistsCalls++ - if path == expectedBrewPath { - return pathExistsCalls > 1, nil - } - if path == tempScriptPath { - return true, nil - } - return false, nil - }, - CreateTemporaryFileFunc: func(dir, pattern string) (string, error) { - return tempScriptPath, nil - }, - WriteFileFunc: func(path string, reader io.Reader) (int64, error) { - if path == tempScriptPath { - return int64(len(installScript)), nil - } - return 0, fmt.Errorf("unexpected path: %s", path) - }, - RemovePathFunc: func(path string) error { - return nil - }, - } - - mockHTTP := &httpclient.MoqHTTPClient{ - GetFunc: func(url string) (*http.Response, error) { - if url == "https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh" { - return &http.Response{ - StatusCode: http.StatusOK, - Body: io.NopCloser(bytes.NewBufferString(installScript)), - }, nil - } - return nil, fmt.Errorf("unexpected URL: %s", url) - }, - } - - mockOsManager := &osmanager.MoqOsManager{ - UserExistsFunc: func(username string) (bool, error) { - if username == brew.BrewUserOnMultiUserSystem { - return false, nil // User does not exist - } - return false, fmt.Errorf("unexpected user check: %s", username) - }, - AddUserFunc: func(username string) error { - if username == brew.BrewUserOnMultiUserSystem { - return nil - } - return fmt.Errorf("unexpected user add: %s", username) - }, - AddUserToGroupFunc: func(username, group string) error { - if username == brew.BrewUserOnMultiUserSystem && group == "sudo" { - return nil - } - return fmt.Errorf("unexpected user/group add: %s/%s", username, group) - }, - AddSudoAccessFunc: func(username string) error { - if username == brew.BrewUserOnMultiUserSystem { - return nil - } - return fmt.Errorf("unexpected sudo access for: %s", username) - }, - SetOwnershipFunc: func(path, username string) error { - if path == "/home/linuxbrew" && username == brew.BrewUserOnMultiUserSystem { - return nil - } - return fmt.Errorf("unexpected ownership set: %s for %s", path, username) - }, - SetPermissionsFunc: func(path string, perms os.FileMode) error { - if path == tempScriptPath && perms == 0o755 { - return nil - } - return fmt.Errorf("unexpected permission call: %s %o", path, perms) - }, - GetFileOwnerFunc: func(path string) (string, error) { - if path == expectedBrewPath { - return brew.BrewUserOnMultiUserSystem, nil - } - return "", fmt.Errorf("unexpected get owner for: %s", path) - }, - } - - opts := brew.Options{ - Logger: &logger.NoopLogger{}, - SystemInfo: &compatibility.SystemInfo{OSName: "linux", Arch: "amd64"}, - Commander: mockCommander, - HTTPClient: mockHTTP, - OsManager: mockOsManager, - Fs: mockFS, - MultiUserSystem: true, - BrewPathOverride: expectedBrewPath, - } - - installer := brew.NewBrewInstaller(opts) - err := installer.Install() - - require.NoError(t, err) - require.Len(t, mockOsManager.UserExistsCalls(), 1) - require.Len(t, mockOsManager.AddUserCalls(), 1) // User should be added - require.Len(t, mockOsManager.AddUserToGroupCalls(), 1) - require.Len(t, mockOsManager.AddSudoAccessCalls(), 1) - require.Len(t, mockOsManager.SetOwnershipCalls(), 1) - require.Len(t, mockHTTP.GetCalls(), 1) - require.Len(t, mockFS.CreateTemporaryFileCalls(), 1) - require.Len(t, mockFS.WriteFileCalls(), 1) - require.Len(t, mockOsManager.SetPermissionsCalls(), 1) - require.Len(t, mockCommander.RunCommandCalls(), 2) - require.Len(t, mockFS.RemovePathCalls(), 1) -} - -//nolint:cyclop // This test is complex due to the multi-user setup and various checks involved. -func Test_MultiUserBrew_InstallsFromScratch_WhenUserAlreadyExists(t *testing.T) { - expectedBrewPath := "/home/linuxbrew/.linuxbrew/bin/brew" - tempScriptPath := "/tmp/brew-install-12345.sh" - installScript := "#!/bin/bash\necho 'Installing Homebrew...'" - - mockCommander := &utils.MoqCommander{ - RunCommandFunc: func(name string, args []string, opts ...utils.Option) (*utils.Result, error) { - if name == "sudo" && len(args) == 4 && args[0] == "-Hu" && - args[1] == brew.BrewUserOnMultiUserSystem && - args[2] == "bash" && args[3] == tempScriptPath { - return &utils.Result{ExitCode: 0}, nil - } - if name == expectedBrewPath && len(args) == 1 && args[0] == "--version" { - return &utils.Result{ExitCode: 0}, nil - } - return nil, fmt.Errorf("unexpected command: %s %v", name, args) - }, - } - - pathExistsCalls := 0 - mockFS := &utils.MoqFileSystem{ - PathExistsFunc: func(path string) (bool, error) { - pathExistsCalls++ - if path == expectedBrewPath { - return pathExistsCalls > 1, nil - } - if path == tempScriptPath { - return true, nil - } - return false, nil - }, - CreateTemporaryFileFunc: func(dir, pattern string) (string, error) { - return tempScriptPath, nil - }, - WriteFileFunc: func(path string, reader io.Reader) (int64, error) { - if path == tempScriptPath { - return int64(len(installScript)), nil - } - return 0, fmt.Errorf("unexpected path: %s", path) - }, - RemovePathFunc: func(path string) error { - return nil - }, - } - - mockHTTP := &httpclient.MoqHTTPClient{ - GetFunc: func(url string) (*http.Response, error) { - if url == "https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh" { - return &http.Response{ - StatusCode: http.StatusOK, - Body: io.NopCloser(bytes.NewBufferString(installScript)), - }, nil - } - return nil, fmt.Errorf("unexpected URL: %s", url) - }, - } - - mockOsManager := &osmanager.MoqOsManager{ - UserExistsFunc: func(username string) (bool, error) { - if username == brew.BrewUserOnMultiUserSystem { - return true, nil // User already exists - } - return false, fmt.Errorf("unexpected user check: %s", username) - }, - AddUserFunc: func(username string) error { - return fmt.Errorf("unexpected user add: %s", username) // Should not be called - }, - AddUserToGroupFunc: func(username, group string) error { - if username == brew.BrewUserOnMultiUserSystem && group == "sudo" { - return nil - } - return fmt.Errorf("unexpected user/group add: %s/%s", username, group) - }, - AddSudoAccessFunc: func(username string) error { - if username == brew.BrewUserOnMultiUserSystem { - return nil - } - return fmt.Errorf("unexpected sudo access for: %s", username) - }, - SetOwnershipFunc: func(path, username string) error { - if path == "/home/linuxbrew" && username == brew.BrewUserOnMultiUserSystem { - return nil - } - return fmt.Errorf("unexpected ownership set: %s for %s", path, username) - }, - SetPermissionsFunc: func(path string, perms os.FileMode) error { - if path == tempScriptPath && perms == 0o755 { - return nil - } - return fmt.Errorf("unexpected permission call: %s %o", path, perms) - }, - GetFileOwnerFunc: func(path string) (string, error) { - if path == expectedBrewPath { - return brew.BrewUserOnMultiUserSystem, nil - } - return "", fmt.Errorf("unexpected get owner for: %s", path) - }, - } - - opts := brew.Options{ - Logger: &logger.NoopLogger{}, - SystemInfo: &compatibility.SystemInfo{OSName: "linux", Arch: "amd64"}, - Commander: mockCommander, - HTTPClient: mockHTTP, - OsManager: mockOsManager, - Fs: mockFS, - MultiUserSystem: true, - BrewPathOverride: expectedBrewPath, - } - - installer := brew.NewBrewInstaller(opts) - err := installer.Install() - - require.NoError(t, err) - require.Len(t, mockOsManager.UserExistsCalls(), 1) - require.Empty(t, mockOsManager.AddUserCalls()) // User should not be added - require.Len(t, mockOsManager.AddUserToGroupCalls(), 1) - require.Len(t, mockOsManager.AddSudoAccessCalls(), 1) - require.Len(t, mockOsManager.SetOwnershipCalls(), 1) - require.Len(t, mockHTTP.GetCalls(), 1) - require.Len(t, mockFS.CreateTemporaryFileCalls(), 1) - require.Len(t, mockFS.WriteFileCalls(), 1) - require.Len(t, mockOsManager.SetPermissionsCalls(), 1) - require.Len(t, mockCommander.RunCommandCalls(), 2) - require.Len(t, mockFS.RemovePathCalls(), 1) -} - func Test_SingleUserBrew_Install_DiscardsOutput_WhenDisplayModeIsNotPassthrough(t *testing.T) { mockLogger := &logger.NoopLogger{} tempScriptPath := "/tmp/brew-install-12345.sh" @@ -1421,14 +952,13 @@ func Test_SingleUserBrew_Install_DiscardsOutput_WhenDisplayModeIsNotPassthrough( } opts := brew.Options{ - Logger: mockLogger, - SystemInfo: &compatibility.SystemInfo{OSName: "darwin", Arch: "arm64"}, - Commander: mockCommander, - HTTPClient: mockHTTPClient, - OsManager: mockOsManager, - Fs: mockFS, - MultiUserSystem: false, - DisplayMode: utils.DisplayModeProgress, // Non-passthrough mode + Logger: mockLogger, + SystemInfo: &compatibility.SystemInfo{OSName: "darwin", Arch: "arm64"}, + Commander: mockCommander, + HTTPClient: mockHTTPClient, + OsManager: mockOsManager, + Fs: mockFS, + DisplayMode: utils.DisplayModeProgress, // Non-passthrough mode } installer := brew.NewBrewInstaller(opts) @@ -1516,14 +1046,13 @@ func Test_SingleUserBrew_Install_DoesNotDiscardOutput_WhenDisplayModeIsPassthrou } opts := brew.Options{ - Logger: mockLogger, - SystemInfo: &compatibility.SystemInfo{OSName: "darwin", Arch: "arm64"}, - Commander: mockCommander, - HTTPClient: mockHTTPClient, - OsManager: mockOsManager, - Fs: mockFS, - MultiUserSystem: false, - DisplayMode: utils.DisplayModePassthrough, // Passthrough mode + Logger: mockLogger, + SystemInfo: &compatibility.SystemInfo{OSName: "darwin", Arch: "arm64"}, + Commander: mockCommander, + HTTPClient: mockHTTPClient, + OsManager: mockOsManager, + Fs: mockFS, + DisplayMode: utils.DisplayModePassthrough, // Passthrough mode } installer := brew.NewBrewInstaller(opts) diff --git a/installer/lib/dotfilesmanager/chezmoi/data.go b/installer/lib/dotfilesmanager/chezmoi/data.go index 71a2088..ff99bb4 100644 --- a/installer/lib/dotfilesmanager/chezmoi/data.go +++ b/installer/lib/dotfilesmanager/chezmoi/data.go @@ -55,8 +55,6 @@ func (c *ChezmoiManager) Initialize(data dotfilesmanager.DotfilesData) error { data.SystemData.MapValue(func(value dotfilesmanager.DotfilesSystemData) dotfilesmanager.DotfilesSystemData { viperObject.Set("data.system.shell", value.Shell) - viperObject.Set("data.system.multi_user_system", value.MultiUserSystem) - viperObject.Set("data.system.brew_multi_user", value.BrewMultiUser) if value.GenericWorkProfile.IsPresent() { viperObject.Set("data.system.work_generic_dotfiles_profile", value.GenericWorkProfile) diff --git a/installer/lib/dotfilesmanager/chezmoi/data_test.go b/installer/lib/dotfilesmanager/chezmoi/data_test.go index 04e0497..58e1679 100644 --- a/installer/lib/dotfilesmanager/chezmoi/data_test.go +++ b/installer/lib/dotfilesmanager/chezmoi/data_test.go @@ -281,9 +281,7 @@ func Test_Initialize_WritesSystemData_WhenProvided(t *testing.T) { manager := chezmoi.NewChezmoiManager(logger.DefaultLogger, fileSystem, mockUserManager, mockCommander, mockPackageManager, mockHTTPClient, utils.DisplayModeProgress, config) systemData := dotfilesmanager.DotfilesSystemData{ - Shell: "/bin/zsh", - MultiUserSystem: true, - BrewMultiUser: "multifoo", + Shell: "/bin/zsh", } data := dotfilesmanager.DotfilesData{ Email: "test@example.com", @@ -303,9 +301,6 @@ func Test_Initialize_WritesSystemData_WhenProvided(t *testing.T) { require.NoError(t, err) configStr := string(configContent) require.Contains(t, configStr, "/bin/zsh") - require.Contains(t, configStr, "multi_user_system = true") - require.Contains(t, configStr, "brew_multi_user") - require.Contains(t, configStr, "multifoo") } func Test_Initialize_WritesCompleteData_WhenAllFieldsProvided(t *testing.T) { @@ -336,8 +331,6 @@ func Test_Initialize_WritesCompleteData_WhenAllFieldsProvided(t *testing.T) { } systemData := dotfilesmanager.DotfilesSystemData{ Shell: "/bin/zsh", - MultiUserSystem: true, - BrewMultiUser: "multifoo", GenericWorkProfile: mo.Some(path.Join("/home", "user", ".work", "profile")), SpecificWorkProfile: mo.Some(path.Join("/home", "user", ".work", "foobar", "profile")), } @@ -365,7 +358,4 @@ func Test_Initialize_WritesCompleteData_WhenAllFieldsProvided(t *testing.T) { require.Contains(t, configStr, "Acme Corp") require.Contains(t, configStr, "john.doe@acme.com") require.Contains(t, configStr, "/bin/zsh") - require.Contains(t, configStr, "multi_user_system = true") - require.Contains(t, configStr, "brew_multi_user") - require.Contains(t, configStr, "multifoo") } diff --git a/installer/lib/dotfilesmanager/data.go b/installer/lib/dotfilesmanager/data.go index 4a87c03..cf4bdd0 100644 --- a/installer/lib/dotfilesmanager/data.go +++ b/installer/lib/dotfilesmanager/data.go @@ -20,8 +20,6 @@ type DotfilesWorkEnvData struct { type DotfilesSystemData struct { Shell string `mapstructure:"shell"` - MultiUserSystem bool `mapstructure:"multi_user_system"` - BrewMultiUser string `mapstructure:"brew_multi_user"` GenericWorkProfile mo.Option[string] `mapstructure:"generic_work_profile"` SpecificWorkProfile mo.Option[string] `mapstructure:"specific_work_profile"` } From b9b48fbc5786b3b48f26f1de96bb4c5f1ea9a568 Mon Sep 17 00:00:00 2001 From: Timor Gruber Date: Sun, 23 Nov 2025 18:34:10 +0200 Subject: [PATCH 2/3] remove references to chezmoi's data multi-user entry --- dot_zprofile.tmpl | 4 ---- dot_zshenv.tmpl | 4 ---- dot_zshrc.tmpl | 4 ---- 3 files changed, 12 deletions(-) diff --git a/dot_zprofile.tmpl b/dot_zprofile.tmpl index 16a0a21..dce999c 100644 --- a/dot_zprofile.tmpl +++ b/dot_zprofile.tmpl @@ -17,10 +17,6 @@ if [[ ! -v BREW_LOADED || "$BREW_LOADED" == "false" ]]; then # Load (home)brew eval "$("$BREW_BINARY" shellenv)" {{- end -}} - {{ if .system.multi_user_system -}} - # Impersonate brew management user - alias brew="sudo -Hu {{ .system.brew_multi_user }} $BREW_BINARY" - {{- end }} fi BREW_LOADED=true diff --git a/dot_zshenv.tmpl b/dot_zshenv.tmpl index 0c9f88d..f4e2a8d 100644 --- a/dot_zshenv.tmpl +++ b/dot_zshenv.tmpl @@ -32,10 +32,6 @@ if [[ ! -v BREW_LOADED || "$BREW_LOADED" == "false" ]]; then load_brew_env {{- end -}} {{- end -}} - {{- if .system.multi_user_system }} - # Impersonate brew management user - alias brew="sudo -Hu {{ .system.brew_multi_user }} $BREW_BINARY" - {{- end }} fi BREW_LOADED=true diff --git a/dot_zshrc.tmpl b/dot_zshrc.tmpl index 3a9eb2e..8a4f3b0 100644 --- a/dot_zshrc.tmpl +++ b/dot_zshrc.tmpl @@ -26,11 +26,7 @@ zstyle ':completion:*' menu select mkdir -p ~/.zfunc &>/dev/null fpath+=~/.zfunc -{{ if .system.multi_user_system -}} -autoload -U +X compinit && compinit -u -{{- else -}} autoload -U +X compinit && compinit -{{- end }} autoload -U +X bashcompinit && bashcompinit autoload -U select-word-style From 9ee101b4c8e248ae3a9a3eb81299a5f09643a1c9 Mon Sep 17 00:00:00 2001 From: Timor Gruber Date: Sun, 23 Nov 2025 18:56:34 +0200 Subject: [PATCH 3/3] add ability to apply dotfiles for custom branches This is critical to get this PR passing CI, as our CI currently tries to apply dotfiles from `main`, but they're incompatible with our installer as they reference data that isn't there anymore. We need the CI to use the current branch it's running on for full compatibility. --- .github/workflows/installer-ci.yml | 32 +++++++++++++- installer/cmd/install.go | 7 ++- .../lib/dotfilesmanager/chezmoi/apply.go | 3 ++ .../lib/dotfilesmanager/chezmoi/apply_test.go | 44 ++++++++++++++++++- .../lib/dotfilesmanager/chezmoi/manager.go | 10 +++-- installer/test-interactive-gpg.exp | 20 +++++++-- 6 files changed, 105 insertions(+), 11 deletions(-) diff --git a/.github/workflows/installer-ci.yml b/.github/workflows/installer-ci.yml index 61ab7d0..8915da2 100644 --- a/.github/workflows/installer-ci.yml +++ b/.github/workflows/installer-ci.yml @@ -207,14 +207,29 @@ jobs: mkdir -p "$HOME/.gnupg" chmod 700 "$HOME/.gnupg" + # Get current branch name for testing with fallbacks + # GITHUB_HEAD_REF is set for pull requests, GITHUB_REF for pushes + # If both fail, default to main branch to prevent CI failures + if [ -n "$GITHUB_HEAD_REF" ]; then + CURRENT_BRANCH="$GITHUB_HEAD_REF" + elif [ -n "$GITHUB_REF" ]; then + CURRENT_BRANCH="${GITHUB_REF#refs/heads/}" + else + CURRENT_BRANCH="main" + echo "Warning: Could not detect branch, defaulting to main" + fi + echo "Using branch: $CURRENT_BRANCH" + # Run the installer with minimal configuration that shouldn't require interaction # We use https as the git clone protocol to avoid SSH key prompts + # We use the current branch to test branch-specific changes timeout 300 ./dotfiles-installer install \ --non-interactive \ --plain \ --extra-verbose \ --install-prerequisites=true \ --git-clone-protocol=https \ + --git-branch="$CURRENT_BRANCH" \ --work-env=false \ - name: Install expect tool @@ -242,12 +257,25 @@ jobs: mkdir -p "$HOME/.gnupg" chmod 700 "$HOME/.gnupg" - # Run the expect script with test parameters + # Get current branch name for testing with fallbacks + # This ensures we test against the same branch as the installer changes + if [ -n "$GITHUB_HEAD_REF" ]; then + CURRENT_BRANCH="$GITHUB_HEAD_REF" + elif [ -n "$GITHUB_REF" ]; then + CURRENT_BRANCH="${GITHUB_REF#refs/heads/}" + else + CURRENT_BRANCH="main" + echo "Warning: Could not detect branch, defaulting to main" + fi + echo "Using branch: $CURRENT_BRANCH" + + # Run the expect script with test parameters and current branch installer/test-interactive-gpg.exp \ "./dotfiles-installer" \ "test-user@example.com" \ "Test CI User" \ - "test-ci-passphrase" + "test-ci-passphrase" \ + "$CURRENT_BRANCH" - name: Verify Installation Artifacts (if created) run: | diff --git a/installer/cmd/install.go b/installer/cmd/install.go index 2b7cfe5..feffac1 100644 --- a/installer/cmd/install.go +++ b/installer/cmd/install.go @@ -32,6 +32,7 @@ var ( installShellWithBrew bool gitCloneProtocol string + gitBranch string verbose bool installPrerequisites bool ) @@ -427,7 +428,7 @@ func installGpgClient(log logger.Logger) error { func setupDotfilesManager(log logger.Logger) error { log.StartProgress("Setting up dotfiles manager") - dm, err := chezmoi.TryStandardChezmoiManager(log, globalFilesystem, globalOsManager, globalCommander, globalPackageManager, globalHttpClient, GetDisplayMode(), chezmoi.DefaultGitHubUsername, gitCloneProtocol == "ssh") + dm, err := chezmoi.TryStandardChezmoiManager(log, globalFilesystem, globalOsManager, globalCommander, globalPackageManager, globalHttpClient, GetDisplayMode(), chezmoi.DefaultGitHubUsername, gitCloneProtocol == "ssh", gitBranch) if err != nil { log.FailProgress("Failed to create dotfiles manager", err) return err @@ -514,6 +515,9 @@ func init() { "Install shell with brew if not already installed") installCmd.Flags().StringVar(&gitCloneProtocol, "git-clone-protocol", "https", "Use the given git clone protocol (ssh or https) for git operations") + installCmd.Flags().StringVar(&gitBranch, "git-branch", "", + "Use the given git branch for dotfiles repository operations (defaults to repository's default branch). "+ + "Useful for testing changes in feature branches or when running in CI/CD pipelines.") installCmd.Flags().BoolVar(&installPrerequisites, "install-prerequisites", false, "Automatically install missing prerequisites") @@ -524,6 +528,7 @@ func init() { viper.BindPFlag("install-brew", installCmd.Flags().Lookup("install-brew")) viper.BindPFlag("install-shell-with-brew", installCmd.Flags().Lookup("install-shell-with-brew")) viper.BindPFlag("git-clone-protocol", installCmd.Flags().Lookup("git-clone-protocol")) + viper.BindPFlag("git-branch", installCmd.Flags().Lookup("git-branch")) viper.BindPFlag("install-prerequisites", installCmd.Flags().Lookup("install-prerequisites")) } diff --git a/installer/lib/dotfilesmanager/chezmoi/apply.go b/installer/lib/dotfilesmanager/chezmoi/apply.go index 0e5b6ab..e1d0655 100644 --- a/installer/lib/dotfilesmanager/chezmoi/apply.go +++ b/installer/lib/dotfilesmanager/chezmoi/apply.go @@ -23,6 +23,9 @@ func (c *ChezmoiManager) Apply() error { if c.chezmoiConfig.cloneViaSSH { chezmoiApplyCmdArgs = append(chezmoiApplyCmdArgs, "--ssh") } + if c.chezmoiConfig.branch != "" { + chezmoiApplyCmdArgs = append(chezmoiApplyCmdArgs, "--branch", c.chezmoiConfig.branch) + } chezmoiApplyCmdArgs = append(chezmoiApplyCmdArgs, c.chezmoiConfig.githubUsername) var discardOutputOption utils.Option = utils.EmptyOption() diff --git a/installer/lib/dotfilesmanager/chezmoi/apply_test.go b/installer/lib/dotfilesmanager/chezmoi/apply_test.go index 48c9fe8..6d88846 100644 --- a/installer/lib/dotfilesmanager/chezmoi/apply_test.go +++ b/installer/lib/dotfilesmanager/chezmoi/apply_test.go @@ -145,6 +145,7 @@ func Test_Apply_RunsChezmoiInitApplyCommand_WithSSHCloningPreference(t *testing. "/home/user/.local/share/chezmoi", "testuser", true, + "", ) manager := chezmoi.NewChezmoiManager(logger.DefaultLogger, mockFileSystem, mockUserManager, mockCommander, mockPackageManager, mockHTTPClient, utils.DisplayModeProgress, chezmoiConfig) @@ -166,7 +167,8 @@ func Test_Apply_RunsChezmoiInitApplyCommand_WithCustomUsername(t *testing.T) { mockCommander := &utils.MoqCommander{} mockCommander.RunCommandFunc = func(name string, args []string, opts ...utils.Option) (*utils.Result, error) { require.Equal(t, "chezmoi", name) - require.Equal(t, []string{"init", "--apply", "customuser", "--config", "/home/user/.config/chezmoi/chezmoi.toml"}, args) + expectedArgs := []string{"init", "--apply", "--source", "/home/user/.local/share/chezmoi", "customuser", "--config", "/home/user/.config/chezmoi/chezmoi.toml"} + require.Equal(t, expectedArgs, args) return &utils.Result{ExitCode: 0}, nil } @@ -176,9 +178,10 @@ func Test_Apply_RunsChezmoiInitApplyCommand_WithCustomUsername(t *testing.T) { chezmoiConfig := chezmoi.NewChezmoiConfig( "/home/user/.config/chezmoi", "/home/user/.config/chezmoi/chezmoi.toml", - "", + "/home/user/.local/share/chezmoi", "customuser", false, + "", ) manager := chezmoi.NewChezmoiManager(logger.DefaultLogger, mockFileSystem, mockUserManager, mockCommander, mockPackageManager, mockHTTPClient, utils.DisplayModeProgress, chezmoiConfig) @@ -271,6 +274,7 @@ func Test_Apply_SucceedsWithAllParametersCombined(t *testing.T) { "/custom/clone/dir", "testuser123", true, + "", ) manager := chezmoi.NewChezmoiManager(logger.DefaultLogger, mockFileSystem, mockUserManager, mockCommander, mockPackageManager, mockHTTPClient, utils.DisplayModeProgress, chezmoiConfig) @@ -316,6 +320,7 @@ func Test_Apply_DiscardsOutput_WhenDisplayModeIsNotPassthrough(t *testing.T) { "/custom/clone/dir", "testuser123", true, + "", ) manager := chezmoi.NewChezmoiManager(logger.DefaultLogger, mockFileSystem, mockUserManager, mockCommander, mockPackageManager, mockHTTPClient, utils.DisplayModeProgress, chezmoiConfig) @@ -361,6 +366,7 @@ func Test_Apply_DoesNotDiscardOutput_WhenDisplayModeIsPassthrough(t *testing.T) "/custom/clone/dir", "testuser123", true, + "", ) manager := chezmoi.NewChezmoiManager(logger.DefaultLogger, mockFileSystem, mockUserManager, mockCommander, mockPackageManager, mockHTTPClient, utils.DisplayModePassthrough, chezmoiConfig) @@ -369,3 +375,37 @@ func Test_Apply_DoesNotDiscardOutput_WhenDisplayModeIsPassthrough(t *testing.T) require.NoError(t, err) } + +func Test_Apply_UsesBranchParameter_WhenSpecified(t *testing.T) { + mockFileSystem := &utils.MoqFileSystem{ + RemovePathFunc: func(path string) error { + return nil + }, + } + mockUserManager := &osmanager.MoqUserManager{} + mockCommander := &utils.MoqCommander{} + mockCommander.RunCommandFunc = func(name string, args []string, opts ...utils.Option) (*utils.Result, error) { + require.Equal(t, "chezmoi", name) + expectedArgs := []string{"init", "--apply", "--source", "/home/user/.local/share/chezmoi", "--branch", "feature-branch", "testuser", "--config", "/home/user/.config/chezmoi/chezmoi.toml"} + require.Equal(t, expectedArgs, args) + return &utils.Result{ExitCode: 0}, nil + } + + mockPackageManager := &pkgmanager.MoqPackageManager{} + mockHTTPClient := &httpclient.MoqHTTPClient{} + + chezmoiConfig := chezmoi.NewChezmoiConfig( + "/home/user/.config/chezmoi", + "/home/user/.config/chezmoi/chezmoi.toml", + "/home/user/.local/share/chezmoi", + "testuser", + false, + "feature-branch", + ) + + manager := chezmoi.NewChezmoiManager(logger.DefaultLogger, mockFileSystem, mockUserManager, mockCommander, mockPackageManager, mockHTTPClient, utils.DisplayModeProgress, chezmoiConfig) + + err := manager.Apply() + + require.NoError(t, err) +} diff --git a/installer/lib/dotfilesmanager/chezmoi/manager.go b/installer/lib/dotfilesmanager/chezmoi/manager.go index 7b4d0c7..70af011 100644 --- a/installer/lib/dotfilesmanager/chezmoi/manager.go +++ b/installer/lib/dotfilesmanager/chezmoi/manager.go @@ -20,15 +20,17 @@ type ChezmoiConfig struct { chezmoiCloneDir string githubUsername string cloneViaSSH bool + branch string } -func NewChezmoiConfig(configDir, configFilePath, cloneDir, githubUsername string, cloneViaSSH bool) ChezmoiConfig { +func NewChezmoiConfig(configDir, configFilePath, cloneDir, githubUsername string, cloneViaSSH bool, branch string) ChezmoiConfig { return ChezmoiConfig{ chezmoiConfigDir: configDir, chezmoiConfigFilePath: configFilePath, chezmoiCloneDir: cloneDir, githubUsername: githubUsername, cloneViaSSH: cloneViaSSH, + branch: branch, } } @@ -39,6 +41,7 @@ func DefaultChezmoiConfig(chezmoiConfigFilePath string, chezmoiCloneDir string) chezmoiCloneDir: chezmoiCloneDir, githubUsername: "MrPointer", cloneViaSSH: false, + branch: "", } } @@ -68,7 +71,7 @@ func NewChezmoiManager(logger logger.Logger, filesystem utils.FileSystem, userMa } } -func TryStandardChezmoiManager(logger logger.Logger, filesystem utils.FileSystem, userManager osmanager.UserManager, commander utils.Commander, pkgManager pkgmanager.PackageManager, httpClient httpclient.HTTPClient, displayMode utils.DisplayMode, githubUsername string, cloneViaSSH bool) (*ChezmoiManager, error) { +func TryStandardChezmoiManager(logger logger.Logger, filesystem utils.FileSystem, userManager osmanager.UserManager, commander utils.Commander, pkgManager pkgmanager.PackageManager, httpClient httpclient.HTTPClient, displayMode utils.DisplayMode, githubUsername string, cloneViaSSH bool, branch string) (*ChezmoiManager, error) { chezmoiConfigHome, err := userManager.GetChezmoiConfigHome() if err != nil { return nil, fmt.Errorf("failed to get chezmoi config home directory: %w", err) @@ -97,10 +100,11 @@ func TryStandardChezmoiManager(logger logger.Logger, filesystem utils.FileSystem chezmoiCloneDir, githubUsername, cloneViaSSH, + branch, ), ), nil } func TryStandardChezmoiManagerWithDefaults(logger logger.Logger, filesystem utils.FileSystem, userManager osmanager.UserManager, commander utils.Commander, pkgManager pkgmanager.PackageManager, httpClient httpclient.HTTPClient, displayMode utils.DisplayMode) (*ChezmoiManager, error) { - return TryStandardChezmoiManager(logger, filesystem, userManager, commander, pkgManager, httpClient, displayMode, DefaultGitHubUsername, false) + return TryStandardChezmoiManager(logger, filesystem, userManager, commander, pkgManager, httpClient, displayMode, DefaultGitHubUsername, false, "") } diff --git a/installer/test-interactive-gpg.exp b/installer/test-interactive-gpg.exp index e34a49f..b750f52 100755 --- a/installer/test-interactive-gpg.exp +++ b/installer/test-interactive-gpg.exp @@ -1,7 +1,7 @@ #!/usr/bin/expect -f # # Interactive GPG Testing Script for Dotfiles Installer -# Usage: ./test-interactive-gpg.exp [installer_path] [email] [name] [passphrase] +# Usage: ./test-interactive-gpg.exp [installer_path] [email] [name] [passphrase] [branch] # # This script automates GPG key setup during interactive installation # for both CI testing and local development. @@ -14,7 +14,8 @@ set installer_path [lindex $argv 0] set email [lindex $argv 1] set name [lindex $argv 2] set passphrase [lindex $argv 3] -set verbosity [lindex $argv 4] +set branch [lindex $argv 4] +set verbosity [lindex $argv 5] # Use defaults if not provided if {$installer_path eq ""} { @@ -29,6 +30,9 @@ if {$name eq ""} { if {$passphrase eq ""} { set passphrase "test-ci-passphrase" } +if {$branch eq ""} { + set branch "" +} if {$verbosity eq ""} { set verbosity "" } @@ -45,10 +49,20 @@ puts " Installer: $installer_path" puts " Email: $email" puts " Name: $name" puts " Passphrase: [string repeat "*" [string length $passphrase]]" +puts " Branch: $branch" puts " Verbosity: $verbosity" puts "" -spawn $installer_path install --plain --install-prerequisites=true --git-clone-protocol=https "$verbosity" +# Build command with optional branch parameter +set cmd_args [list "install" "--plain" "--install-prerequisites=true" "--git-clone-protocol=https"] +if {$branch ne ""} { + lappend cmd_args "--git-branch=$branch" +} +if {$verbosity ne ""} { + lappend cmd_args $verbosity +} + +spawn $installer_path {*}$cmd_args # Main interaction loop - Only handle GPG-specific prompts expect {