make mox compile on windows, without "mox serve" but with working "mo…

…x localserve"

getting mox to compile required changing code in only a few places where
package "syscall" was used: for accessing file access times and for umask
handling. an open problem is how to start a process as an unprivileged user on
windows.  that's why "mox serve" isn't implemented yet. and just finding a way
to implement it now may not be good enough in the near future: we may want to
starting using a more complete privilege separation approach, with a process
handling sensitive tasks (handling private keys, authentication), where we may
want to pass file descriptors between processes. how would that work on
windows?

anyway, getting mox to compile for windows doesn't mean it works properly on
windows. the largest issue: mox would normally open a file, rename or remove
it, and finally close it. this happens during message delivery. that doesn't
work on windows, the rename/remove would fail because the file is still open.
so this commit swaps many "remove" and "close" calls. renames are a longer
story: message delivery had two ways to deliver: with "consuming" the
(temporary) message file (which would rename it to its final destination), and
without consuming (by hardlinking the file, falling back to copying). the last
delivery to a recipient of a message (and the only one in the common case of a
single recipient) would consume the message, and the earlier recipients would
not.  during delivery, the already open message file was used, to parse the
message.  we still want to use that open message file, and the caller now stays
responsible for closing it, but we no longer try to rename (consume) the file.
we always hardlink (or copy) during delivery (this works on windows), and the
caller is responsible for closing and removing (in that order) the original
temporary file. this does cost one syscall more. but it makes the delivery code
(responsibilities) a bit simpler.

there is one more obvious issue: the file system path separator. mox already
used the "filepath" package to join paths in many places, but not everywhere.
and it still used strings with slashes for local file access. with this commit,
the code now uses filepath.FromSlash for path strings with slashes, uses
"filepath" in a few more places where it previously didn't. also switches from
"filepath" to regular "path" package when handling mailbox names in a few
places, because those always use forward slashes, regardless of local file
system conventions.  windows can handle forward slashes when opening files, so
test code that passes path strings with forward slashes straight to go stdlib
file i/o functions are left unchanged to reduce code churn. the regular
non-test code, or test code that uses path strings in places other than
standard i/o functions, does have the paths converted for consistent paths
(otherwise we would end up with paths with mixed forward/backward slashes in
log messages).

windows cannot dup a listening socket. for "mox localserve", it isn't
important, and we can work around the issue. the current approach for "mox
serve" (forking a process and passing file descriptors of listening sockets on
"privileged" ports) won't work on windows. perhaps it isn't needed on windows,
and any user can listen on "privileged" ports? that would be welcome.

on windows, os.Open cannot open a directory, so we cannot call Sync on it after
message delivery. a cursory internet search indicates that directories cannot
be synced on windows. the story is probably much more nuanced than that, with
long deep technical details/discussions/disagreement/confusion, like on unix.
for "mox localserve" we can get away with making syncdir a no-op.

Makefile

@@ -107,3 +107,19 @@ docker:
 
 docker-release:
 	./docker-release.sh
+
+buildall:
+	GOOS=linux GOARCH=arm go build
+	GOOS=linux GOARCH=arm64 go build
+	GOOS=linux GOARCH=amd64 go build
+	GOOS=linux GOARCH=386 go build
+	GOOS=openbsd GOARCH=amd64 go build
+	GOOS=freebsd GOARCH=amd64 go build
+	GOOS=netbsd GOARCH=amd64 go build
+	GOOS=darwin GOARCH=amd64 go build
+	GOOS=dragonfly GOARCH=amd64 go build
+	GOOS=illumos GOARCH=amd64 go build
+	GOOS=solaris GOARCH=amd64 go build
+	GOOS=aix GOARCH=ppc64 go build
+	GOOS=windows GOARCH=amd64 go build
+	# no plan9 for now

README.md

@@ -91,7 +91,10 @@ Verify you have a working mox binary:
 
 	./mox version
 
-Mox only compiles for/works on unix systems, not on Plan 9 or Windows.
+Mox only compiles for and fully works on unix systems. Mox also compiles for
+Windows, but "mox serve" does not yet work, though "mox localserve" (for a
+local test instance) and most other subcommands do. Mox does not compile for
+Plan 9.
 
 You can also run mox with docker image `r.xmox.nl/mox`, with tags like `v0.0.1`
 and `v0.0.1-go1.20.1-alpine3.17.2`, see https://r.xmox.nl/r/mox/. Though new

autotls/autotls.go

@@ -83,7 +83,7 @@ func Load(name, acmeDir, contactEmail, directoryURL string, getPrivateKey func(h
 	}
 
 	// Load identity key if it exists. Otherwise, create a new key.
-	p := filepath.Join(acmeDir + "/" + name + ".key")
+	p := filepath.Join(acmeDir, name+".key")
 	var key crypto.Signer
 	f, err := os.Open(p)
 	if f != nil {
@@ -135,7 +135,7 @@ func Load(name, acmeDir, contactEmail, directoryURL string, getPrivateKey func(h
 	}
 
 	m := &autocert.Manager{
-		Cache:  dirCache(acmeDir + "/keycerts/" + name),
+		Cache:  dirCache(filepath.Join(acmeDir, "keycerts", name)),
 		Prompt: autocert.AcceptTOS,
 		Email:  contactEmail,
 		Client: &acme.Client{

backup.go

@@ -372,7 +372,7 @@ func backupctl(ctx context.Context, ctl *ctl) {
 
 		xvlog("queue backed finished", mlog.Field("duration", time.Since(tmQueue)))
 	}
-	backupQueue("queue/index.db")
+	backupQueue(filepath.FromSlash("queue/index.db"))
 
 	backupAccount := func(acc *store.Account) {
 		defer acc.Close()
@@ -469,7 +469,7 @@ func backupctl(ctx context.Context, ctl *ctl) {
 				return nil
 			}
 			ap := filepath.Join("accounts", acc.Name, p)
-			if strings.HasPrefix(p, "msg/") {
+			if strings.HasPrefix(p, "msg"+string(filepath.Separator)) {
 				xwarnx("backing up unrecognized file in account message directory (should be moved away)", nil, mlog.Field("path", ap))
 			} else {
 				xwarnx("backing up unrecognized file in account directory", nil, mlog.Field("path", ap))

ctl.go

@@ -320,12 +320,11 @@ func servectlcmd(ctx context.Context, ctl *ctl, shutdown func()) {
 		msgFile, err := store.CreateMessageTemp("ctl-deliver")
 		ctl.xcheck(err, "creating temporary message file")
 		defer func() {
-			if msgFile != nil {
-				err := os.Remove(msgFile.Name())
-				log.Check(err, "removing temporary message file", mlog.Field("path", msgFile.Name()))
-				err = msgFile.Close()
-				log.Check(err, "closing temporary message file")
-			}
+			name := msgFile.Name()
+			err := msgFile.Close()
+			log.Check(err, "closing temporary message file")
+			err = os.Remove(name)
+			log.Check(err, "removing temporary message file", mlog.Field("path", name))
 		}()
 		mw := message.NewWriter(msgFile)
 		ctl.xwriteok()
@@ -340,14 +339,11 @@ func servectlcmd(ctx context.Context, ctl *ctl, shutdown func()) {
 		}
 
 		a.WithWLock(func() {
-			err := a.DeliverDestination(log, addr, m, msgFile, true)
+			err := a.DeliverDestination(log, addr, m, msgFile)
 			ctl.xcheck(err, "delivering message")
 			log.Info("message delivered through ctl", mlog.Field("to", to))
 		})
 
-		err = msgFile.Close()
-		log.Check(err, "closing delivered message file")
-		msgFile = nil
 		err = a.Close()
 		ctl.xcheck(err, "closing account")
 		ctl.xwriteok()

ctl_test.go

@@ -7,6 +7,7 @@ import (
 	"flag"
 	"net"
 	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/mjl-/mox/dmarcdb"
@@ -33,8 +34,8 @@ func tcheck(t *testing.T, err error, errmsg string) {
 // unhandled errors would cause a panic.
 func TestCtl(t *testing.T) {
 	os.RemoveAll("testdata/ctl/data")
-	mox.ConfigStaticPath = "testdata/ctl/mox.conf"
-	mox.ConfigDynamicPath = "testdata/ctl/domains.conf"
+	mox.ConfigStaticPath = filepath.FromSlash("testdata/ctl/mox.conf")
+	mox.ConfigDynamicPath = filepath.FromSlash("testdata/ctl/domains.conf")
 	if errs := mox.LoadConfig(ctxbg, true, false); len(errs) > 0 {
 		t.Fatalf("loading mox config: %v", errs)
 	}
@@ -147,13 +148,13 @@ func TestCtl(t *testing.T) {
 	})
 
 	// Export data, import it again
-	xcmdExport(true, []string{"testdata/ctl/data/tmp/export/mbox/", "testdata/ctl/data/accounts/mjl"}, nil)
-	xcmdExport(false, []string{"testdata/ctl/data/tmp/export/maildir/", "testdata/ctl/data/accounts/mjl"}, nil)
+	xcmdExport(true, []string{filepath.FromSlash("testdata/ctl/data/tmp/export/mbox/"), filepath.FromSlash("testdata/ctl/data/accounts/mjl")}, nil)
+	xcmdExport(false, []string{filepath.FromSlash("testdata/ctl/data/tmp/export/maildir/"), filepath.FromSlash("testdata/ctl/data/accounts/mjl")}, nil)
 	testctl(func(ctl *ctl) {
-		ctlcmdImport(ctl, true, "mjl", "inbox", "testdata/ctl/data/tmp/export/mbox/Inbox.mbox")
+		ctlcmdImport(ctl, true, "mjl", "inbox", filepath.FromSlash("testdata/ctl/data/tmp/export/mbox/Inbox.mbox"))
 	})
 	testctl(func(ctl *ctl) {
-		ctlcmdImport(ctl, false, "mjl", "inbox", "testdata/ctl/data/tmp/export/maildir/Inbox")
+		ctlcmdImport(ctl, false, "mjl", "inbox", filepath.FromSlash("testdata/ctl/data/tmp/export/maildir/Inbox"))
 	})
 
 	// "recalculatemailboxcounts"
@@ -177,12 +178,12 @@ func TestCtl(t *testing.T) {
 			m.Size = int64(len(content))
 			msgf, err := store.CreateMessageTemp("ctltest")
 			tcheck(t, err, "create temp file")
+			defer os.Remove(msgf.Name())
+			defer msgf.Close()
 			_, err = msgf.Write(content)
 			tcheck(t, err, "write message file")
-			err = acc.DeliverMailbox(xlog, "Inbox", m, msgf, true)
+			err = acc.DeliverMailbox(xlog, "Inbox", m, msgf)
 			tcheck(t, err, "deliver message")
-			err = msgf.Close()
-			tcheck(t, err, "close message file")
 		}
 
 		var msgBadSize store.Message
@@ -236,13 +237,13 @@ func TestCtl(t *testing.T) {
 		os.RemoveAll("testdata/ctl/data/tmp/backup-data")
 		err := os.WriteFile("testdata/ctl/data/receivedid.key", make([]byte, 16), 0600)
 		tcheck(t, err, "writing receivedid.key")
-		ctlcmdBackup(ctl, "testdata/ctl/data/tmp/backup-data", false)
+		ctlcmdBackup(ctl, filepath.FromSlash("testdata/ctl/data/tmp/backup-data"), false)
 	})
 
 	// Verify the backup.
 	xcmd := cmd{
 		flag:     flag.NewFlagSet("", flag.ExitOnError),
-		flagArgs: []string{"testdata/ctl/data/tmp/backup-data"},
+		flagArgs: []string{filepath.FromSlash("testdata/ctl/data/tmp/backup-data")},
 	}
 	cmdVerifydata(&xcmd)
 }

develop.txt

@@ -1,5 +1,15 @@
 This file has notes useful for mox developers.
 
+# Code style & guidelines
+
+- Keep the same style as existing code.
+- For Windows: use package "path/filepath" when dealing with files/directories.
+  Test code can pass forward-slashed paths directly to standard library functions,
+  but use proper filepath functions when parameters are passed and in non-test
+  code.  Mailbox names always use forward slash, so use package "path" for mailbox
+  name/path manipulation. Do not remove/rename files that are still open.
+
+
 # TLS certificates
 
 https://github.com/cloudflare/cfssl is useful for testing with TLS

dmarcdb/db_test.go

@@ -17,16 +17,17 @@ var ctxbg = context.Background()
 
 func TestDMARCDB(t *testing.T) {
 	mox.Shutdown = ctxbg
-	mox.ConfigStaticPath = "../testdata/dmarcdb/fake.conf"
+	mox.ConfigStaticPath = filepath.FromSlash("../testdata/dmarcdb/fake.conf")
 	mox.Conf.Static.DataDir = "."
 
 	dbpath := mox.DataDirPath("dmarcrpt.db")
 	os.MkdirAll(filepath.Dir(dbpath), 0770)
-	defer os.Remove(dbpath)
 
 	if err := Init(); err != nil {
 		t.Fatalf("init database: %s", err)
 	}
+	defer os.Remove(dbpath)
+	defer DB.Close()
 
 	feedback := &dmarcrpt.Feedback{
 		ReportMetadata: dmarcrpt.ReportMetadata{

dmarcrpt/parse_test.go

@@ -2,6 +2,7 @@ package dmarcrpt
 
 import (
 	"os"
+	"path/filepath"
 	"reflect"
 	"strings"
 	"testing"
@@ -122,14 +123,14 @@ func TestParseReport(t *testing.T) {
 }
 
 func TestParseMessageReport(t *testing.T) {
-	const dir = "../testdata/dmarc-reports"
+	dir := filepath.FromSlash("../testdata/dmarc-reports")
 	files, err := os.ReadDir(dir)
 	if err != nil {
 		t.Fatalf("listing dmarc report emails: %s", err)
 	}
 
 	for _, file := range files {
-		p := dir + "/" + file.Name()
+		p := filepath.Join(dir, file.Name())
 		f, err := os.Open(p)
 		if err != nil {
 			t.Fatalf("open %q: %s", p, err)

doc.go

@@ -89,6 +89,8 @@ IMAP. HTTP listeners are started for the admin/account web interfaces, and for
 automated TLS configuration. Missing essential TLS certificates are immediately
 requested, other TLS certificates are requested on demand.
 
+Only implemented on unix systems, not Windows.
+
 	usage: mox serve
 
 # mox quickstart

dsn/dsn_test.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"net"
+	"path/filepath"
 	"reflect"
 	"strings"
 	"testing"
@@ -131,7 +132,7 @@ func TestDSN(t *testing.T) {
 
 	// Test for valid DKIM signature.
 	mox.Context = context.Background()
-	mox.ConfigStaticPath = "../testdata/dsn/mox.conf"
+	mox.ConfigStaticPath = filepath.FromSlash("../testdata/dsn/mox.conf")
 	mox.MustLoadConfig(true, false)
 	msgbuf, err = m.Compose(log, false)
 	if err != nil {

gentestdata.go

@@ -85,8 +85,8 @@ Accounts:
 				IgnoreWords: 0.1
 `
 
-	mox.ConfigStaticPath = "/tmp/mox-bogus/mox.conf"
-	mox.ConfigDynamicPath = "/tmp/mox-bogus/domains.conf"
+	mox.ConfigStaticPath = filepath.FromSlash("/tmp/mox-bogus/mox.conf")
+	mox.ConfigDynamicPath = filepath.FromSlash("/tmp/mox-bogus/domains.conf")
 	mox.Conf.DynamicLastCheck = time.Now() // Should prevent warning.
 	mox.Conf.Static = config.Static{
 		DataDir: destDataDir,
@@ -228,11 +228,12 @@ Accounts:
 	prefix := []byte{}
 	mf := tempfile()
 	xcheckf(err, "temp file for queue message")
+	defer os.Remove(mf.Name())
 	defer mf.Close()
 	const qmsg = "From: <[email protected]>\r\nTo: <[email protected]>\r\nSubject: test\r\n\r\nthe message...\r\n"
 	_, err = fmt.Fprint(mf, qmsg)
 	xcheckf(err, "writing message")
-	_, err = queue.Add(ctxbg, log, "test0", mailfrom, rcptto, false, false, int64(len(qmsg)), "<test@localhost>", prefix, mf, nil, true)
+	_, err = queue.Add(ctxbg, log, "test0", mailfrom, rcptto, false, false, int64(len(qmsg)), "<test@localhost>", prefix, mf, nil)
 	xcheckf(err, "enqueue message")
 
 	// Create three accounts.
@@ -283,10 +284,14 @@ Accounts:
 		xcheckf(err, "creating temp file for delivery")
 		_, err = fmt.Fprint(mf, msg)
 		xcheckf(err, "writing deliver message to file")
-		err = accTest1.DeliverMessage(log, tx, &m, mf, true, false, true, false)
+		err = accTest1.DeliverMessage(log, tx, &m, mf, false, true, false)
+
+		mfname := mf.Name()
 		xcheckf(err, "add message to account test1")
 		err = mf.Close()
 		xcheckf(err, "closing file")
+		err = os.Remove(mfname)
+		xcheckf(err, "removing temp message file")
 
 		err = tx.Get(&inbox)
 		xcheckf(err, "get inbox")
@@ -339,10 +344,14 @@ Accounts:
 		xcheckf(err, "creating temp file for delivery")
 		_, err = fmt.Fprint(mf0, msg0)
 		xcheckf(err, "writing deliver message to file")
-		err = accTest2.DeliverMessage(log, tx, &m0, mf0, true, false, false, false)
+		err = accTest2.DeliverMessage(log, tx, &m0, mf0, false, false, false)
 		xcheckf(err, "add message to account test2")
+
+		mf0name := mf0.Name()
 		err = mf0.Close()
 		xcheckf(err, "closing file")
+		err = os.Remove(mf0name)
+		xcheckf(err, "removing temp message file")
 
 		err = tx.Get(&inbox)
 		xcheckf(err, "get inbox")
@@ -366,10 +375,14 @@ Accounts:
 		xcheckf(err, "creating temp file for delivery")
 		_, err = fmt.Fprint(mf1, msg1)
 		xcheckf(err, "writing deliver message to file")
-		err = accTest2.DeliverMessage(log, tx, &m1, mf1, true, false, false, false)
+		err = accTest2.DeliverMessage(log, tx, &m1, mf1, false, false, false)
 		xcheckf(err, "add message to account test2")
+
+		mf1name := mf1.Name()
 		err = mf1.Close()
 		xcheckf(err, "closing file")
+		err = os.Remove(mf1name)
+		xcheckf(err, "removing temp message file")
 
 		err = tx.Get(&sent)
 		xcheckf(err, "get sent")

go.mod

@@ -3,7 +3,7 @@ module github.com/mjl-/mox
 go 1.20
 
 require (
-	github.com/mjl-/adns v0.0.0-20231009145311-e3834995f16c
+	github.com/mjl-/adns v0.0.0-20231013194548-ea0378d616ab
 	github.com/mjl-/autocert v0.0.0-20231013072455-c361ae2e20a6
 	github.com/mjl-/bstore v0.0.2
 	github.com/mjl-/sconf v0.0.5

go.sum

@@ -145,10 +145,8 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/matttproud/golang_protobuf_extensions v1.0.1 h1:4hp9jkHxhMHkqkrB3Ix0jegS5sx/RkqARlsWZ6pIwiU=
 github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0=
-github.com/mjl-/adns v0.0.0-20231009145311-e3834995f16c h1:ZOr9KnCxfAwJWSeZn8Qs6cSF7TrmBa8hVIpLcEvx/Ec=
-github.com/mjl-/adns v0.0.0-20231009145311-e3834995f16c/go.mod h1:JWhGACVviyVUEra9Zv1M8JMkDVXArVt+AIXjTXtuwb4=
-github.com/mjl-/autocert v0.0.0-20231009155929-d0d48f2f0290 h1:0hCRSu8+XCZ2cSRW+ZtP/7L5wMYjOKFSQthoyj+4cN8=
-github.com/mjl-/autocert v0.0.0-20231009155929-d0d48f2f0290/go.mod h1:taMFU86abMxKLPV4Bynhv8enbYmS67b8LG80qZv2Qus=
+github.com/mjl-/adns v0.0.0-20231013194548-ea0378d616ab h1:fL+dZP+IxX08+ugLq42bkvOfV42muXET+T+Ei1K16bI=
+github.com/mjl-/adns v0.0.0-20231013194548-ea0378d616ab/go.mod h1:v47qUMJnipnmDTRGaHwpCwzE6oypa5K33mUvBfzZBn8=
 github.com/mjl-/autocert v0.0.0-20231013072455-c361ae2e20a6 h1:TEXyTghAN9pmV2ffzdnhmzkML08e1Z/oGywJ9eunbRI=
 github.com/mjl-/autocert v0.0.0-20231013072455-c361ae2e20a6/go.mod h1:taMFU86abMxKLPV4Bynhv8enbYmS67b8LG80qZv2Qus=
 github.com/mjl-/bstore v0.0.2 h1:4fdpIOY/+Dv1dBHyzdqa4PD90p8Mz86FeyRpI4qcehw=

http/atime.go

@@ -1,9 +1,16 @@
-//go:build !netbsd && !freebsd && !darwin
+//go:build !netbsd && !freebsd && !darwin && !windows
 
 package http
 
-import "syscall"
+import (
+	"fmt"
+	"syscall"
+)
 
-func statAtime(sys *syscall.Stat_t) int64 {
-	return int64(sys.Atim.Sec)*1000*1000*1000 + int64(sys.Atim.Nsec)
+func statAtime(sys any) (int64, error) {
+	x, ok := sys.(*syscall.Stat_t)
+	if !ok {
+		return 0, fmt.Errorf("sys is a %T, expected *syscall.Stat_t", sys)
+	}
+	return int64(x.Atim.Sec)*1000*1000*1000 + int64(x.Atim.Nsec), nil
 }

http/atime_bsd.go

@@ -2,8 +2,15 @@
 
 package http
 
-import "syscall"
+import (
+	"fmt"
+	"syscall"
+)
 
-func statAtime(sys *syscall.Stat_t) int64 {
-	return int64(sys.Atimespec.Sec)*1000*1000*1000 + int64(sys.Atimespec.Nsec)
+func statAtime(sys any) (int64, error) {
+	x, ok := sys.(*syscall.Stat_t)
+	if !ok {
+		return 0, fmt.Errorf("stat sys is a %T, expected *syscall.Stat_t", sys)
+	}
+	return int64(x.Atimespec.Sec)*1000*1000*1000 + int64(x.Atimespec.Nsec), nil
 }