web-server: add '--remove' option to 'stop'

Add a '--remove' option to 'git-bundle-server web-server stop' to fully
remove the service daemon and its resources on disk after stopping the
daemon process. For launchd, this is done by first running 'launchctl
bootout' on the appropriate service, then removing the plist file; for
systemd, the service unit file is removed then 'systemctl daemon-reload' is
run. Neither implementation fails if the web server was never loaded.

Although it is unlikely that users will need this option in day-to-day use,
this cleanup option will be needed to fully remove bundle server resources
in the uninstall scripts implemented in later patches.

Signed-off-by: Victoria Dye <[email protected]>

Author: vdye

cmd/git-bundle-server/web-server.go

@@ -145,7 +145,8 @@ func (w *webServer) startServer(args []string) error {
 
 func (w *webServer) stopServer(args []string) error {
 	// Parse subcommand arguments
-	parser := argparse.NewArgParser("git-bundle-server web-server stop")
+	parser := argparse.NewArgParser("git-bundle-server web-server stop [--remove]")
+	remove := parser.Bool("remove", false, "Remove the web server daemon configuration from the system after stopping")
 	parser.Parse(args)
 
 	d, err := daemon.NewDaemonProvider(w.user, w.cmdExec, w.fileSystem)
@@ -163,6 +164,13 @@ func (w *webServer) stopServer(args []string) error {
 		return err
 	}
 
+	if *remove {
+		err = d.Remove(config.Label)
+		if err != nil {
+			return err
+		}
+	}
+
 	return nil
 }
 

internal/common/filesystem.go

@@ -6,11 +6,13 @@ import (
 	"fmt"
 	"os"
 	"path"
+	"syscall"
 )
 
 type FileSystem interface {
 	FileExists(filename string) (bool, error)
 	WriteFile(filename string, content []byte) error
+	DeleteFile(filename string) (bool, error)
 	ReadFileLines(filename string) ([]string, error)
 }
 
@@ -46,6 +48,20 @@ func (f *fileSystem) WriteFile(filename string, content []byte) error {
 	return nil
 }
 
+func (f *fileSystem) DeleteFile(filename string) (bool, error) {
+	err := os.Remove(filename)
+	if err == nil {
+		return true, nil
+	}
+
+	pathErr, ok := err.(*os.PathError)
+	if ok && pathErr.Err == syscall.ENOENT {
+		return false, nil
+	} else {
+		return false, err
+	}
+}
+
 func (f *fileSystem) ReadFileLines(filename string) ([]string, error) {
 	file, err := os.OpenFile(filename, os.O_RDONLY|os.O_CREATE, 0o600)
 	if err != nil {

internal/daemon/daemon.go

@@ -20,6 +20,8 @@ type DaemonProvider interface {
 	Start(label string) error
 
 	Stop(label string) error
+
+	Remove(label string) error
 }
 
 func NewDaemonProvider(

internal/daemon/launchd.go

@@ -265,3 +265,26 @@ func (l *launchd) Stop(label string) error {
 
 	return nil
 }
+
+func (l *launchd) Remove(label string) error {
+	user, err := l.user.CurrentUser()
+	if err != nil {
+		return fmt.Errorf("could not get current user for launchd service: %w", err)
+	}
+
+	filename := filepath.Join(user.HomeDir, "Library", "LaunchAgents", fmt.Sprintf("%s.plist", label))
+	domainTarget := fmt.Sprintf(domainFormat, user.Uid)
+	serviceTarget := fmt.Sprintf("%s/%s", domainTarget, label)
+
+	_, err = l.bootout(serviceTarget)
+	if err != nil {
+		return fmt.Errorf("could not remove daemon process '%s': %w", label, err)
+	}
+
+	_, err = l.fileSystem.DeleteFile(filename)
+	if err != nil {
+		return fmt.Errorf("could not delete launchd plist: %w", err)
+	}
+
+	return nil
+}

internal/daemon/launchd_test.go

@@ -435,7 +435,7 @@ func TestLaunchd_Stop(t *testing.T) {
 	// Reset the mock structure between tests
 	testCommandExecutor.Mock = mock.Mock{}
 
-	// Test #3: launchctl fails with uncaught error
+	// Test #3: launchctl fails with expected error
 	t.Run("Exits without error if service not found", func(t *testing.T) {
 		testCommandExecutor.On("Run",
 			mock.AnythingOfType("string"),
@@ -447,3 +447,102 @@ func TestLaunchd_Stop(t *testing.T) {
 		mock.AssertExpectationsForObjects(t, testCommandExecutor)
 	})
 }
+
+var launchdRemoveTests = []struct {
+	title string
+
+	// Inputs
+	label string
+
+	// Mocked responses
+	launchctlBootout *Pair[int, error]
+	deleteFile       *Pair[bool, error]
+
+	// Expected values
+	expectErr bool
+}{
+	{
+		"Unloads and deletes plist when service loaded",
+		"com.test.service",
+		PtrTo(NewPair[int, error](0, nil)),     // launchctl bootout
+		PtrTo(NewPair[bool, error](true, nil)), // delete file
+		false,
+	},
+	{
+		"Removes plist when service is missing",
+		"com.test.service",
+		PtrTo(NewPair[int, error](daemon.LaunchdNoSuchProcessErrorCode, nil)), // launchctl bootout
+		PtrTo(NewPair[bool, error](true, nil)),                                // delete file
+		false,
+	},
+	{
+		"Removal of non-existent service succeeds",
+		"com.test.service",
+		PtrTo(NewPair[int, error](daemon.LaunchdNoSuchProcessErrorCode, nil)), // launchctl bootout
+		PtrTo(NewPair[bool, error](false, nil)),                               // delete file
+		false,
+	},
+	{
+		"launchctl error code skips file deletion",
+		"com.test.service",
+		PtrTo(NewPair[int, error](-1, nil)), // launchctl bootout
+		nil,                                 // delete file
+		true,
+	},
+	{
+		"Unhandled launchctl error skips file deletion",
+		"com.test.service",
+		PtrTo(NewPair(0, fmt.Errorf("some unhandled error"))), // launchctl bootout
+		nil, // delete file
+		true,
+	},
+}
+
+func TestLaunchd_Remove(t *testing.T) {
+	// Set up mocks
+	testUser := &user.User{
+		Uid:      "123",
+		Username: "testuser",
+		HomeDir:  "/my/test/dir",
+	}
+	testUserProvider := &MockUserProvider{}
+	testUserProvider.On("CurrentUser").Return(testUser, nil)
+
+	testCommandExecutor := &MockCommandExecutor{}
+	testFileSystem := &MockFileSystem{}
+
+	launchd := daemon.NewLaunchdProvider(testUserProvider, testCommandExecutor, testFileSystem)
+
+	for _, tt := range launchdRemoveTests {
+		t.Run(tt.title, func(t *testing.T) {
+			// Setup expected values
+			expectedFilename := filepath.Clean(fmt.Sprintf("/my/test/dir/Library/LaunchAgents/%s.plist", tt.label))
+
+			// Mock responses
+			if tt.launchctlBootout != nil {
+				testCommandExecutor.On("Run",
+					"launchctl",
+					[]string{"bootout", fmt.Sprintf("user/123/%s", tt.label)},
+				).Return(tt.launchctlBootout.First, tt.launchctlBootout.Second).Once()
+			}
+			if tt.deleteFile != nil {
+				testFileSystem.On("DeleteFile",
+					expectedFilename,
+				).Return(tt.deleteFile.First, tt.deleteFile.Second).Once()
+			}
+
+			// Call function
+			err := launchd.Remove(tt.label)
+			mock.AssertExpectationsForObjects(t, testCommandExecutor)
+			if tt.expectErr {
+				assert.NotNil(t, err)
+			} else {
+				assert.Nil(t, err)
+			}
+		})
+
+		// Reset the mocks between tests
+		testCommandExecutor.Mock = mock.Mock{}
+		testFileSystem.Mock = mock.Mock{}
+	}
+}

internal/daemon/systemd.go

@@ -38,6 +38,19 @@ func NewSystemdProvider(
 	}
 }
 
+func (s *systemd) reloadDaemon() error {
+	exitCode, err := s.cmdExec.Run("systemctl", "--user", "daemon-reload")
+	if err != nil {
+		return err
+	}
+
+	if exitCode != 0 {
+		return fmt.Errorf("'systemctl --user daemon-reload' exited with status %d", exitCode)
+	}
+
+	return nil
+}
+
 func (s *systemd) Create(config *DaemonConfig, force bool) error {
 	user, err := s.user.CurrentUser()
 	if err != nil {
@@ -75,16 +88,12 @@ func (s *systemd) Create(config *DaemonConfig, force bool) error {
 		return fmt.Errorf("unable to write service unit: %w", err)
 	}
 
-	// Reload the user-scoped service units
-	exitCode, err := s.cmdExec.Run("systemctl", "--user", "daemon-reload")
+	// Reload the user-scoped service units after adding
+	err = s.reloadDaemon()
 	if err != nil {
 		return err
 	}
 
-	if exitCode != 0 {
-		return fmt.Errorf("'systemctl --user daemon-reload' exited with status %d", exitCode)
-	}
-
 	return nil
 }
 
@@ -115,3 +124,24 @@ func (s *systemd) Stop(label string) error {
 
 	return nil
 }
+
+func (s *systemd) Remove(label string) error {
+	user, err := s.user.CurrentUser()
+	if err != nil {
+		return fmt.Errorf("could not get current user for launchd service: %w", err)
+	}
+	filename := filepath.Join(user.HomeDir, ".config", "systemd", "user", fmt.Sprintf("%s.service", label))
+
+	_, err = s.fileSystem.DeleteFile(filename)
+	if err != nil {
+		return fmt.Errorf("could not delete service unit: %w", err)
+	}
+
+	// Reload the user-scoped service units after removing
+	err = s.reloadDaemon()
+	if err != nil {
+		return err
+	}
+
+	return nil
+}

internal/daemon/systemd_test.go

@@ -329,3 +329,88 @@ func TestSystemd_Stop(t *testing.T) {
 		mock.AssertExpectationsForObjects(t, testCommandExecutor)
 	})
 }
+
+var systemdRemoveTests = []struct {
+	title string
+
+	// Inputs
+	label string
+
+	// Mocked responses
+	deleteFile            *Pair[bool, error]
+	systemctlDaemonReload *Pair[int, error]
+
+	// Expected values
+	expectErr bool
+}{
+	{
+		"Unloads and deletes service unit",
+		"com.test.service",
+		PtrTo(NewPair[bool, error](true, nil)), // delete file
+		PtrTo(NewPair[int, error](0, nil)),     // systemctl daemon-reload
+		false,
+	},
+	{
+		"Reloads daemon even if service unit missing",
+		"com.test.service",
+		PtrTo(NewPair[bool, error](false, nil)), // delete file
+		PtrTo(NewPair[int, error](0, nil)),      // systemctl daemon-reload
+		false,
+	},
+	{
+		"Daemon not reloaded if file cannot be deleted",
+		"com.test.service",
+		PtrTo(NewPair(false, fmt.Errorf("unhandled error"))), // delete file
+		nil, // systemctl daemon-reload
+		true,
+	},
+}
+
+func TestSystemd_Remove(t *testing.T) {
+	// Set up mocks
+	testUser := &user.User{
+		Uid:      "123",
+		Username: "testuser",
+		HomeDir:  "/my/test/dir",
+	}
+	testUserProvider := &MockUserProvider{}
+	testUserProvider.On("CurrentUser").Return(testUser, nil)
+
+	testCommandExecutor := &MockCommandExecutor{}
+	testFileSystem := &MockFileSystem{}
+
+	systemd := daemon.NewSystemdProvider(testUserProvider, testCommandExecutor, testFileSystem)
+
+	for _, tt := range systemdRemoveTests {
+		t.Run(tt.title, func(t *testing.T) {
+			// Setup expected values
+			expectedFilename := filepath.Clean(fmt.Sprintf("/my/test/dir/.config/systemd/user/%s.service", tt.label))
+
+			// Mock responses
+			if tt.deleteFile != nil {
+				testFileSystem.On("DeleteFile",
+					expectedFilename,
+				).Return(tt.deleteFile.First, tt.deleteFile.Second).Once()
+			}
+			if tt.systemctlDaemonReload != nil {
+				testCommandExecutor.On("Run",
+					"systemctl",
+					[]string{"--user", "daemon-reload"},
+				).Return(tt.systemctlDaemonReload.First, tt.systemctlDaemonReload.Second).Once()
+			}
+
+			// Call function
+			err := systemd.Remove(tt.label)
+			mock.AssertExpectationsForObjects(t, testCommandExecutor)
+			if tt.expectErr {
+				assert.NotNil(t, err)
+			} else {
+				assert.Nil(t, err)
+			}
+		})
+
+		// Reset the mocks between tests
+		testCommandExecutor.Mock = mock.Mock{}
+		testFileSystem.Mock = mock.Mock{}
+	}
+}

internal/testhelpers/funcs.go

@@ -0,0 +1,9 @@
+package testhelpers
+
+/********************************************/
+/************* Helper Functions *************/
+/********************************************/
+
+func PtrTo[T any](val T) *T {
+	return &val
+}

internal/testhelpers/mocks.go

@@ -38,6 +38,11 @@ func (m *MockFileSystem) WriteFile(filename string, content []byte) error {
 	return fnArgs.Error(0)
 }
 
+func (m *MockFileSystem) DeleteFile(filename string) (bool, error) {
+	fnArgs := m.Called(filename)
+	return fnArgs.Bool(0), fnArgs.Error(1)
+}
+
 func (m *MockFileSystem) ReadFileLines(filename string) ([]string, error) {
 	fnArgs := m.Called(filename)
 	return fnArgs.Get(0).([]string), fnArgs.Error(1)