Allow disabling modesetting, better outputs, fix directory permission…

…s, readme (#103)

* only print debug logs with verbose flag (closes #100)

* fix config dir permissions (closes #102)

* allow disabling modesetting, cleanup logging output, embed template from file (closes #95)

* change disableModesetting to nomodesetting

* allow passing nomodesetting as flag, update readme

* better logging output

* update readme

README.md

@@ -41,7 +41,7 @@ and if so, make X.Org prefer it.
 
 ### Ubuntu (apt)
 
-*The PPA is no longer maintained for now (see #90)*
+*The PPA is no longer maintained for now (see [#90](https://github.com/hertg/egpu-switcher/issues/90))*
 
 ### Arch (aur)
 
@@ -53,6 +53,30 @@ paru -S egpu-switcher
 
 ### Manual
 
+
+#### Installation and setup
+
+Download binary from [latest release](https://github.com/hertg/egpu-switcher/releases)
+
+Copy binary to `/opt`, apply proper permissions, and link it in `/usr/bin`
+
+```bash
+sudo cp <downloaded-binary> /opt/egpu-switcher
+sudo chmod 755 /opt/egpu-switcher
+sudo ln -s /opt/egpu-switcher /usr/bin/egpu-switcher
+sudo egpu-switcher enable
+```
+
+#### Uninstall
+
+```bash
+sudo egpu-switcher disable --hard
+sudo rm /usr/bin/egpu-switcher
+sudo rm /opt/egpu-switcher
+```
+
+### Build
+
 #### Prerequisites
 
 Install the [go toolchain](https://go.dev/doc/install)
@@ -70,7 +94,7 @@ sudo egpu-switcher enable
 #### Uninstall
 
 ```bash
-sudo egpu-switcher disable
+sudo egpu-switcher disable --hard
 sudo make uninstall -s
 ```
 
@@ -104,18 +128,21 @@ Use "egpu-switcher [command] --help" for more information about a command.
 ## Configuration
 
 The config file is created automatically and can be found at `/etc/egpu-switcher/config.yaml`.
-Below you can see an example of a configuration file, annotated with some notes.
+Below you can see an example of a configuration file, annotated with additional information.
 
 ```yaml
-# the 'egpu' config is generated by 'egpu-switcher config'.
-# you probably shouldn't change this manually.
 egpu:
+    # the 'driver' and 'id' configs are generated by 'egpu-switcher config'.
+    # you probably shouldn't change this manually unless you understand why.
     driver: amdgpu
     id: 1153611719250962689
+
+    # OPTIONAL: do not load 'modesetting' in the egpu config
+    nomodesetting: false
 
 # OPTIONAL: how many times 'egpu-switcher switch auto' should retry finding the egpu.
 # this can be helpful if the egpu takes some time to connect on your machine,
-# the following values are the default, if this config is omitted.
+# the following values are the default.
 detection:
   retries: 6
   interval: 500 # milliseconds
@@ -125,8 +152,7 @@ detection:
 # then be run with '/bin/sh $script'.
 # 
 # it is required that the script is owned by root (uid 0) 
-# and has a permission of -rwx------ (0700), in order to
-# prevent potential privilege escalation.
+# and has a permission of -rwx------ (0700).
 hooks:
   internal: /home/michael/tmp/internal.sh
   egpu: /home/michael/tmp/egpu.sh

cmd/config.go

@@ -26,11 +26,6 @@ var configCommand = &cobra.Command{
 		gpus := pci.ReadGPUs()
 		amount := int(len(gpus))
 
-		/*if amount < 2 {
-			logger.Error("only one GPU found... please plug in your eGPU to continue")
-			os.Exit(1)
-		}*/
-
 		fmt.Println()
 		fmt.Printf("Found %d possible GPU(s)...\n", amount)
 		fmt.Println()
@@ -67,7 +62,7 @@ var configCommand = &cobra.Command{
 
 		fmt.Println()
 
-		logger.Success("Your selection has been saved")
+		logger.Success("Your selection was saved to the config file")
 
 		return nil
 	},

cmd/disable.go

@@ -34,9 +34,11 @@ var disableCommand = &cobra.Command{
 		if err != nil {
 			return err
 		}
-		if err := init.TeardownService(ctx); err != nil {
+
+		if err := init.TeardownService(ctx, verbose); err != nil {
 			return fmt.Errorf("unable to tear down service: %s", err)
 		}
+		logger.Info("removed egpu bootup service")
 
 		if hard {
 			// remove /etc/egpu-switcher

cmd/enable.go

@@ -34,6 +34,7 @@ var setupCommand = &cobra.Command{
 			logger.Info("no eGPU has been configured yet")
 			if noPrompt {
 				logger.Warn("please run 'egpu-switcher config' to configure your eGPU")
+				return fmt.Errorf("setup aborted")
 			} else {
 				err := configCommand.RunE(cmd, []string{})
 				if err != nil {
@@ -42,10 +43,12 @@ var setupCommand = &cobra.Command{
 			}
 		}
 
-		if err := init.CreateService(ctx); err != nil {
+		if err := init.CreateService(ctx, verbose); err != nil {
 			return err
 		}
 
+		logger.Info("created egpu bootup service to autorun 'egpu-switcher switch'")
+
 		logger.Success("setup successful")
 		return nil
 	},

cmd/root.go

@@ -63,7 +63,7 @@ func initConfig() {
 				if verbose {
 					logger.Debug("no configuration file found, creating a new one at %s\n", configPath)
 				}
-				err = os.MkdirAll(configPath, 0744)
+				err = os.MkdirAll(configPath, 0755)
 				cobra.CheckErr(err)
 				err = viper.SafeWriteConfig()
 				cobra.CheckErr(err)

cmd/switch.go

@@ -19,6 +19,7 @@ import (
 const x11ConfPath = "/etc/X11/xorg.conf.d/99-egpu-switcher.conf"
 
 var override bool
+var nomodesetting bool
 
 var switchCommand = &cobra.Command{
 	Use:   "switch [auto|internal|egpu]",
@@ -60,8 +61,12 @@ var switchCommand = &cobra.Command{
 		}
 
 		if arg == "internal" {
+			if err := switchInternal(); err != nil {
+				logger.Error("switch failed")
+				return err
+			}
 			logger.Success("switch successful")
-			return switchInternal()
+			return nil
 		}
 
 		gpu := pci.Find(uint64(id))
@@ -113,6 +118,7 @@ var switchCommand = &cobra.Command{
 func init() {
 	rootCmd.AddCommand(switchCommand)
 	switchCommand.PersistentFlags().BoolVar(&override, "override", false, "switch to the eGPU even if there are no displays attached") // todo: usage
+	switchCommand.PersistentFlags().BoolVar(&nomodesetting, "nomodesetting", false, "do not load modesetting module with egpu")
 }
 
 func switchEgpu(gpu *pci.GPU) error {
@@ -136,7 +142,8 @@ func switchEgpu(gpu *pci.GPU) error {
 		}
 	}
 
-	conf := xorg.RenderConf("Device0", driver, gpu.XorgPCIString())
+	nomodesetting = nomodesetting || viper.GetBool("egpu.nomodesetting")
+	conf := xorg.RenderConf("Device0", driver, gpu.XorgPCIString(), !nomodesetting)
 	if err := xorg.CreateEgpuFile(x11ConfPath, conf, verbose); err != nil {
 		return err
 	}

internal/service/interface.go

@@ -3,11 +3,11 @@ package service
 import "context"
 
 type InitSystem interface {
-	CreateService(context.Context) error
-	TeardownService(context.Context) error
-	StopUnit(context.Context, string) error
-	StartUnit(context.Context, string) error
-	StopDisplayManager(context.Context) error
-	StartDisplayManager(context.Context) error
-	IsDisplayManagerStopped(context.Context) (bool, error)
+	CreateService(ctx context.Context, verbose bool) error
+	TeardownService(ctx context.Context, verbose bool) error
+	StopUnit(ctx context.Context, unit string, verbose bool) error
+	StartUnit(ctx context.Context, unit string, verbose bool) error
+	StopDisplayManager(ctx context.Context, verbose bool) error
+	StartDisplayManager(ctx context.Context, verbose bool) error
+	IsDisplayManagerStopped(ctx context.Context, verbose bool) (bool, error)
 }

internal/service/systemd.go

@@ -41,35 +41,35 @@ func (s *Systemd) displayManagerName() (string, error) {
 	return filepath.Base(dmServiceName), nil
 }
 
-func (s *Systemd) StopUnit(ctx context.Context, name string) error {
+func (s *Systemd) StopUnit(ctx context.Context, name string, verbose bool) error {
 	conn := s.conn(ctx)
 	_, err := conn.StopUnitContext(ctx, name, "replace", nil)
 	return err
 }
 
-func (s *Systemd) StartUnit(ctx context.Context, name string) error {
+func (s *Systemd) StartUnit(ctx context.Context, name string, verbose bool) error {
 	conn := s.conn(ctx)
 	_, err := conn.StopUnitContext(ctx, name, "replace", nil)
 	return err
 }
 
-func (s *Systemd) StopDisplayManager(ctx context.Context) error {
+func (s *Systemd) StopDisplayManager(ctx context.Context, verbose bool) error {
 	name, err := s.displayManagerName()
 	if err != nil {
 		return err
 	}
-	return s.StopUnit(ctx, name)
+	return s.StopUnit(ctx, name, verbose)
 }
 
-func (s *Systemd) StartDisplayManager(ctx context.Context) error {
+func (s *Systemd) StartDisplayManager(ctx context.Context, verbose bool) error {
 	name, err := s.displayManagerName()
 	if err != nil {
 		return err
 	}
-	return s.StartUnit(ctx, name)
+	return s.StartUnit(ctx, name, verbose)
 }
 
-func (s *Systemd) IsDisplayManagerStopped(ctx context.Context) (bool, error) {
+func (s *Systemd) IsDisplayManagerStopped(ctx context.Context, verbose bool) (bool, error) {
 	conn := s.conn(ctx)
 	name, err := s.displayManagerName()
 	if err != nil {
@@ -82,7 +82,7 @@ func (s *Systemd) IsDisplayManagerStopped(ctx context.Context) (bool, error) {
 	return props["ActiveState"] == "inactive", nil
 }
 
-func (s *Systemd) CreateService(ctx context.Context) error {
+func (s *Systemd) CreateService(ctx context.Context, verbose bool) error {
 	serviceTemplate := `# generated by egpu-switcher
 [Unit]
 Description=EGPU Service
@@ -111,7 +111,9 @@ WantedBy=graphical.target
 		return fmt.Errorf("unable to generate systemd service file: %s", err)
 	}
 
-	logger.Debug("generated systemd unit file at '%s'", sharePath)
+	if verbose {
+		logger.Debug("generated systemd unit file at '%s'", sharePath)
+	}
 
 	systemd := s.conn(ctx)
 	err = systemd.ReloadContext(ctx)
@@ -122,11 +124,14 @@ WantedBy=graphical.target
 	if err != nil {
 		return fmt.Errorf("unable to enable %s: %s", serviceName, err)
 	}
-	logger.Debug("enabled systemd unit '%s'", serviceName)
+
+	if verbose {
+		logger.Debug("enabled systemd unit '%s'", serviceName)
+	}
 	return nil
 }
 
-func (s *Systemd) TeardownService(ctx context.Context) error {
+func (s *Systemd) TeardownService(ctx context.Context, verbose bool) error {
 	conn := s.conn(ctx)
 
 	prop, err := conn.GetUnitPropertiesContext(ctx, serviceName)
@@ -154,7 +159,9 @@ func (s *Systemd) TeardownService(ctx context.Context) error {
 			return fmt.Errorf("unable to remove %s: %s", sharePath, err)
 		}
 	} else {
-		logger.Debug("file '%s' was removed", sharePath)
+		if verbose {
+			logger.Debug("file '%s' was removed", sharePath)
+		}
 	}
 
 	return nil

internal/xorg/conf.go

@@ -2,13 +2,17 @@ package xorg
 
 import (
 	"bytes"
+	_ "embed"
 	"fmt"
 	"html/template"
 	"os"
 
 	"github.com/hertg/egpu-switcher/internal/logger"
 )
 
+//go:embed conf.template
+var confTemplate string
+
 func RemoveEgpuFile(path string, verbose bool) error {
 	f, _ := os.Stat(path)
 	if f != nil {
@@ -18,56 +22,42 @@ func RemoveEgpuFile(path string, verbose bool) error {
 		}
 	}
 	if verbose {
-		logger.Info("%s has been removed", path)
+		logger.Debug("deleted '%s'", path)
 	}
+	logger.Info("egpu has been removed from X.Org config")
 	return nil
 }
 
 func CreateEgpuFile(path string, contents string, verbose bool) error {
-	_, err := os.Stat(path)
+	f, err := os.Create(path)
 	if err != nil {
-		f, err := os.Create(path)
-		if err != nil {
-			return fmt.Errorf("unable to create file %s", path)
-		}
-		_, err = f.Write([]byte(contents))
-		if err != nil {
-			return fmt.Errorf("unable to write config to file %s", path)
-		}
+		return fmt.Errorf("unable to create file %s", path)
+	}
+	_, err = f.Write([]byte(contents))
+	if err != nil {
+		return fmt.Errorf("unable to write config to file %s", path)
 	}
 	if verbose {
-		logger.Info("%s has been created", path)
+		logger.Debug("written '%s'", path)
 	}
+	logger.Info("egpu has been added to X.Org config")
 	return nil
 }
 
-func RenderConf(id string, driver string, busid string) string {
-
-	const confTemplate = `# autogenerated by egpu-switcher
-
-Section "Module"
-    Load           "modesetting"
-EndSection
-
-Section "Device"
-    Identifier     "{{.Id}}"
-    Driver         "{{.Driver}}"
-    BusID          "{{.Bus}}"
-    Option         "AllowEmptyInitialConfiguration"
-    Option         "AllowExternalGpus" "True"
-EndSection
-`
+func RenderConf(id string, driver string, busid string, modesetting bool) string {
 
 	type conf struct {
-		Id     string
-		Driver string
-		Bus    string
+		Id          string
+		Driver      string
+		Bus         string
+		Modesetting bool
 	}
 
 	c := conf{
-		Id:     id,
-		Driver: driver,
-		Bus:    busid,
+		Id:          id,
+		Driver:      driver,
+		Bus:         busid,
+		Modesetting: modesetting,
 	}
 
 	buf := bytes.NewBuffer(nil)