From 4b6c5eba306e6e69f3dd07a6c102bfc8040b38c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Wed, 31 Jul 2019 08:21:17 +0200 Subject: [PATCH] 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. --- hugolib/config.go | 84 +++++++++++++++++++++-------------- hugolib/filesystems/basefs.go | 42 ++---------------- modules/client.go | 43 +++++++++--------- modules/collect.go | 78 +++++++++++++++++++++++++------- modules/collect_test.go | 16 +++++++ modules/config.go | 14 +----- 6 files changed, 154 insertions(+), 123 deletions(-) diff --git a/hugolib/config.go b/hugolib/config.go index b7ac46171..07a8d4100 100644 --- a/hugolib/config.go +++ b/hugolib/config.go @@ -207,20 +207,28 @@ func LoadConfig(d ConfigSourceDescriptor, doWithConfig ...func(cfg config.Provid return v, configFiles, err } - mods, modulesConfigFiles, err := l.collectModules(modulesConfig, v) + // 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 + } + + mods := m.ActiveModules + + // Apply default project mounts. + if err := modules.ApplyProjectConfigDefaults(v, mods[len(mods)-1]); err != nil { + return err + } + + return nil + } + + _, modulesConfigFiles, err := l.collectModules(modulesConfig, v, collectHook) if err != nil { return v, configFiles, err } - if err := loadLanguageSettings(v, nil); err != nil { - return v, configFiles, err - } - - // Apply default project mounts. - if err := modules.ApplyProjectConfigDefaults(v, mods[len(mods)-1]); err != nil { - return v, configFiles, err - } - if len(modulesConfigFiles) > 0 { configFiles = append(configFiles, modulesConfigFiles...) } @@ -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/ 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 }