Skip to content

Commit 0eca3b9

Browse files
gregmagolangregmagolan
authored
perf: optimize fs patches when using Bazel 6 with unresolved symlinks (#985)
Relative symlinks are maintained in the sandbox and in runfiles in this case so we can remove the additional logic to check if following hops outside of the sandbox has a corresponding path within the sandbox
1 parent 8f8cbc4 commit 0eca3b9

File tree

5 files changed

+16
-428
lines changed

5 files changed

+16
-428
lines changed

‎js/private/node-patches/fs.js‎

Lines changed: 3 additions & 60 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎js/private/node-patches/src/fs.ts‎

Lines changed: 3 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -634,23 +634,7 @@ export const patcher = (fs: any = _fs, roots: string[]) => {
634634
}
635635
if (isEscape(loc, next)) {
636636
// this hop takes us out of the guard
637-
return nextHop(next, (next2: string | false) => {
638-
if (!next2) {
639-
// the chain is done
640-
return cb(loc)
641-
}
642-
const maybe = path.resolve(
643-
path.dirname(loc),
644-
path.relative(path.dirname(next), next2)
645-
)
646-
if (!isEscape(loc, maybe)) {
647-
// outside of the guard is a symlink but it is a relative link path
648-
// we can map within the guard so return that
649-
return cb(maybe)
650-
}
651-
// outside of the guard is a symlink that is not mappable inside the guard
652-
return cb(loc)
653-
})
637+
return cb(loc)
654638
}
655639
return cb(next)
656640
})
@@ -666,21 +650,6 @@ export const patcher = (fs: any = _fs, roots: string[]) => {
666650
}
667651
if (isEscape(loc, next)) {
668652
// this hop takes us out of the guard
669-
const next2: string | false = nextHopSync(next)
670-
if (!next2) {
671-
// the chain is done
672-
return loc
673-
}
674-
const maybe = path.resolve(
675-
path.dirname(loc),
676-
path.relative(path.dirname(next), next2)
677-
)
678-
if (!isEscape(loc, maybe)) {
679-
// outside of the guard is a symlink but it is a relative link path
680-
// we can map within the guard so return that
681-
return maybe
682-
}
683-
// outside of the guard is a symlink that is not mappable inside the guard
684653
return loc
685654
}
686655
return next
@@ -735,26 +704,7 @@ export const patcher = (fs: any = _fs, roots: string[]) => {
735704
: isEscape(loc, next)
736705
) {
737706
// this hop takes us out of the guard
738-
return nextHop(next, (next2) => {
739-
if (!next2) {
740-
// the chain is done
741-
return cb(null, loc)
742-
}
743-
const maybe = path.resolve(
744-
path.dirname(loc),
745-
path.relative(path.dirname(next), next2)
746-
)
747-
if (isEscape(loc, maybe)) {
748-
// outside of the guard is a symlink that is not mappable inside the guard;
749-
// call the unguarded realpath which will throw if the link is dangling;
750-
// if it doesn't throw then return the last path within the guard
751-
return origRealpath(start, (err, _) => {
752-
if (err) return cb(err)
753-
return cb(null, loc)
754-
})
755-
}
756-
return oneHop(maybe, cb)
757-
})
707+
return cb(null, loc)
758708
}
759709
oneHop(next, cb)
760710
})
@@ -799,25 +749,7 @@ export const patcher = (fs: any = _fs, roots: string[]) => {
799749
: isEscape(loc, next)
800750
) {
801751
// this hop takes us out of the guard
802-
const next2: string | false = nextHopSync(next)
803-
if (!next2) {
804-
// the chain is done
805-
return loc
806-
}
807-
const maybe = path.resolve(
808-
path.dirname(loc),
809-
path.relative(path.dirname(next), next2)
810-
)
811-
if (isEscape(loc, maybe)) {
812-
// outside of the guard is a symlink that is not mappable inside the guard;
813-
// call the unguarded realpath which will throw if the link is dangling;
814-
// if it doesn't throw then return the last path within the guard
815-
origRealpathSync(start)
816-
return loc
817-
}
818-
next = maybe
819-
// outside of the guard is a symlink but it is a relative link path
820-
// we can map within the guard so lets iterate one more time
752+
return loc
821753
}
822754
}
823755
}

‎js/private/test/node-patches/opendir.js‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -202,18 +202,18 @@ describe('testing opendir', async () => {
202202
path.join(fixturesDir, 'execroot', 'link')
203203
)
204204

205-
// create sandbox
205+
// create sandbox; relative symlinks are maintained (assume Bazel 6 with allow unresolved symlinks)
206206
fs.symlinkSync(
207207
path.join(fixturesDir, 'execroot', 'file'),
208208
path.join(fixturesDir, 'sandbox', 'file')
209209
)
210210
fs.symlinkSync(
211-
path.join(fixturesDir, 'execroot', 'link'),
212-
path.join(fixturesDir, 'sandbox', 'link')
211+
path.join(fixturesDir, 'sandbox', 'file'),
212+
path.join(fixturesDir, 'sandbox', 'link2')
213213
)
214214
fs.symlinkSync(
215-
path.join(fixturesDir, 'execroot', 'link2'),
216-
path.join(fixturesDir, 'sandbox', 'link2')
215+
path.join(fixturesDir, 'sandbox', 'link2'),
216+
path.join(fixturesDir, 'sandbox', 'link')
217217
)
218218

219219
const patchedFs = Object.assign({}, fs)

‎js/private/test/node-patches/readdir.js‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -160,18 +160,18 @@ describe('testing readdir', async () => {
160160
path.join(fixturesDir, 'execroot', 'link')
161161
)
162162

163-
// create sandbox
163+
// create sandbox; relative symlinks are maintained (assume Bazel 6 with allow unresolved symlinks)
164164
fs.symlinkSync(
165165
path.join(fixturesDir, 'execroot', 'file'),
166166
path.join(fixturesDir, 'sandbox', 'file')
167167
)
168168
fs.symlinkSync(
169-
path.join(fixturesDir, 'execroot', 'link'),
170-
path.join(fixturesDir, 'sandbox', 'link')
169+
path.join(fixturesDir, 'sandbox', 'file'),
170+
path.join(fixturesDir, 'sandbox', 'link2')
171171
)
172172
fs.symlinkSync(
173-
path.join(fixturesDir, 'execroot', 'link2'),
174-
path.join(fixturesDir, 'sandbox', 'link2')
173+
path.join(fixturesDir, 'sandbox', 'link2'),
174+
path.join(fixturesDir, 'sandbox', 'link')
175175
)
176176

177177
const patchedFs = Object.assign({}, fs)

0 commit comments

Comments
 (0)