commit 4b189d8fd93d3fa326b31d451d5594c917e6c714
parent c2fa0a33209da7cfd9a09f8fc528415578a9edf8
Author: Bjørn Erik Pedersen <bjorn.erik.pedersen@gmail.com>
Date: Sat, 14 May 2022 15:51:04 +0200
postcss: Fix import error handling
Note that we will now fail if `inlineImports` is enabled and we cannot resolve an import.
You can work around this by either:
* Use url imports or imports with media queries.
* Set `skipInlineImportsNotFound=true` in the options
Also get the argument order in the different NewFileError* funcs in line.
Fixes #9895
Diffstat:
20 files changed, 157 insertions(+), 41 deletions(-)
diff --git a/common/herrors/file_error.go b/common/herrors/file_error.go
@@ -142,7 +142,7 @@ func (e *fileError) Unwrap() error {
// NewFileError creates a new FileError that wraps err.
// The value for name should identify the file, the best
// being the full filename to the file on disk.
-func NewFileError(name string, err error) FileError {
+func NewFileError(err error, name string) FileError {
// Filetype is used to determine the Chroma lexer to use.
fileType, pos := extractFileTypePos(err)
pos.Filename = name
@@ -155,7 +155,7 @@ func NewFileError(name string, err error) FileError {
}
// NewFileErrorFromPos will use the filename and line number from pos to create a new FileError, wrapping err.
-func NewFileErrorFromPos(pos text.Position, err error) FileError {
+func NewFileErrorFromPos(err error, pos text.Position) FileError {
// Filetype is used to determine the Chroma lexer to use.
fileType, _ := extractFileTypePos(err)
if fileType == "" {
@@ -165,20 +165,52 @@ func NewFileErrorFromPos(pos text.Position, err error) FileError {
}
+func NewFileErrorFromFileInPos(err error, pos text.Position, fs afero.Fs, linematcher LineMatcherFn) FileError {
+ if err == nil {
+ panic("err is nil")
+ }
+ f, realFilename, err2 := openFile(pos.Filename, fs)
+ if err2 != nil {
+ return NewFileErrorFromPos(err, pos)
+ }
+ pos.Filename = realFilename
+ defer f.Close()
+ return NewFileErrorFromPos(err, pos).UpdateContent(f, linematcher)
+}
+
// NewFileErrorFromFile is a convenience method to create a new FileError from a file.
-func NewFileErrorFromFile(err error, filename, realFilename string, fs afero.Fs, linematcher LineMatcherFn) FileError {
+func NewFileErrorFromFile(err error, filename string, fs afero.Fs, linematcher LineMatcherFn) FileError {
if err == nil {
panic("err is nil")
}
- if linematcher == nil {
- linematcher = SimpleLineMatcher
+ f, realFilename, err2 := openFile(filename, fs)
+ if err2 != nil {
+ return NewFileError(err, realFilename)
}
+ defer f.Close()
+ return NewFileError(err, realFilename).UpdateContent(f, linematcher)
+}
+
+func openFile(filename string, fs afero.Fs) (afero.File, string, error) {
+ realFilename := filename
+
+ // We want the most specific filename possible in the error message.
+ fi, err2 := fs.Stat(filename)
+ if err2 == nil {
+ if s, ok := fi.(interface {
+ Filename() string
+ }); ok {
+ realFilename = s.Filename()
+ }
+
+ }
+
f, err2 := fs.Open(filename)
if err2 != nil {
- return NewFileError(realFilename, err)
+ return nil, realFilename, err2
}
- defer f.Close()
- return NewFileError(realFilename, err).UpdateContent(f, linematcher)
+
+ return f, realFilename, nil
}
// Cause returns the underlying error or itself if it does not implement Unwrap.
diff --git a/common/herrors/file_error_test.go b/common/herrors/file_error_test.go
@@ -30,7 +30,7 @@ func TestNewFileError(t *testing.T) {
c := qt.New(t)
- fe := NewFileError("foo.html", errors.New("bar"))
+ fe := NewFileError(errors.New("bar"), "foo.html")
c.Assert(fe.Error(), qt.Equals, `"foo.html:1:1": bar`)
lines := ""
@@ -70,7 +70,7 @@ func TestNewFileErrorExtractFromMessage(t *testing.T) {
{errors.New(`execute of template failed: template: index.html:2:5: executing "index.html" at <partial "foo.html" .>: error calling partial: "/layouts/partials/foo.html:3:6": execute of template failed: template: partials/foo.html:3:6: executing "partials/foo.html" at <.ThisDoesNotExist>: can't evaluate field ThisDoesNotExist in type *hugolib.pageStat`), 0, 2, 5},
} {
- got := NewFileError("test.txt", test.in)
+ got := NewFileError(test.in, "test.txt")
errMsg := qt.Commentf("[%d][%T]", i, got)
diff --git a/config/configLoader.go b/config/configLoader.go
@@ -59,7 +59,7 @@ func FromConfigString(config, configType string) (Provider, error) {
func FromFile(fs afero.Fs, filename string) (Provider, error) {
m, err := loadConfigFromFile(fs, filename)
if err != nil {
- return nil, herrors.NewFileErrorFromFile(err, filename, filename, fs, nil)
+ return nil, herrors.NewFileErrorFromFile(err, filename, fs, nil)
}
return NewFrom(m), nil
}
diff --git a/docs/content/en/hugo-pipes/postcss.md b/docs/content/en/hugo-pipes/postcss.md
@@ -44,6 +44,10 @@ URL imports (e.g. `@import url('https://fonts.googleapis.com/css?family=Open+San
Note that this import routine does not care about the CSS spec, so you can have @import anywhere in the file.
Hugo will look for imports relative to the module mount and will respect theme overrides.
+skipInlineImportsNotFound [bool] {{< new-in "0.99.0" >}}
+
+Before Hugo 0.99.0 when `inlineImports` was enabled and we failed to resolve an import, we logged it as a warning. We now fail the build. If you have regular CSS imports in your CSS that you want to preserve, you can either use imports with URL or media queries (Hugo does not try to resolve those) or set `skipInlineImportsNotFound` to true.
+
_If no configuration file is used:_
use [string]
diff --git a/hugofs/fileinfo.go b/hugofs/fileinfo.go
@@ -139,6 +139,17 @@ type fileInfoMeta struct {
m *FileMeta
}
+type filenameProvider interface {
+ Filename() string
+}
+
+var _ filenameProvider = (*fileInfoMeta)(nil)
+
+// Filename returns the full filename.
+func (fi *fileInfoMeta) Filename() string {
+ return fi.m.Filename
+}
+
// Name returns the file's name. Note that we follow symlinks,
// if supported by the file system, and the Name given here will be the
// name of the symlink, which is what Hugo needs in all situations.
diff --git a/hugolib/config.go b/hugolib/config.go
@@ -511,5 +511,5 @@ func (configLoader) loadSiteConfig(cfg config.Provider) (scfg SiteConfig, err er
}
func (l configLoader) wrapFileError(err error, filename string) error {
- return herrors.NewFileErrorFromFile(err, filename, filename, l.Fs, nil)
+ return herrors.NewFileErrorFromFile(err, filename, l.Fs, nil)
}
diff --git a/hugolib/hugo_sites.go b/hugolib/hugo_sites.go
@@ -968,7 +968,7 @@ func (h *HugoSites) errWithFileContext(err error, f source.File) error {
}
realFilename := fim.Meta().Filename
- return herrors.NewFileErrorFromFile(err, realFilename, realFilename, h.SourceSpec.Fs.Source, nil)
+ return herrors.NewFileErrorFromFile(err, realFilename, h.SourceSpec.Fs.Source, nil)
}
diff --git a/hugolib/page.go b/hugolib/page.go
@@ -588,7 +588,7 @@ func (p *pageState) wrapError(err error) error {
}
}
- return herrors.NewFileErrorFromFile(err, filename, filename, p.s.SourceSpec.Fs.Source, herrors.NopLineMatcher)
+ return herrors.NewFileErrorFromFile(err, filename, p.s.SourceSpec.Fs.Source, herrors.NopLineMatcher)
}
@@ -788,7 +788,7 @@ func (p *pageState) outputFormat() (f output.Format) {
func (p *pageState) parseError(err error, input []byte, offset int) error {
pos := p.posFromInput(input, offset)
- return herrors.NewFileError(p.File().Filename(), err).UpdatePosition(pos)
+ return herrors.NewFileError(err, p.File().Filename()).UpdatePosition(pos)
}
func (p *pageState) pathOrTitle() string {
diff --git a/hugolib/shortcode.go b/hugolib/shortcode.go
@@ -298,7 +298,7 @@ func renderShortcode(
var err error
tmpl, err = s.TextTmpl().Parse(templName, templStr)
if err != nil {
- fe := herrors.NewFileError(p.File().Filename(), err)
+ fe := herrors.NewFileError(err, p.File().Filename())
pos := fe.Position()
pos.LineNumber += p.posOffset(sc.pos).LineNumber
fe = fe.UpdatePosition(pos)
@@ -391,7 +391,7 @@ func renderShortcode(
result, err := renderShortcodeWithPage(s.Tmpl(), tmpl, data)
if err != nil && sc.isInline {
- fe := herrors.NewFileError(p.File().Filename(), err)
+ fe := herrors.NewFileError(err, p.File().Filename())
pos := fe.Position()
pos.LineNumber += p.posOffset(sc.pos).LineNumber
fe = fe.UpdatePosition(pos)
diff --git a/langs/i18n/translationProvider.go b/langs/i18n/translationProvider.go
@@ -138,6 +138,6 @@ func errWithFileContext(inerr error, r source.File) error {
}
defer f.Close()
- return herrors.NewFileError(realFilename, inerr).UpdateContent(f, nil)
+ return herrors.NewFileError(inerr, realFilename).UpdateContent(f, nil)
}
diff --git a/markup/goldmark/codeblocks/render.go b/markup/goldmark/codeblocks/render.go
@@ -130,7 +130,7 @@ func (r *htmlRenderer) renderCodeBlock(w util.BufWriter, src []byte, node ast.No
ctx.AddIdentity(cr)
if err != nil {
- return ast.WalkContinue, herrors.NewFileErrorFromPos(cbctx.createPos(), err)
+ return ast.WalkContinue, herrors.NewFileErrorFromPos(err, cbctx.createPos())
}
return ast.WalkContinue, nil
diff --git a/parser/metadecoders/decoder.go b/parser/metadecoders/decoder.go
@@ -260,7 +260,7 @@ func (d Decoder) unmarshalORG(data []byte, v any) error {
}
func toFileError(f Format, data []byte, err error) error {
- return herrors.NewFileError(fmt.Sprintf("_stream.%s", f), err).UpdateContent(bytes.NewReader(data), nil)
+ return herrors.NewFileError(err, fmt.Sprintf("_stream.%s", f)).UpdateContent(bytes.NewReader(data), nil)
}
// stringifyMapKeys recurses into in and changes all instances of
diff --git a/resources/resource_transformers/js/build.go b/resources/resource_transformers/js/build.go
@@ -165,7 +165,7 @@ func (t *buildTransformation) Transform(ctx *resources.ResourceTransformationCtx
if err == nil {
fe := herrors.
- NewFileError(path, errors.New(errorMessage)).
+ NewFileError(errors.New(errorMessage), path).
UpdatePosition(text.Position{Offset: -1, LineNumber: loc.Line, ColumnNumber: loc.Column}).
UpdateContent(f, nil)
diff --git a/resources/resource_transformers/postcss/integration_test.go b/resources/resource_transformers/postcss/integration_test.go
@@ -48,7 +48,7 @@ class-in-b {
@tailwind base;
@tailwind components;
@tailwind utilities;
-@import "components/all.css";
+ @import "components/all.css";
h1 {
@apply text-2xl font-bold;
}
@@ -140,3 +140,49 @@ func TestTransformPostCSSError(t *testing.T) {
c.Assert(err.Error(), qt.Contains, "a.css:4:2")
}
+
+// #9895
+func TestTransformPostCSSImportError(t *testing.T) {
+ if !htesting.IsCI() {
+ t.Skip("Skip long running test when running locally")
+ }
+
+ c := qt.New(t)
+
+ s, err := hugolib.NewIntegrationTestBuilder(
+ hugolib.IntegrationTestConfig{
+ T: c,
+ NeedsOsFS: true,
+ NeedsNpmInstall: true,
+ LogLevel: jww.LevelInfo,
+ TxtarString: strings.ReplaceAll(postCSSIntegrationTestFiles, `@import "components/all.css";`, `@import "components/doesnotexist.css";`),
+ }).BuildE()
+
+ s.AssertIsFileError(err)
+ c.Assert(err.Error(), qt.Contains, "styles.css:4:3")
+ c.Assert(err.Error(), qt.Contains, filepath.FromSlash(`failed to resolve CSS @import "css/components/doesnotexist.css"`))
+
+}
+
+func TestTransformPostCSSImporSkipInlineImportsNotFound(t *testing.T) {
+ if !htesting.IsCI() {
+ t.Skip("Skip long running test when running locally")
+ }
+
+ c := qt.New(t)
+
+ files := strings.ReplaceAll(postCSSIntegrationTestFiles, `@import "components/all.css";`, `@import "components/doesnotexist.css";`)
+ files = strings.ReplaceAll(files, `{{ $options := dict "inlineImports" true }}`, `{{ $options := dict "inlineImports" true "skipInlineImportsNotFound" true }}`)
+
+ s := hugolib.NewIntegrationTestBuilder(
+ hugolib.IntegrationTestConfig{
+ T: c,
+ NeedsOsFS: true,
+ NeedsNpmInstall: true,
+ LogLevel: jww.LevelInfo,
+ TxtarString: files,
+ }).Build()
+
+ s.AssertFileContent("public/css/styles.css", filepath.FromSlash(`@import "components/doesnotexist.css";`))
+
+}
diff --git a/resources/resource_transformers/postcss/postcss.go b/resources/resource_transformers/postcss/postcss.go
@@ -28,6 +28,7 @@ import (
"github.com/gohugoio/hugo/common/collections"
"github.com/gohugoio/hugo/common/hexec"
+ "github.com/gohugoio/hugo/common/text"
"github.com/gohugoio/hugo/hugofs"
"github.com/gohugoio/hugo/common/hugo"
@@ -49,9 +50,10 @@ import (
const importIdentifier = "@import"
-var cssSyntaxErrorRe = regexp.MustCompile(`> (\d+) \|`)
-
-var shouldImportRe = regexp.MustCompile(`^@import ["'].*["'];?\s*(/\*.*\*/)?$`)
+var (
+ cssSyntaxErrorRe = regexp.MustCompile(`> (\d+) \|`)
+ shouldImportRe = regexp.MustCompile(`^@import ["'].*["'];?\s*(/\*.*\*/)?$`)
+)
// New creates a new Client with the given specification.
func New(rs *resources.Spec) *Client {
@@ -100,6 +102,12 @@ type Options struct {
// so you can have @import anywhere in the file.
InlineImports bool
+ // When InlineImports is enabled, we fail the build if an import cannot be resolved.
+ // You can enable this to allow the build to continue and leave the import statement in place.
+ // Note that the inline importer does not process url location or imports with media queries,
+ // so those will be left as-is even without enabling this option.
+ SkipInlineImportsNotFound bool
+
// Options for when not using a config file
Use string // List of postcss plugins to use
Parser string // Custom postcss parser
@@ -204,6 +212,7 @@ func (t *postcssTransformation) Transform(ctx *resources.ResourceTransformationC
imp := newImportResolver(
ctx.From,
ctx.InPath,
+ t.options,
t.rs.Assets.Fs, t.rs.Logger,
)
@@ -239,6 +248,7 @@ type fileOffset struct {
type importResolver struct {
r io.Reader
inPath string
+ opts Options
contentSeen map[string]bool
linemap map[int]fileOffset
@@ -246,12 +256,13 @@ type importResolver struct {
logger loggers.Logger
}
-func newImportResolver(r io.Reader, inPath string, fs afero.Fs, logger loggers.Logger) *importResolver {
+func newImportResolver(r io.Reader, inPath string, opts Options, fs afero.Fs, logger loggers.Logger) *importResolver {
return &importResolver{
r: r,
inPath: inPath,
fs: fs, logger: logger,
linemap: make(map[int]fileOffset), contentSeen: make(map[string]bool),
+ opts: opts,
}
}
@@ -282,21 +293,32 @@ func (imp *importResolver) importRecursive(
i := 0
for offset, line := range lines {
i++
- line = strings.TrimSpace(line)
+ lineTrimmed := strings.TrimSpace(line)
+ column := strings.Index(line, lineTrimmed)
+ line = lineTrimmed
if !imp.shouldImport(line) {
trackLine(i, offset, line)
} else {
- i--
path := strings.Trim(strings.TrimPrefix(line, importIdentifier), " \"';")
filename := filepath.Join(basePath, path)
importContent, hash := imp.contentHash(filename)
+
if importContent == nil {
- trackLine(i, offset, "ERROR")
- imp.logger.Warnf("postcss: Failed to resolve CSS @import in %q for path %q", inPath, filename)
- continue
+ if imp.opts.SkipInlineImportsNotFound {
+ trackLine(i, offset, line)
+ continue
+ }
+ pos := text.Position{
+ Filename: inPath,
+ LineNumber: offset + 1,
+ ColumnNumber: column + 1,
+ }
+ return 0, "", herrors.NewFileErrorFromFileInPos(fmt.Errorf("failed to resolve CSS @import %q", filename), pos, imp.fs, nil)
}
+ i--
+
if imp.contentSeen[hash] {
i++
// Just replace the line with an empty string.
@@ -399,7 +421,7 @@ func (imp *importResolver) toFileError(output string) error {
}
defer f.Close()
- ferr := herrors.NewFileError(realFilename, inErr)
+ ferr := herrors.NewFileError(inErr, realFilename)
pos := ferr.Position()
pos.LineNumber = file.Offset + 1
return ferr.UpdatePosition(pos).UpdateContent(f, nil)
diff --git a/resources/resource_transformers/postcss/postcss_test.go b/resources/resource_transformers/postcss/postcss_test.go
@@ -60,6 +60,7 @@ func TestShouldImport(t *testing.T) {
{input: `@import 'navigation.css';`, expect: true},
{input: `@import url("navigation.css");`, expect: false},
{input: `@import url('https://fonts.googleapis.com/css?family=Open+Sans:400,400i,800,800i&display=swap');`, expect: false},
+ {input: `@import "printstyle.css" print;`, expect: false},
} {
c.Assert(imp.shouldImport(test.input), qt.Equals, test.expect)
}
@@ -88,12 +89,12 @@ A_STYLE2
@import "b.css";
LOCAL_STYLE
@import "c.css";
-@import "e.css";
-@import "missing.css";`)
+@import "e.css";`)
imp := newImportResolver(
mainStyles,
"styles.css",
+ Options{},
fs, loggers.NewErrorLogger(),
)
@@ -108,8 +109,7 @@ C_STYLE
A_STYLE1
A_STYLE2
LOCAL_STYLE
-E_STYLE
-@import "missing.css";`)
+E_STYLE`)
dline := imp.linemap[3]
c.Assert(dline, qt.DeepEquals, fileOffset{
@@ -151,6 +151,7 @@ LOCAL_STYLE
imp := newImportResolver(
strings.NewReader(mainStyles),
"styles.css",
+ Options{},
fs, logger,
)
diff --git a/resources/resource_transformers/tocss/dartsass/transform.go b/resources/resource_transformers/tocss/dartsass/transform.go
@@ -125,7 +125,7 @@ func (t *transform) Transform(ctx *resources.ResourceTransformationCtx) error {
return -1
}
- return herrors.NewFileErrorFromFile(sassErr, filename, filename, hugofs.Os, offsetMatcher)
+ return herrors.NewFileErrorFromFile(sassErr, filename, hugofs.Os, offsetMatcher)
}
return err
diff --git a/tpl/tplimpl/template.go b/tpl/tplimpl/template.go
@@ -554,7 +554,7 @@ func (t *templateHandler) addFileContext(templ tpl.Template, inerr error) error
}
defer f.Close()
- fe := herrors.NewFileError(info.realFilename, inErr)
+ fe := herrors.NewFileError(inErr, info.realFilename)
fe.UpdateContent(f, lineMatcher)
if !fe.ErrorContext().Position.IsValid() {
diff --git a/tpl/tplimpl/template_errors.go b/tpl/tplimpl/template_errors.go
@@ -53,7 +53,7 @@ func (t templateInfo) resolveType() templateType {
func (info templateInfo) errWithFileContext(what string, err error) error {
err = fmt.Errorf(what+": %w", err)
- fe := herrors.NewFileError(info.realFilename, err)
+ fe := herrors.NewFileError(err, info.realFilename)
f, err := info.fs.Open(info.filename)
if err != nil {
return err
diff --git a/transform/chain.go b/transform/chain.go
@@ -113,9 +113,9 @@ func (c *Chain) Apply(to io.Writer, from io.Reader) error {
filename = tempfile.Name()
defer tempfile.Close()
_, _ = io.Copy(tempfile, fb.from)
- return herrors.NewFileErrorFromFile(err, filename, filename, hugofs.Os, nil)
+ return herrors.NewFileErrorFromFile(err, filename, hugofs.Os, nil)
}
- return herrors.NewFileError(filename, err).UpdateContent(fb.from, nil)
+ return herrors.NewFileError(err, filename).UpdateContent(fb.from, nil)
}
}