Move the mount duplicate filter to the modules package

Also simplify the mount validation logic. There are plenty of ways a user can create mount configs that behaves oddly.

6 files changed, 152 insertions, 121 deletions

diff --git a/hugolib/config.go b/hugolib/config.go
index b7ac46171..07a8d4100 100644
--- a/hugolib/config.go
+++ b/hugolib/config.go
@@ -207,17 +207,25 @@ func LoadConfig(d ConfigSourceDescriptor, doWithConfig ...func(cfg config.Provid
 		return v, configFiles, err
 	}
 
-	mods, modulesConfigFiles, err := l.collectModules(modulesConfig, v)
-	if err != nil {
-		return v, configFiles, err
-	}
+	// Need to run these after the modules are loaded, but before
+	// they are finalized.
+	collectHook := func(m *modules.ModulesConfig) error {
+		if err := loadLanguageSettings(v, nil); err != nil {
+			return err
+		}
 
-	if err := loadLanguageSettings(v, nil); err != nil {
-		return v, configFiles, err
+		mods := m.ActiveModules
+
+		// Apply default project mounts.
+		if err := modules.ApplyProjectConfigDefaults(v, mods[len(mods)-1]); err != nil {
+			return err
+		}
+
+		return nil
 	}
 
-	// Apply default project mounts.
-	if err := modules.ApplyProjectConfigDefaults(v, mods[len(mods)-1]); err != nil {
+	_, modulesConfigFiles, err := l.collectModules(modulesConfig, v, collectHook)
+	if err != nil {
 		return v, configFiles, err
 	}
 
@@ -406,7 +414,7 @@ func (l configLoader) loadModulesConfig(v1 *viper.Viper) (modules.Config, error)
 	return modConfig, nil
 }
 
-func (l configLoader) collectModules(modConfig modules.Config, v1 *viper.Viper) (modules.Modules, []string, error) {
+func (l configLoader) collectModules(modConfig modules.Config, v1 *viper.Viper, hookBeforeFinalize func(m *modules.ModulesConfig) error) (modules.Modules, []string, error) {
 	workingDir := l.WorkingDir
 	if workingDir == "" {
 		workingDir = v1.GetString("workingDir")
@@ -420,16 +428,40 @@ func (l configLoader) collectModules(modConfig modules.Config, v1 *viper.Viper)
 	if err != nil {
 		return nil, nil, err
 	}
+
 	v1.Set("filecacheConfigs", filecacheConfigs)
 
+	var configFilenames []string
+
+	hook := func(m *modules.ModulesConfig) error {
+		for _, tc := range m.ActiveModules {
+			if tc.ConfigFilename() != "" {
+				if tc.Watch() {
+					configFilenames = append(configFilenames, tc.ConfigFilename())
+				}
+				if err := l.applyThemeConfig(v1, tc); err != nil {
+					return err
+				}
+			}
+		}
+
+		if hookBeforeFinalize != nil {
+			return hookBeforeFinalize(m)
+		}
+
+		return nil
+
+	}
+
 	modulesClient := modules.NewClient(modules.ClientConfig{
-		Fs:           l.Fs,
-		Logger:       l.Logger,
-		WorkingDir:   workingDir,
-		ThemesDir:    themesDir,
-		CacheDir:     filecacheConfigs.CacheDirModules(),
-		ModuleConfig: modConfig,
-		IgnoreVendor: ignoreVendor,
+		Fs:                 l.Fs,
+		Logger:             l.Logger,
+		HookBeforeFinalize: hook,
+		WorkingDir:         workingDir,
+		ThemesDir:          themesDir,
+		CacheDir:           filecacheConfigs.CacheDirModules(),
+		ModuleConfig:       modConfig,
+		IgnoreVendor:       ignoreVendor,
 	})
 
 	v1.Set("modulesClient", modulesClient)
@@ -442,22 +474,6 @@ func (l configLoader) collectModules(modConfig modules.Config, v1 *viper.Viper)
 	// Avoid recreating these later.
 	v1.Set("allModules", moduleConfig.ActiveModules)
 
-	if len(moduleConfig.ActiveModules) == 0 {
-		return nil, nil, nil
-	}
-
-	var configFilenames []string
-	for _, tc := range moduleConfig.ActiveModules {
-		if tc.ConfigFilename() != "" {
-			if tc.Watch() {
-				configFilenames = append(configFilenames, tc.ConfigFilename())
-			}
-			if err := l.applyThemeConfig(v1, tc); err != nil {
-				return nil, nil, err
-			}
-		}
-	}
-
 	if moduleConfig.GoModulesFilename != "" {
 		// We want to watch this for changes and trigger rebuild on version
 		// changes etc.
diff --git a/hugolib/filesystems/basefs.go b/hugolib/filesystems/basefs.go
index 57d9d158b..e550d7f35 100644
--- a/hugolib/filesystems/basefs.go
+++ b/hugolib/filesystems/basefs.go
@@ -18,7 +18,6 @@ package filesystems
 import (
 	"io"
 	"os"
-	"path"
 	"path/filepath"
 	"strings"
 	"sync"
@@ -454,7 +453,6 @@ func (b *sourceFilesystemsBuilder) createMainOverlayFs(p *paths.Paths) (*filesys
 	// The theme components are ordered from left to right.
 	// We need to revert it to get the
 	// overlay logic below working as expected, with the project on top (last).
-
 	for i, mod := range mods {
 		dir := mod.Dir()
 
@@ -463,11 +461,9 @@ func (b *sourceFilesystemsBuilder) createMainOverlayFs(p *paths.Paths) (*filesys
 		}
 
 		isMainProject := mod.Owner() == nil
-		// TODO(bep) embed mount + move any duplicate/overlap
 		modsReversed[i] = mountsDescriptor{
-			mounts:        mod.Mounts(),
+			Module:        mod,
 			dir:           dir,
-			watch:         mod.Watch(),
 			isMainProject: isMainProject,
 		}
 	}
@@ -500,36 +496,7 @@ func (b *sourceFilesystemsBuilder) createModFs(
 		return paths.AbsPathify(md.dir, path)
 	}
 
-	seen := make(map[string]bool)
-
-	var mounts []modules.Mount
-
-OUTER:
-	for i, mount := range md.mounts {
-		key := path.Join(mount.Lang, mount.Source, mount.Target)
-		if seen[key] {
-			continue
-		}
-		seen[key] = true
-
-		// Prevent overlapping mounts
-		for j, mount2 := range md.mounts {
-			if j == i || mount2.Target != mount.Target {
-				continue
-			}
-			source := mount.Source
-			if !strings.HasSuffix(source, filePathSeparator) {
-				source += filePathSeparator
-			}
-			if strings.HasPrefix(mount2.Source, source) {
-				continue OUTER
-			}
-		}
-
-		mounts = append(mounts, mount)
-	}
-
-	for _, mount := range mounts {
+	for _, mount := range md.Mounts() {
 
 		mountWeight := 1
 		if md.isMainProject {
@@ -540,7 +507,7 @@ OUTER:
 			From: mount.Target,
 			To:   absPathify(mount.Source),
 			Meta: hugofs.FileMeta{
-				"watch":       md.watch,
+				"watch":       md.Watch(),
 				"mountWeight": mountWeight,
 			},
 		}
@@ -703,9 +670,8 @@ func (c *filesystemsCollector) reverseFis(fis []hugofs.FileMetaInfo) {
 }
 
 type mountsDescriptor struct {
-	mounts        []modules.Mount
+	modules.Module
 	dir           string
-	watch         bool // whether this is a candidate for watching in server mode.
 	isMainProject bool
 }
 
diff --git a/modules/client.go b/modules/client.go
index 12c9ecffc..ae1a6a2b2 100644
--- a/modules/client.go
+++ b/modules/client.go
@@ -66,7 +66,6 @@ const (
 // level imports to start out.
 func NewClient(cfg ClientConfig) *Client {
 	fs := cfg.Fs
-
 	n := filepath.Join(cfg.WorkingDir, goModFilename)
 	goModEnabled, _ := afero.Exists(fs, n)
 	var goModFilename string
@@ -97,9 +96,7 @@ func NewClient(cfg ClientConfig) *Client {
 
 	return &Client{
 		fs:                fs,
-		ignoreVendor:      cfg.IgnoreVendor,
-		workingDir:        cfg.WorkingDir,
-		themesDir:         cfg.ThemesDir,
+		ccfg:              cfg,
 		logger:            logger,
 		moduleConfig:      mcfg,
 		environ:           env,
@@ -111,14 +108,7 @@ type Client struct {
 	fs     afero.Fs
 	logger *loggers.Logger
 
-	// Ignore any _vendor directory.
-	ignoreVendor bool
-
-	// Absolute path to the project dir.
-	workingDir string
-
-	// Absolute path to the project's themes dir.
-	themesDir string
+	ccfg ClientConfig
 
 	// The top level module config
 	moduleConfig Config
@@ -194,7 +184,7 @@ func (c *Client) Tidy() error {
 // meaning that if the top-level module is vendored, that will be the full
 // set of dependencies.
 func (c *Client) Vendor() error {
-	vendorDir := filepath.Join(c.workingDir, vendord)
+	vendorDir := filepath.Join(c.ccfg.WorkingDir, vendord)
 	if err := c.rmVendorDir(vendorDir); err != nil {
 		return err
 	}
@@ -284,7 +274,7 @@ func (c *Client) Init(path string) error {
 		return errors.Wrap(err, "failed to init modules")
 	}
 
-	c.GoModulesFilename = filepath.Join(c.workingDir, goModFilename)
+	c.GoModulesFilename = filepath.Join(c.ccfg.WorkingDir, goModFilename)
 
 	return nil
 }
@@ -335,7 +325,7 @@ func (c *Client) rewriteGoMod(name string, isGoMod map[string]bool) error {
 		return err
 	}
 	if data != nil {
-		if err := afero.WriteFile(c.fs, filepath.Join(c.workingDir, name), data, 0666); err != nil {
+		if err := afero.WriteFile(c.fs, filepath.Join(c.ccfg.WorkingDir, name), data, 0666); err != nil {
 			return err
 		}
 	}
@@ -352,7 +342,7 @@ func (c *Client) rewriteGoModRewrite(name string, isGoMod map[string]bool) ([]by
 	modlineSplitter := getModlineSplitter(name == goModFilename)
 
 	b := &bytes.Buffer{}
-	f, err := c.fs.Open(filepath.Join(c.workingDir, name))
+	f, err := c.fs.Open(filepath.Join(c.ccfg.WorkingDir, name))
 	if err != nil {
 		if os.IsNotExist(err) {
 			// It's been deleted.
@@ -424,7 +414,7 @@ func (c *Client) runGo(
 	cmd := exec.CommandContext(ctx, "go", args...)
 
 	cmd.Env = c.environ
-	cmd.Dir = c.workingDir
+	cmd.Dir = c.ccfg.WorkingDir
 	cmd.Stdout = stdout
 	cmd.Stderr = io.MultiWriter(stderr, os.Stderr)
 
@@ -482,11 +472,22 @@ func (c *Client) tidy(mods Modules, goModOnly bool) error {
 
 // ClientConfig configures the module Client.
 type ClientConfig struct {
-	Fs           afero.Fs
-	Logger       *loggers.Logger
+	Fs     afero.Fs
+	Logger *loggers.Logger
+
+	// If set, it will be run before we do any duplicate checks for modules
+	// etc.
+	HookBeforeFinalize func(m *ModulesConfig) error
+
+	// Ignore any _vendor directory.
 	IgnoreVendor bool
-	WorkingDir   string
-	ThemesDir    string // Absolute directory path
+
+	// Absolute path to the project dir.
+	WorkingDir string
+
+	// Absolute path to the project's themes dir.
+	ThemesDir string
+
 	CacheDir     string // Module cache
 	ModuleConfig Config
 }
diff --git a/modules/collect.go b/modules/collect.go
index 5ba7f74e2..808353608 100644
--- a/modules/collect.go
+++ b/modules/collect.go
@@ -20,6 +20,8 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/gohugoio/hugo/common/loggers"
+
 	"github.com/spf13/cast"
 
 	"github.com/gohugoio/hugo/common/maps"
@@ -62,8 +64,25 @@ func CreateProjectModule(cfg config.Provider) (Module, error) {
 
 func (h *Client) Collect() (ModulesConfig, error) {
 	mc, coll := h.collect(true)
-	return mc, coll.err
+	if coll.err != nil {
+		return mc, coll.err
+	}
+
+	if err := (&mc).setActiveMods(h.logger); err != nil {
+		return mc, err
+	}
+
+	if h.ccfg.HookBeforeFinalize != nil {
+		if err := h.ccfg.HookBeforeFinalize(&mc); err != nil {
+			return mc, err
+		}
+	}
 
+	if err := (&mc).finalize(h.logger); err != nil {
+		return mc, err
+	}
+
+	return mc, nil
 }
 
 func (h *Client) collect(tidy bool) (ModulesConfig, *collector) {
@@ -83,20 +102,8 @@ func (h *Client) collect(tidy bool) (ModulesConfig, *collector) {
 		}
 	}
 
-	// TODO(bep) consider --ignoreVendor vs removing from go.mod
-	var activeMods Modules
-	for _, mod := range c.modules {
-		if !mod.Config().HugoVersion.IsValid() {
-			h.logger.WARN.Printf(`Module %q is not compatible with this Hugo version; run "hugo mod graph" for more information.`, mod.Path())
-		}
-		if !mod.Disabled() {
-			activeMods = append(activeMods, mod)
-		}
-	}
-
 	return ModulesConfig{
 		AllModules:        c.modules,
-		ActiveModules:     activeMods,
 		GoModulesFilename: c.GoModulesFilename,
 	}, c
 
@@ -113,6 +120,43 @@ type ModulesConfig struct {
 	GoModulesFilename string
 }
 
+func (m *ModulesConfig) setActiveMods(logger *loggers.Logger) error {
+	var activeMods Modules
+	for _, mod := range m.AllModules {
+		if !mod.Config().HugoVersion.IsValid() {
+			logger.WARN.Printf(`Module %q is not compatible with this Hugo version; run "hugo mod graph" for more information.`, mod.Path())
+		}
+		if !mod.Disabled() {
+			activeMods = append(activeMods, mod)
+		}
+	}
+
+	m.ActiveModules = activeMods
+
+	return nil
+}
+
+func (m *ModulesConfig) finalize(logger *loggers.Logger) error {
+	for _, mod := range m.AllModules {
+		m := mod.(*moduleAdapter)
+		m.mounts = filterUnwantedMounts(m.mounts)
+	}
+	return nil
+}
+
+func filterUnwantedMounts(mounts []Mount) []Mount {
+	// Remove duplicates
+	seen := make(map[Mount]bool)
+	tmp := mounts[:0]
+	for _, m := range mounts {
+		if !seen[m] {
+			tmp = append(tmp, m)
+		}
+		seen[m] = true
+	}
+	return tmp
+}
+
 type collected struct {
 	// Pick the first and prevent circular loops.
 	seen map[string]bool
@@ -177,7 +221,7 @@ func (c *collector) add(owner *moduleAdapter, moduleImport Import, disabled bool
 	modulePath := moduleImport.Path
 	var realOwner Module = owner
 
-	if !c.ignoreVendor {
+	if !c.ccfg.IgnoreVendor {
 		if err := c.collectModulesTXT(owner); err != nil {
 			return nil, err
 		}
@@ -223,10 +267,10 @@ func (c *collector) add(owner *moduleAdapter, moduleImport Import, disabled bool
 
 			// Fall back to /themes/<mymodule>
 			if moduleDir == "" {
-				moduleDir = filepath.Join(c.themesDir, modulePath)
+				moduleDir = filepath.Join(c.ccfg.ThemesDir, modulePath)
 
 				if found, _ := afero.Exists(c.fs, moduleDir); !found {
-					c.err = c.wrapModuleNotFound(errors.Errorf(`module %q not found; either add it as a Hugo Module or store it in %q.`, modulePath, c.themesDir))
+					c.err = c.wrapModuleNotFound(errors.Errorf(`module %q not found; either add it as a Hugo Module or store it in %q.`, modulePath, c.ccfg.ThemesDir))
 					return nil, nil
 				}
 			}
@@ -427,7 +471,7 @@ func (c *collector) collect() {
 		return
 	}
 
-	projectMod := createProjectModule(c.gomods.GetMain(), c.workingDir, c.moduleConfig)
+	projectMod := createProjectModule(c.gomods.GetMain(), c.ccfg.WorkingDir, c.moduleConfig)
 
 	if err := c.addAndRecurse(projectMod, false); err != nil {
 		c.err = err
diff --git a/modules/collect_test.go b/modules/collect_test.go
index d76c0b2bb..63410ddb1 100644
--- a/modules/collect_test.go
+++ b/modules/collect_test.go
@@ -36,3 +36,19 @@ func TestPathKey(t *testing.T) {
 	}
 
 }
+
+func TestFilterUnwantedMounts(t *testing.T) {
+
+	mounts := []Mount{
+		Mount{Source: "a", Target: "b", Lang: "en"},
+		Mount{Source: "a", Target: "b", Lang: "en"},
+		Mount{Source: "b", Target: "c", Lang: "en"},
+	}
+
+	filtered := filterUnwantedMounts(mounts)
+
+	assert := require.New(t)
+	assert.Len(filtered, 2)
+	assert.Equal([]Mount{Mount{Source: "a", Target: "b", Lang: "en"}, Mount{Source: "b", Target: "c", Lang: "en"}}, filtered)
+
+}
diff --git a/modules/config.go b/modules/config.go
index 163bc7049..62e6f5e4c 100644
--- a/modules/config.go
+++ b/modules/config.go
@@ -15,7 +15,6 @@ package modules
 
 import (
 	"fmt"
-	"path"
 	"path/filepath"
 	"strings"
 
@@ -174,18 +173,7 @@ func ApplyProjectConfigDefaults(cfg config.Provider, mod Module) error {
 	// Prepend the mounts from configuration.
 	mounts = append(moda.mounts, mounts...)
 
-	// Remove duplicates
-	seen := make(map[string]bool)
-	tmp := mounts[:0]
-	for _, m := range mounts {
-		key := path.Join(m.Lang, m.Source, m.Target)
-		if !seen[key] {
-			tmp = append(tmp, m)
-		}
-		seen[key] = true
-	}
-
-	moda.mounts = tmp
+	moda.mounts = mounts
 
 	return nil
 }