Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix NPX issue with TailwindCSS v4 #13226

Merged
merged 1 commit into from
Jan 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 85 additions & 17 deletions common/hexec/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ import (
"strings"
"sync"

"github.com/bep/logg"
"github.com/cli/safeexec"
"github.com/gohugoio/hugo/common/loggers"
"github.com/gohugoio/hugo/common/maps"
"github.com/gohugoio/hugo/config"
"github.com/gohugoio/hugo/config/security"
)
Expand Down Expand Up @@ -86,7 +89,7 @@ var WithEnviron = func(env []string) func(c *commandeer) {
}

// New creates a new Exec using the provided security config.
func New(cfg security.Config, workingDir string) *Exec {
func New(cfg security.Config, workingDir string, log loggers.Logger) *Exec {
var baseEnviron []string
for _, v := range os.Environ() {
k, _ := config.SplitEnvVar(v)
Expand All @@ -96,9 +99,11 @@ func New(cfg security.Config, workingDir string) *Exec {
}

return &Exec{
sc: cfg,
workingDir: workingDir,
baseEnviron: baseEnviron,
sc: cfg,
workingDir: workingDir,
infol: log.InfoCommand("exec"),
baseEnviron: baseEnviron,
newNPXRunnerCache: maps.NewCache[string, func(arg ...any) (Runner, error)](),
}
}

Expand All @@ -124,12 +129,14 @@ func SafeCommand(name string, arg ...string) (*exec.Cmd, error) {
type Exec struct {
sc security.Config
workingDir string
infol logg.LevelLogger

// os.Environ filtered by the Exec.OsEnviron whitelist filter.
baseEnviron []string

npxInit sync.Once
npxAvailable bool
newNPXRunnerCache *maps.Cache[string, func(arg ...any) (Runner, error)]
npxInit sync.Once
npxAvailable bool
}

func (e *Exec) New(name string, arg ...any) (Runner, error) {
Expand All @@ -155,25 +162,86 @@ func (e *Exec) new(name string, fullyQualifiedName string, arg ...any) (Runner,
return cm.command(arg...)
}

type binaryLocation int

func (b binaryLocation) String() string {
switch b {
case binaryLocationNodeModules:
return "node_modules/.bin"
case binaryLocationNpx:
return "npx"
case binaryLocationPath:
return "PATH"
}
return "unknown"
}

const (
binaryLocationNodeModules binaryLocation = iota + 1
binaryLocationNpx
binaryLocationPath
)

// Npx will in order:
// 1. Try fo find the binary in the WORKINGDIR/node_modules/.bin directory.
// 2. If not found, and npx is available, run npx --no-install <name> <args>.
// 3. Fall back to the PATH.
// If name is "tailwindcss", we will try the PATH as the second option.
func (e *Exec) Npx(name string, arg ...any) (Runner, error) {
// npx is slow, so first try the common case.
nodeBinFilename := filepath.Join(e.workingDir, nodeModulesBinPath, name)
_, err := safeexec.LookPath(nodeBinFilename)
if err == nil {
return e.new(name, nodeBinFilename, arg...)
if err := e.sc.CheckAllowedExec(name); err != nil {
return nil, err
}
e.checkNpx()
if e.npxAvailable {
r, err := e.npx(name, arg...)
if err == nil {
return r, nil

newRunner, err := e.newNPXRunnerCache.GetOrCreate(name, func() (func(...any) (Runner, error), error) {
type tryFunc func() func(...any) (Runner, error)
tryFuncs := map[binaryLocation]tryFunc{
binaryLocationNodeModules: func() func(...any) (Runner, error) {
nodeBinFilename := filepath.Join(e.workingDir, nodeModulesBinPath, name)
_, err := safeexec.LookPath(nodeBinFilename)
if err != nil {
return nil
}
return func(arg2 ...any) (Runner, error) {
return e.new(name, nodeBinFilename, arg2...)
}
},
binaryLocationNpx: func() func(...any) (Runner, error) {
e.checkNpx()
if !e.npxAvailable {
return nil
}
return func(arg2 ...any) (Runner, error) {
return e.npx(name, arg2...)
}
},
binaryLocationPath: func() func(...any) (Runner, error) {
if _, err := safeexec.LookPath(name); err != nil {
return nil
}
return func(arg2 ...any) (Runner, error) {
return e.New(name, arg2...)
}
},
}

locations := []binaryLocation{binaryLocationNodeModules, binaryLocationNpx, binaryLocationPath}
if name == "tailwindcss" {
// See https://github.com/gohugoio/hugo/issues/13221#issuecomment-2574801253
locations = []binaryLocation{binaryLocationNodeModules, binaryLocationPath, binaryLocationNpx}
}
for _, loc := range locations {
if f := tryFuncs[loc](); f != nil {
e.infol.Logf("resolve %q using %s", name, loc)
return f, nil
}
}
return nil, &NotFoundError{name: name, method: fmt.Sprintf("in %s", locations[len(locations)-1])}
})
if err != nil {
return nil, err
}
return e.New(name, arg...)

return newRunner(arg...)
}

const (
Expand Down
2 changes: 1 addition & 1 deletion config/allconfig/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ func (l *configLoader) loadModules(configs *Configs, ignoreModuleDoesNotExist bo
ignoreVendor, _ = hglob.GetGlob(hglob.NormalizePath(s))
}

ex := hexec.New(conf.Security, workingDir)
ex := hexec.New(conf.Security, workingDir, l.Logger)

hook := func(m *modules.ModulesConfig) error {
for _, tc := range m.AllModules {
Expand Down
2 changes: 1 addition & 1 deletion deps/deps.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ func (d *Deps) Init() error {
}

if d.ExecHelper == nil {
d.ExecHelper = hexec.New(d.Conf.GetConfigSection("security").(security.Config), d.Conf.WorkingDir())
d.ExecHelper = hexec.New(d.Conf.GetConfigSection("security").(security.Config), d.Conf.WorkingDir(), d.Log)
}

if d.MemCache == nil {
Expand Down
2 changes: 1 addition & 1 deletion hugolib/integrationtest_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -729,7 +729,7 @@ func (s *IntegrationTestBuilder) initBuilder() error {
sc := security.DefaultConfig
sc.Exec.Allow, err = security.NewWhitelist("npm")
s.Assert(err, qt.IsNil)
ex := hexec.New(sc, s.Cfg.WorkingDir)
ex := hexec.New(sc, s.Cfg.WorkingDir, loggers.NewDefault())
command, err := ex.New("npm", "install")
s.Assert(err, qt.IsNil)
s.Assert(command.Run(), qt.IsNil)
Expand Down
2 changes: 1 addition & 1 deletion hugolib/testhelpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -838,7 +838,7 @@ func (s *sitesBuilder) NpmInstall() hexec.Runner {
var err error
sc.Exec.Allow, err = security.NewWhitelist("npm")
s.Assert(err, qt.IsNil)
ex := hexec.New(sc, s.workingDir)
ex := hexec.New(sc, s.workingDir, loggers.NewDefault())
command, err := ex.New("npm", "install")
s.Assert(err, qt.IsNil)
return command
Expand Down
2 changes: 1 addition & 1 deletion markup/asciidocext/convert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ allow = ['asciidoctor']
converter.ProviderConfig{
Logger: loggers.NewDefault(),
Conf: conf,
Exec: hexec.New(securityConfig, ""),
Exec: hexec.New(securityConfig, "", loggers.NewDefault()),
},
)
c.Assert(err, qt.IsNil)
Expand Down
2 changes: 1 addition & 1 deletion markup/pandoc/convert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func TestConvert(t *testing.T) {
var err error
sc.Exec.Allow, err = security.NewWhitelist("pandoc")
c.Assert(err, qt.IsNil)
p, err := Provider.New(converter.ProviderConfig{Exec: hexec.New(sc, ""), Logger: loggers.NewDefault()})
p, err := Provider.New(converter.ProviderConfig{Exec: hexec.New(sc, "", loggers.NewDefault()), Logger: loggers.NewDefault()})
c.Assert(err, qt.IsNil)
conv, err := p.New(converter.DocumentContext{})
c.Assert(err, qt.IsNil)
Expand Down
2 changes: 1 addition & 1 deletion markup/rst/convert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func TestConvert(t *testing.T) {
p, err := Provider.New(
converter.ProviderConfig{
Logger: loggers.NewDefault(),
Exec: hexec.New(sc, ""),
Exec: hexec.New(sc, "", loggers.NewDefault()),
})
c.Assert(err, qt.IsNil)
conv, err := p.New(converter.DocumentContext{})
Expand Down
3 changes: 2 additions & 1 deletion modules/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"testing"

"github.com/gohugoio/hugo/common/hexec"
"github.com/gohugoio/hugo/common/loggers"
"github.com/gohugoio/hugo/config/security"
"github.com/gohugoio/hugo/hugofs/glob"

Expand Down Expand Up @@ -61,7 +62,7 @@ github.com/gohugoio/hugoTestModules1_darwin/[email protected] github.com/gohugoio/h
WorkingDir: workingDir,
ThemesDir: themesDir,
PublishDir: publishDir,
Exec: hexec.New(security.DefaultConfig, ""),
Exec: hexec.New(security.DefaultConfig, "", loggers.NewDefault()),
}

withConfig(&ccfg)
Expand Down
Loading