GODT-1523: Reduce unnecessary shell executions. Inspired by @kortschak.

.gitlab-ci.yml

@@ -180,6 +180,7 @@ build-linux-qa:
     - export PATH=$GOPATH/bin:$PATH
     - export CGO_CPPFLAGS='-Wno-error -Wno-nullability-completeness -Wno-expansion-to-defined -Wno-builtin-requires-header'
   script:
+    - go version
     - make build
     - git diff && git diff-index --quiet HEAD
   cache: {}

cmd/launcher/main.go

@@ -98,7 +98,7 @@ func main() { //nolint:funlen
 		logrus.WithError(err).Fatal("Failed to determine path to launcher")
 	}
 
-	cmd := execabs.Command(exe, appendLauncherPath(launcher, os.Args[1:])...) // nolint:gosec
+	cmd := execabs.Command(exe, appendLauncherPath(launcher, os.Args[1:])...) //nolint:gosec
 
 	cmd.Stdin = os.Stdin
 	cmd.Stdout = os.Stdout

go.mod

@@ -73,7 +73,7 @@ require (
 	golang.org/x/net v0.0.0-20220425223048-2871e0cb64e4
 	golang.org/x/sys v0.0.0-20220111092808-5a964db01320
 	golang.org/x/text v0.3.7
-	howett.net/plist v1.0.0 // indirect
+	howett.net/plist v1.0.0
 )
 
 replace (

internal/config/useragent/platform.go

@@ -22,7 +22,6 @@ import (
 	"strings"
 
 	"github.com/Masterminds/semver/v3"
-	"golang.org/x/sys/execabs"
 )
 
 // IsCatalinaOrNewer checks whether the host is MacOS Catalina 10.15.x or higher.
@@ -35,20 +34,20 @@ func IsBigSurOrNewer() bool {
 	return isThisDarwinNewerOrEqual(getMinBigSur())
 }
 
-func getMinCatalina() *semver.Version { return semver.MustParse("10.15.0") }
-func getMinBigSur() *semver.Version   { return semver.MustParse("10.16.0") }
+func getMinCatalina() *semver.Version { return semver.MustParse("19.0.0") }
+func getMinBigSur() *semver.Version   { return semver.MustParse("20.0.0") }
 
 func isThisDarwinNewerOrEqual(minVersion *semver.Version) bool {
 	if runtime.GOOS != "darwin" {
 		return false
 	}
 
-	rawVersion, err := execabs.Command("sw_vers", "-productVersion").Output()
+	rawVersion, err := getDarwinVersion()
 	if err != nil {
 		return false
 	}
 
-	return isVersionEqualOrNewer(minVersion, strings.TrimSpace(string(rawVersion)))
+	return isVersionEqualOrNewer(minVersion, strings.TrimSpace(rawVersion))
 }
 
 // isVersionEqualOrNewer is separated to be able to run test on other than darwin.

internal/config/useragent/platform_darwin.go

@@ -0,0 +1,29 @@
+// Copyright (c) 2022 Proton AG
+//
+// This file is part of Proton Mail Bridge.
+//
+// Proton Mail Bridge is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Proton Mail Bridge is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Proton Mail Bridge. If not, see <https://www.gnu.org/licenses/>.
+
+//go:build darwin
+// +build darwin
+
+package useragent
+
+import (
+	"syscall"
+)
+
+func getDarwinVersion() (string, error) {
+	return syscall.Sysctl("kern.osrelease")
+}

internal/config/useragent/platform_default.go

@@ -0,0 +1,27 @@
+// Copyright (c) 2022 Proton AG
+//
+// This file is part of Proton Mail Bridge.
+//
+// Proton Mail Bridge is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Proton Mail Bridge is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Proton Mail Bridge. If not, see <https://www.gnu.org/licenses/>.
+
+//go:build !darwin
+// +build !darwin
+
+package useragent
+
+import "errors"
+
+func getDarwinVersion() (string, error) {
+	return "", errors.New("implemented only for darwin")
+}

internal/config/useragent/platform_test.go

@@ -26,15 +26,10 @@ import (
 func TestIsVersionCatalinaOrNewer(t *testing.T) {
 	testData := map[struct{ version string }]bool{
 		{""}:       false,
-		{"9.0.0"}:    false,
-		{"9.15.0"}:   false,
-		{"10.13.0"}:  false,
-		{"10.14.0"}:  false,
-		{"10.14.99"}: false,
-		{"10.15.0"}:  true,
-		{"10.16.0"}:  true,
-		{"11.0.0"}:   true,
-		{"11.1"}:     true,
+		{"18.0.0"}: false,
+		{"19.0.0"}: true,
+		{"20.0.0"}: true,
+		{"21.0.0"}: true,
 	}
 
 	for args, exp := range testData {
@@ -46,15 +41,10 @@ func TestIsVersionCatalinaOrNewer(t *testing.T) {
 func TestIsVersionBigSurOrNewer(t *testing.T) {
 	testData := map[struct{ version string }]bool{
 		{""}:       false,
-		{"9.0.0"}:    false,
-		{"9.15.0"}:   false,
-		{"10.13.0"}:  false,
-		{"10.14.0"}:  false,
-		{"10.14.99"}: false,
-		{"10.15.0"}:  false,
-		{"10.16.0"}:  true,
-		{"11.0.0"}:   true,
-		{"11.1"}:     true,
+		{"18.0.0"}: false,
+		{"19.0.0"}: false,
+		{"20.0.0"}: true,
+		{"21.0.0"}: true,
 	}
 
 	for args, exp := range testData {

internal/frontend/clientconfig/config_applemail.go

@@ -28,10 +28,10 @@ import (
 	"strings"
 	"time"
 
-	"github.com/ProtonMail/proton-bridge/internal/bridge"
-	"github.com/ProtonMail/proton-bridge/internal/config/useragent"
-	"github.com/ProtonMail/proton-bridge/internal/frontend/types"
-	"github.com/ProtonMail/proton-bridge/pkg/mobileconfig"
+	"github.com/ProtonMail/proton-bridge/v2/internal/bridge"
+	"github.com/ProtonMail/proton-bridge/v2/internal/config/useragent"
+	"github.com/ProtonMail/proton-bridge/v2/internal/frontend/types"
+	"github.com/ProtonMail/proton-bridge/v2/pkg/mobileconfig"
 	"golang.org/x/sys/execabs"
 )
 

internal/frontend/theme/detect_darwin.go

@@ -21,15 +21,34 @@
 package theme
 
 import (
-	"strings"
+	"os"
+	"path/filepath"
 
-	"golang.org/x/sys/execabs"
+	"howett.net/plist"
 )
 
 func detectSystemTheme() Theme {
-	out, err := execabs.Command("defaults", "read", "-g", "AppleInterfaceStyle").Output() //nolint:gosec
-	if err == nil && strings.TrimSpace(string(out)) == "Dark" {
+	home, err := os.UserHomeDir()
+	if err != nil {
+		return Light
+	}
+
+	path := filepath.Join(home, "/Library/Preferences/.GlobalPreferences.plist")
+	prefFile, err := os.Open(path)
+	if err != nil {
+		return Light
+	}
+	defer prefFile.Close()
+
+	var data struct {
+		AppleInterfaceStyle string `plist:AppleInterfaceStyle`
+	}
+
+	dec := plist.NewDecoder(prefFile)
+	err = dec.Decode(&data)
+	if err == nil && data.AppleInterfaceStyle == "Dark" {
 		return Dark
 	}
+
 	return Light
 }

internal/smtp/user.go

@@ -313,16 +313,16 @@ func (su *smtpUser) Send(returnPath string, to []string, messageReader io.Reader
 
 	startTime := time.Now()
 	for isSending && time.Since(startTime) < 90*time.Second {
-		log.Debug("Message is still in send queue, waiting for a bit")
+		log.Warn("Message is still in send queue, waiting for a bit")
 		time.Sleep(15 * time.Second)
 		isSending, wasSent = su.backend.sendRecorder.isSendingOrSent(su.client(), sendRecorderMessageHash)
 	}
 	if isSending {
-		log.Debug("Message is still in send queue, returning error to prevent client from adding it to the sent folder prematurely")
+		log.Warn("Message is still in send queue, returning error to prevent client from adding it to the sent folder prematurely")
 		return errors.New("original message is still being sent")
 	}
 	if wasSent {
-		log.Debug("Message was already sent")
+		log.Warn("Message was already sent")
 		return nil
 	}
 

internal/store/ulimit_default.go

@@ -15,46 +15,55 @@
 // You should have received a copy of the GNU General Public License
 // along with Proton Mail Bridge. If not, see <https://www.gnu.org/licenses/>.
 
+//go:build !windows
+// +build !windows
+
 package store
 
 import (
-	"os"
 	"runtime"
-
-	"golang.org/x/sys/unix"
+	"syscall"
 )
 
+func getCurrentFDLimit() (int, error) {
+	var limits syscall.Rlimit
+	err := syscall.Getrlimit(syscall.RLIMIT_NOFILE, &limits)
+	if err != nil {
+		return 0, err
+	}
+	return int(limits.Cur), nil
+}
+
+func countOpenedFDs(limit int) int {
+	openedFDs := 0
+
+	for i := 0; i < limit; i++ {
+		_, _, err := syscall.Syscall(syscall.SYS_FCNTL, uintptr(i), uintptr(syscall.F_GETFL), 0)
+		if err == 0 {
+			openedFDs++
+		}
+	}
+
+	return openedFDs
+}
+
 func isFdCloseToULimit() bool {
 	if runtime.GOOS != "darwin" && runtime.GOOS != "linux" {
 		return false
 	}
 
-	var fdPath string
-	switch runtime.GOOS {
-	case "darwin":
-		fdPath = "/dev/fd"
-	case "linux":
-		fdPath = "/proc/self/fd"
-	}
-	f, err := os.Open(fdPath)
+	limit, err := getCurrentFDLimit()
 	if err != nil {
-		log.Warn("isFdCloseToULimit: ", err)
+		log.WithError(err).Error("Cannot get current FD limit")
 		return false
 	}
-	d, err := f.ReadDir(-1)
-	if err != nil {
-		log.Warn("isFdCloseToULimit: ", err)
-		return false
-	}
-	fd := len(d) - 1
 
-	var lim unix.Rlimit
-	err = unix.Getrlimit(unix.RLIMIT_NOFILE, &lim)
-	if err != nil {
-		log.Print(err)
-	}
-	ulimit := lim.Max
+	openedFDs := countOpenedFDs(limit)
 
-	log.Info("File descriptor check: num goroutines ", runtime.NumGoroutine(), " fd ", fd, " ulimit ", ulimit)
-	return fd >= int(0.95*float64(ulimit))
+	log.
+		WithField("noGoroutines", runtime.NumCgoCall()).
+		WithField("noFDs", openedFDs).
+		WithField("limitFD", limit).
+		Info("File descriptor check")
+	return openedFDs >= int(0.95*float64(limit))
 }

internal/store/ulimit_windows.go

@@ -0,0 +1,23 @@
+// Copyright (c) 2022 Proton AG
+//
+// This file is part of Proton Mail Bridge.
+//
+// Proton Mail Bridge is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Proton Mail Bridge is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Proton Mail Bridge. If not, see <https://www.gnu.org/licenses/>.
+
+//go:build windows
+// +build windows
+
+package store
+
+func isFdCloseToULimit() bool { return false }

internal/updater/sync_test.go

@@ -25,7 +25,6 @@ import (
 
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/require"
-	"golang.org/x/sys/execabs"
 )
 
 const (
@@ -39,77 +38,103 @@ const (
 func TestSyncFolder(t *testing.T) {
 	for _, srcType := range []string{EmptyType, FileType, SymlinkType, DirType} {
 		for _, dstType := range []string{EmptyType, FileType, SymlinkType, DirType} {
-			require.NoError(t, checkCopyWorks(srcType, dstType))
+			checkCopyWorks(t, srcType, dstType)
 			logrus.Warn("OK: from ", srcType, " to ", dstType)
 		}
 	}
 }
 
-func checkCopyWorks(srcType, dstType string) error {
+func checkCopyWorks(tb testing.TB, srcType, dstType string) {
+	r := require.New(tb)
 	dirName := "from_" + srcType + "_to_" + dstType
 	AppCacheDir := "/tmp"
 	srcDir := filepath.Join(AppCacheDir, "sync_src", dirName)
 	destDir := filepath.Join(AppCacheDir, "sync_dst", dirName)
 
 	// clear before
-	logrus.Info("remove all ", srcDir)
-	err := os.RemoveAll(srcDir)
-	if err != nil {
-		return err
-	}
-
-	logrus.Info("remove all ", destDir)
-	err = os.RemoveAll(destDir)
-	if err != nil {
-		return err
-	}
+	r.NoError(os.RemoveAll(srcDir))
+	r.NoError(os.RemoveAll(destDir))
 
 	// create
-	err = createTestFolder(srcDir, srcType)
-	if err != nil {
-		return err
-	}
-
-	err = createTestFolder(destDir, dstType)
-	if err != nil {
-		return err
-	}
+	r.NoError(createTestFolder(srcDir, srcType))
+	r.NoError(createTestFolder(destDir, dstType))
 
 	// copy
-	logrus.Info("Sync from ", srcDir, " to ", destDir)
-	err = syncFolders(destDir, srcDir)
-	if err != nil {
-		return err
-	}
+	r.NoError(syncFolders(destDir, srcDir))
 
 	// Check
-	logrus.Info("check ", srcDir, " and ", destDir)
-	err = checkThatFilesAreSame(srcDir, destDir)
-	if err != nil {
-		return err
-	}
+	checkThatFilesAreSame(r, srcDir, destDir)
 
 	// clear after
-	logrus.Info("remove all ", srcDir)
-	err = os.RemoveAll(srcDir)
-	if err != nil {
-		return err
-	}
-
-	logrus.Info("remove all ", destDir)
-	err = os.RemoveAll(destDir)
-	if err != nil {
-		return err
-	}
-
-	return err
+	r.NoError(os.RemoveAll(srcDir))
+	r.NoError(os.RemoveAll(destDir))
 }
 
-func checkThatFilesAreSame(src, dst string) error {
-	cmd := execabs.Command("diff", "-qr", src, dst) //nolint:gosec
-	cmd.Stderr = logrus.StandardLogger().WriterLevel(logrus.ErrorLevel)
-	cmd.Stdout = logrus.StandardLogger().WriterLevel(logrus.InfoLevel)
-	return cmd.Run()
+func checkThatFilesAreSame(r *require.Assertions, src, dst string) {
+	srcFiles, srcDirs, err := walkDir(src)
+	r.NoError(err)
+
+	dstFiles, dstDirs, err := walkDir(dst)
+	r.NoError(err)
+
+	r.ElementsMatch(srcFiles, dstFiles)
+	r.ElementsMatch(srcDirs, dstDirs)
+
+	for _, relPath := range srcFiles {
+		srcPath := filepath.Join(src, relPath)
+		r.FileExists(srcPath)
+
+		dstPath := filepath.Join(dst, relPath)
+		r.FileExists(dstPath)
+
+		srcInfo, err := os.Lstat(srcPath)
+		r.NoError(err)
+
+		dstInfo, err := os.Lstat(dstPath)
+		r.NoError(err)
+
+		r.Equal(srcInfo.Mode(), dstInfo.Mode())
+
+		if srcInfo.Mode()&os.ModeSymlink == os.ModeSymlink {
+			srcLnk, err := os.Readlink(srcPath)
+			r.NoError(err)
+
+			dstLnk, err := os.Readlink(dstPath)
+			r.NoError(err)
+
+			r.Equal(srcLnk, dstLnk)
+		} else {
+			srcContent, err := ioutil.ReadFile(srcPath)
+			r.NoError(err)
+
+			dstContent, err := ioutil.ReadFile(dstPath)
+			r.NoError(err)
+
+			r.Equal(srcContent, dstContent)
+		}
+	}
+}
+
+func walkDir(dir string) (files, dirs []string, err error) {
+	err = filepath.Walk(dir, func(path string, info os.FileInfo, errWalk error) error {
+		if errWalk != nil {
+			return errWalk
+		}
+
+		relPath, errRel := filepath.Rel(dir, path)
+		if errRel != nil {
+			return errRel
+		}
+
+		if info.IsDir() {
+			dirs = append(dirs, relPath)
+		} else {
+			files = append(files, relPath)
+		}
+
+		return nil
+	})
+	return
 }
 
 func createTestFolder(dirPath, dirType string) error {

test/context/context.go

@@ -176,4 +176,5 @@ func (ctx *TestContext) MessagePreparationFinished(username string) {
 
 func (ctx *TestContext) CredentialsFailsOnWrite(shouldFail bool) {
 	ctx.credStore.(*fakeCredStore).failOnWrite = shouldFail                                              //nolint:forcetypeassert
+	ctx.addCleanup(func() { ctx.credStore.(*fakeCredStore).failOnWrite = false }, "credentials-cleanup") //nolint:forcetypeassert
 }

test/context/credentials.go

@@ -35,7 +35,7 @@ type fakeCredStore struct {
 
 // newFakeCredStore returns a fake credentials store (optionally configured with the given credentials).
 func newFakeCredStore(initCreds ...*credentials.Credentials) (c *fakeCredStore) {
-	c = &fakeCredStore{credentials: map[string]*credentials.Credentials{}}
+	c = &fakeCredStore{credentials: map[string]*credentials.Credentials{}, failOnWrite: false}
 	for _, creds := range initCreds {
 		if creds == nil {
 			continue