Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

chore(bazel): enable rules_esbuild sandbox with object-inspect workaround#61969

Merged
Strum355 merged 1 commit into
mainfrom
nsc/bump-rules-esbuild
Jun 5, 2024
Merged

chore(bazel): enable rules_esbuild sandbox with object-inspect workaround#61969
Strum355 merged 1 commit into
mainfrom
nsc/bump-rules-esbuild

Conversation

@Strum355

@Strum355 Strum355 commented Apr 17, 2024

Copy link
Copy Markdown
Contributor

Sandbox escapes be-gone

Test plan

Tested in CI and locally with bazel build //client/... as well as a lot of blood, sweat n tears tearing through failed sandboxes

Changelog

@cla-bot cla-bot Bot added the cla-signed label Apr 17, 2024
@Strum355 Strum355 force-pushed the nsc/legacy-pgsql-image-bazel branch 4 times, most recently from fd509ef to 652d0e6 Compare April 23, 2024 20:01
@Strum355 Strum355 closed this Apr 25, 2024
@Strum355 Strum355 reopened this Apr 30, 2024
@Strum355 Strum355 changed the base branch from nsc/legacy-pgsql-image-bazel to main April 30, 2024 17:31
@Strum355 Strum355 force-pushed the nsc/bump-rules-esbuild branch 4 times, most recently from b7fa259 to cf2a1c8 Compare May 3, 2024 13:35
@Strum355 Strum355 force-pushed the nsc/bump-rules-esbuild branch 2 times, most recently from 5150396 to 5db1845 Compare May 16, 2024 20:01
@Strum355 Strum355 force-pushed the nsc/bump-rules-esbuild branch 2 times, most recently from 8c070f6 to 6f720f6 Compare June 4, 2024 14:56
@Strum355 Strum355 changed the title bazel: bump rules_esbuild to 0.19.0 chore(bazel): enable rules_esbuild sandbox with object-inspect workaround Jun 4, 2024
@Strum355 Strum355 force-pushed the nsc/bump-rules-esbuild branch 2 times, most recently from 8342914 to 7931d31 Compare June 4, 2024 16:32
@Strum355 Strum355 force-pushed the nsc/bump-rules-esbuild branch from 7931d31 to 1300a5d Compare June 4, 2024 17:05
name = "fetch-mock",
srcs = ["src/fetch.js"],
declarations = ["src/fetch.js"],
types = ["src/fetch.js"],

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread package.json
Comment on lines +454 to +458
"mz": {
"dependencies": {
"graceful-fs": "*"
}
},

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Point the finger of shame at this shameful package

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://www.npmjs.com/package/mz "modernize" ... grabs pitchfork TO ARMS

Comment thread package.json
Comment on lines +459 to +468
"follow-redirects": {
"dependencies": {
"debug": "*"
}
},
"debug": {
"dependencies": {
"supports-color": "*"
}
},

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are less shameful (the deps are marked as optional in peerDependenciesMeta and idk if theres a better way to surface them without adding them directly to our root dependencies list)

@Strum355 Strum355 marked this pull request as ready for review June 4, 2024 17:13
@Strum355 Strum355 requested a review from a team June 4, 2024 17:14

- return await resolveInExecroot(build, importPath, otherOptions)
+ const res = await resolveInExecroot(build, importPath, otherOptions)
+ // Needed due to an issue with esbuild when it comes to plguins + `"browser": false` in package.json

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From now on the Cody PLG team shall be referred to as "PLGuins"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

darn, I completely skimmed this 😆 didnt notice the typo

@burmudar burmudar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the uninitiated - how did you find the offending packages that did not declare their deps properly?

@Strum355

Strum355 commented Jun 5, 2024

Copy link
Copy Markdown
Contributor Author

For the uninitiated - how did you find the offending packages that did not declare their deps properly?

See the build errors here: https://buildkite.com/sourcegraph/sourcegraph/builds/276958. For better behaved packages, its a case of it being an optional peer dependency (aka one that would be dragged in by another package afaik, so not brought in my default)

@Strum355 Strum355 merged commit 4a93f29 into main Jun 5, 2024
@Strum355 Strum355 deleted the nsc/bump-rules-esbuild branch June 5, 2024 14:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants