fix: data race on Systray.SetCurrentConfigFile

config/config.go

@@ -18,6 +18,7 @@ package config
 import (
 	// we need this for the config ini in this package
 	_ "embed"
+	"fmt"
 	"os"
 
 	"github.com/arduino/go-paths-helper"
@@ -142,3 +143,42 @@ func SetInstallCertsIni(filename string, value string) error {
 	}
 	return nil
 }
+
+// GetConfigPath returns the path to the config file
+func GetConfigPath() *paths.Path {
+	// Let's handle the config
+	configDir := GetDefaultConfigDir()
+	var configPath *paths.Path
+
+	// see if the env var is defined, if it is take the config from there, this will override the default path
+	if envConfig := os.Getenv("ARDUINO_CREATE_AGENT_CONFIG"); envConfig != "" {
+		configPath = paths.New(envConfig)
+		if configPath.NotExist() {
+			panic(fmt.Sprintf("config from env var %s does not exists", envConfig))
+		}
+		log.Infof("using config from env variable: %s", configPath)
+	} else if defaultConfigPath := configDir.Join("config.ini"); defaultConfigPath.Exist() {
+		// by default take the config from the ~/.arduino-create/config.ini file
+		configPath = defaultConfigPath
+		log.Infof("using config from default: %s", configPath)
+	} else {
+		// Fall back to the old config.ini location
+		src, _ := os.Executable()
+		oldConfigPath := paths.New(src).Parent().Join("config.ini")
+		if oldConfigPath.Exist() {
+			err := oldConfigPath.CopyTo(defaultConfigPath)
+			if err != nil {
+				log.Errorf("cannot copy old %s, to %s, generating new config", oldConfigPath, configPath)
+			} else {
+				configPath = defaultConfigPath
+				log.Infof("copied old %s, to %s", oldConfigPath, configPath)
+			}
+		}
+	}
+	if configPath == nil {
+		configPath = GenerateConfig(configDir)
+	}
+
+	return configPath
+
+}

config/config_test.go

@@ -0,0 +1,119 @@
+package config
+
+import (
+	"fmt"
+	"os"
+	"runtime"
+	"testing"
+
+	"github.com/arduino/go-paths-helper"
+	"github.com/go-ini/ini"
+	"github.com/stretchr/testify/assert"
+)
+
+// TestGetConfigPathFromXDG_CONFIG_HOME tests the case when the config.ini is read from XDG_CONFIG_HOME/ArduinoCreateAgent/config.ini
+func TestGetConfigPathFrom_XDG_CONFIG_HOME(t *testing.T) {
+	if runtime.GOOS != "linux" {
+		t.Skip("Skipping test on non-linux OS")
+	}
+	// read config from $XDG_CONFIG_HOME/ArduinoCreateAgent/config.ini
+	os.Setenv("XDG_CONFIG_HOME", "./testdata/fromxdghome")
+	defer os.Unsetenv("XDG_CONFIG_HOME")
+	configPath := GetConfigPath()
+
+	assert.Equal(t, "testdata/fromxdghome/ArduinoCreateAgent/config.ini", configPath.String())
+	checkIniName(t, configPath, "this-is-a-config-file-from-xdghome-dir")
+}
+
+// TestGetConfigPathFromHOME tests the case when the config.ini is read from $HOME/.config/ArduinoCreateAgent/config.ini
+func TestGetConfigPathFrom_HOME(t *testing.T) {
+	if runtime.GOOS != "linux" {
+		t.Skip("Skipping test on non-linux OS")
+	}
+	os.Setenv("HOME", "./testdata/fromhome")
+	defer os.Unsetenv("HOME")
+	configPath := GetConfigPath()
+
+	assert.Equal(t, "testdata/fromhome/.config/ArduinoCreateAgent/config.ini", configPath.String())
+	checkIniName(t, configPath, "this-is-a-config-file-from-home-dir")
+}
+
+// TestGetConfigPathFromARDUINO_CREATE_AGENT_CONFIG tests the case when the config.ini is read from ARDUINO_CREATE_AGENT_CONFIG env variable
+func TestGetConfigPathFrom_ARDUINO_CREATE_AGENT_CONFIG(t *testing.T) {
+	if runtime.GOOS != "linux" {
+		t.Skip("Skipping test on non-linux OS")
+	}
+	//  $HOME must be always set, otherwise panic
+	os.Setenv("HOME", "./testdata/dummyhome")
+
+	os.Setenv("ARDUINO_CREATE_AGENT_CONFIG", "./testdata/from-arduino-create-agent-config-env/config.ini")
+	defer os.Unsetenv("ARDUINO_CREATE_AGENT_CONFIG")
+
+	configPath := GetConfigPath()
+	assert.Equal(t, "./testdata/from-arduino-create-agent-config-env/config.ini", configPath.String())
+	checkIniName(t, configPath, "this-is-a-config-file-from-home-dir-from-ARDUINO_CREATE_AGENT_CONFIG-env")
+}
+
+// TestIfHomeDoesNotContainConfigTheDefaultConfigAreCopied tests the case when the default config.ini is copied into $HOME/.config/ArduinoCreateAgent/config.ini
+// from the default config.ini
+// If the ARDUINO_CREATE_AGENT_CONFIG is NOT set and the config.ini does not exist in HOME directory
+// then it copies the default config (the config.ini) into the HOME directory
+func TestIfHomeDoesNotContainConfigTheDefaultConfigAreCopied(t *testing.T) {
+	if runtime.GOOS != "linux" {
+		t.Skip("Skipping test on non-linux OS")
+	}
+	//  $HOME must be always set, otherwise panic
+	os.Setenv("HOME", "./testdata/home-without-config")
+
+	os.Unsetenv("ARDUINO_CREATE_AGENT_CONFIG")
+	// Clean the home folder by deleting the config.ini
+	os.Remove("./testdata/home-without-config/.config/ArduinoCreateAgent/config.ini")
+
+	configPath := GetConfigPath()
+
+	assert.Equal(t, "testdata/home-without-config/.config/ArduinoCreateAgent/config.ini", configPath.String())
+	checkIniName(t, configPath, "") // the name of the default config is missing (an empty string)
+
+	givenContent, err := paths.New(configPath.String()).ReadFile()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	assert.Equal(t, string(configContent), string(givenContent))
+}
+
+// TestGetConfigPathPanicIfPathDoesNotExist tests that it panics if the ARDUINO_CREATE_AGENT_CONFIG env variable point to an non-existing path
+func TestGetConfigPathPanicIfPathDoesNotExist(t *testing.T) {
+	if runtime.GOOS != "linux" {
+		t.Skip("Skipping test on non-linux OS")
+	}
+	os.Setenv("HOME", "./testdata/dummyhome")
+	defer os.Unsetenv("HOME")
+
+	os.Setenv("ARDUINO_CREATE_AGENT_CONFIG", "./testdata/a-not-existing-path/config.ini")
+
+	defer func() {
+		if r := recover(); r != nil {
+			assert.Equal(t, fmt.Sprintf("config from env var %s does not exists", "./testdata/a-not-existing-path/config.ini"), r)
+		} else {
+			t.Fatal("Expected panic but did not occur")
+		}
+	}()
+
+	configPath := GetConfigPath()
+
+	assert.Equal(t, "testdata/fromxdghome/ArduinoCreateAgent/config.ini", configPath.String())
+	checkIniName(t, configPath, "this-is-a-config-file-from-xdghome-dir")
+}
+
+func checkIniName(t *testing.T, confipath *paths.Path, expected string) {
+	cfg, err := ini.LoadSources(ini.LoadOptions{IgnoreInlineComment: true, AllowPythonMultilineValues: true}, confipath.String())
+	if err != nil {
+		t.Fatal(err)
+	}
+	defaultSection, err := cfg.GetSection("")
+	if err != nil {
+		t.Fatal(err)
+	}
+	assert.Equal(t, expected, defaultSection.Key("name").String())
+}

config/testdata/.gitignore

@@ -0,0 +1,5 @@
+# The function that read the HOME fodler automatically creates the home folder if missing.
+
+# This are the folders that are created for testing purpose and there is no need to be tracked
+dummyhome
+home-without-config

config/testdata/from-arduino-create-agent-config-env/config.ini

@@ -0,0 +1,9 @@
+name = this-is-a-config-file-from-home-dir-from-ARDUINO_CREATE_AGENT_CONFIG-env
+gc = std
+hostname = an-hostname
+regex = usb|acm|com
+v = true
+appName = CreateAgent/Stable
+updateUrl = https://downloads.arduino.cc/
+origins = https://local.arduino.cc:8000, https://local.arduino.cc:8001
+crashreport = false

config/testdata/fromhome/.config/ArduinoCreateAgent/config.ini

@@ -0,0 +1,9 @@
+name = this-is-a-config-file-from-home-dir
+gc = std
+hostname = an-hostname
+regex = usb|acm|com
+v = true
+appName = an-app-n
+updateUrl = https://downloads.arduino.cc/
+origins = https://local.arduino.cc:8000, https://local.arduino.cc:8001, https://*.iot-cloud-arduino-cc.pages.dev
+crashreport = false

config/testdata/fromxdghome/ArduinoCreateAgent/config.ini

@@ -0,0 +1,10 @@
+name = this-is-a-config-file-from-xdghome-dir
+gc = std
+hostname = an-hostname
+regex = usb|acm|com
+v = true
+appName = CreateAgent/Stable
+updateUrl = https://downloads.arduino.cc/
+origins = https://local.arduino.cc:8000
+crashreport = false
+autostartMacOS = true

main.go

@@ -141,8 +141,10 @@ func main() {
 	// Check if certificates made with Agent <=1.2.7 needs to be moved over the new location
 	cert.MigrateCertificatesGeneratedWithOldAgentVersions(config.GetCertificatesDir())
 
+	configPath := config.GetConfigPath()
+
 	// Launch main loop in a goroutine
-	go loop()
+	go loop(configPath)
 
 	// SetupSystray is the main thread
 	configDir := config.GetDefaultConfigDir()
@@ -155,6 +157,7 @@ func main() {
 		AdditionalConfig: *additionalConfig,
 		ConfigDir:        configDir,
 	}
+	Systray.SetCurrentConfigFile(configPath)
 
 	if src, err := os.Executable(); err != nil {
 		panic(err)
@@ -165,11 +168,15 @@ func main() {
 	}
 }
 
-func loop() {
+func loop(configPath *paths.Path) {
 	if *hibernate {
 		return
 	}
 
+	if configPath == nil {
+		log.Panic("configPath is nil")
+	}
+
 	log.SetLevel(log.InfoLevel)
 	log.SetOutput(os.Stdout)
 
@@ -188,39 +195,6 @@ func loop() {
 		h.broadcastSys <- mapB
 	}
 
-	// Let's handle the config
-	configDir := config.GetDefaultConfigDir()
-	var configPath *paths.Path
-
-	// see if the env var is defined, if it is take the config from there, this will override the default path
-	if envConfig := os.Getenv("ARDUINO_CREATE_AGENT_CONFIG"); envConfig != "" {
-		configPath = paths.New(envConfig)
-		if configPath.NotExist() {
-			log.Panicf("config from env var %s does not exists", envConfig)
-		}
-		log.Infof("using config from env variable: %s", configPath)
-	} else if defaultConfigPath := configDir.Join("config.ini"); defaultConfigPath.Exist() {
-		// by default take the config from the ~/.arduino-create/config.ini file
-		configPath = defaultConfigPath
-		log.Infof("using config from default: %s", configPath)
-	} else {
-		// Fall back to the old config.ini location
-		src, _ := os.Executable()
-		oldConfigPath := paths.New(src).Parent().Join("config.ini")
-		if oldConfigPath.Exist() {
-			err := oldConfigPath.CopyTo(defaultConfigPath)
-			if err != nil {
-				log.Errorf("cannot copy old %s, to %s, generating new config", oldConfigPath, configPath)
-			} else {
-				configPath = defaultConfigPath
-				log.Infof("copied old %s, to %s", oldConfigPath, configPath)
-			}
-		}
-	}
-	if configPath == nil {
-		configPath = config.GenerateConfig(configDir)
-	}
-
 	// if the default browser is Safari, prompt the user to install HTTPS certificates
 	// and eventually install them
 	if runtime.GOOS == "darwin" {
@@ -258,7 +232,6 @@ func loop() {
 	if err != nil {
 		log.Panicf("cannot parse arguments: %s", err)
 	}
-	Systray.SetCurrentConfigFile(configPath)
 
 	// Parse additional ini config if defined
 	if len(*additionalConfig) > 0 {