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 #2773: eval dependency symlinks instead of reading symlink target #2774

Merged
merged 3 commits into from Dec 27, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -32,6 +32,10 @@

However, this transformation is impossible if the code contains direct `eval` because direct `eval` "poisons" all containing scopes by preventing anything in those scopes from being renamed. That prevents esbuild from splitting up accesses to `foo` into two separate variables with different names. Previously esbuild still did this transformation but with two variables both named `foo`, which is a syntax error. With this release esbuild will now skip doing this transformation when direct `eval` is present to avoid generating code with a syntax error. This means that the generated code may no longer behave as intended since the behavior depends on the run-time strict mode setting instead of the strict mode setting present in the original source code. To fix this problem, you will need to remove the use of direct `eval`.

* Fix a bundling scenario involving multiple symlinks ([#2773](https://github.com/evanw/esbuild/issues/2773), [#2774](https://github.com/evanw/esbuild/issues/2774))

This release contains a fix for a bundling scenario involving an import path where multiple path segments are symlinks. Previously esbuild was unable to resolve certain import paths in this scenario, but these import paths should now work starting with this release. This fix was contributed by [@onebytegone](https://github.com/onebytegone).

## 0.16.10

* Change the default "legal comment" behavior again ([#2745](https://github.com/evanw/esbuild/issues/2745))
Expand Down
38 changes: 13 additions & 25 deletions internal/fs/fs_real.go
Expand Up @@ -405,33 +405,21 @@ func (fs *realFS) kind(dir string, base string) (symlink string, kind EntryKind)

// Follow symlinks now so the cache contains the translation
if (mode & os.ModeSymlink) != 0 {
symlink = entryPath
linksWalked := 0
for {
linksWalked++
if linksWalked > 255 {
return // Error: too many links
}
link, err := os.Readlink(symlink)
if err != nil {
return // Skip over this entry
}
if !fs.fp.isAbs(link) {
link = fs.fp.join([]string{dir, link})
}
symlink = fs.fp.clean(link)
link, err := fs.fp.evalSymlinks(entryPath)
if err != nil {
return // Skip over this entry
}

// Re-run "lstat" on the symlink target
stat2, err2 := os.Lstat(symlink)
if err2 != nil {
return // Skip over this entry
}
mode = stat2.Mode()
if (mode & os.ModeSymlink) == 0 {
break
}
dir = fs.fp.dir(symlink)
// Re-run "lstat" on the symlink target to see if it's a file or not
stat2, err2 := os.Lstat(link)
if err2 != nil {
return // Skip over this entry
}
mode = stat2.Mode()
if (mode & os.ModeSymlink) != 0 {
return // This should no longer be a symlink, so this is unexpected
}
symlink = link
}

// We consider the entry either a directory or a file
Expand Down
16 changes: 16 additions & 0 deletions scripts/end-to-end-tests.js
Expand Up @@ -441,6 +441,22 @@ if (process.platform !== 'win32') {
export function fn() { return foo(); }
`,
}),

// These tests are for https://github.com/evanw/esbuild/issues/2773
test(['--bundle', 'in.js', '--outfile=node.js'], {
'in.js': `import {foo} from './baz/bar/foo'; if (foo !== 444) throw 'fail'`,
'foo/index.js': `import {qux} from '../qux'; export const foo = 123 + qux`,
'qux/index.js': `export const qux = 321`,
'bar/foo': { symlink: `../foo` },
'baz/bar': { symlink: `../bar` },
}),
test(['--bundle', 'in.js', '--outfile=node.js'], {
'in.js': `import {foo} from './baz/bar/foo'; if (foo !== 444) throw 'fail'`,
'foo/index.js': `import {qux} from '../qux'; export const foo = 123 + qux`,
'qux/index.js': `export const qux = 321`,
'bar/foo': { symlink: `TEST_DIR_ABS_PATH/foo` },
'baz/bar': { symlink: `TEST_DIR_ABS_PATH/bar` },
}),
)
}

Expand Down