[WIP] Bazel build system #11337
[WIP] Bazel build system #11337
Conversation
Using latest JDK (after proper GraalVM integration) broke compilation of Scala projects due to a problem with SecurityManager in `scala_rules`. Workaround it by having custom toolchain. Added example BUILD for mixed Scala/Java project with a small number of dependencies.
Frizi
added
the
CI: Clean build required
| @@ -1251,7 +1251,7 @@ export default class RemoteBackend extends Backend { | |||
| async logEvent(message: string, projectId?: string | null, metadata?: object | null) { | |||
| // Prevent events from being logged in dev mode, since we are often using production environment | |||
| // and are polluting real logs. | |||
| if (detect.IS_DEV_MODE && process.env.ENSO_CLOUD_ENVIRONMENT === 'production') { | |||
There was a problem hiding this comment.
While the check is questionable, wouldn’t it change the behavior for the dashboard team?
There was a problem hiding this comment.
I added this check myself back when we were unintentionally sending all development logs to production database. The environment check wasn't really the best approach though, but my intention was to indeed do the least amount of change. Now I'm pretty confident that we flat out don't want to send any dev logs to production logger, so I changed that once I noticed this line when applying the config changes. If we want logging during testing, we should configure a separate logger endpoint for that.
| if (payload != null) { | ||
| const contentType = options.mimetype ?? 'application/json' | ||
| headers.set('Content-Type', contentType) | ||
| } | ||
|
|
||
| // `Blob` request payloads are NOT VISIBLE in Playwright due to a Chromium bug. | ||
| // https://github.com/microsoft/playwright/issues/6479#issuecomment-1574627457 | ||
| if (process.env.IS_IN_PLAYWRIGHT_TEST === 'true' && payload instanceof Blob) { |
There was a problem hiding this comment.
Not sure about removing of this one.
There was a problem hiding this comment.
Right, I have in notes to verify on CI if this trips anything up. I needed to remove it to make the dist package independent of envs. After carefully reading the linked issue, it seems to manifest itself only when request payload is being cloned. We don't use that functionality anywhere in our code, so it's worth to verify if it's actually necessary. If it turns out to be necessary, we should somehow fix it in the playwright test setup code instead, so the app code doesn't need to be specialized for tests.
| name = "tsc", | ||
| srcs = glob( | ||
| ["src/**/*.ts"], | ||
| IGNORED_GENERATED_FILES, |
There was a problem hiding this comment.
I wonder why we use IGNORED files as an input. Perhaps a better name is needed?
There was a problem hiding this comment.
It's not an input. Second glob parameter is a list of files to ignore during the search.
| @@ -447,7 +447,7 @@ function makeAbstractType( | |||
| schema, | |||
| ), | |||
| tsf.createEnumDeclaration( | |||
| [modifiers.export, modifiers.const], | |||
| [modifiers.export], | |||
There was a problem hiding this comment.
Hmm, didn’t we fix it with some flag in the config?
There was a problem hiding this comment.
It ended up crashing during tests. I wasn't able to fix it with different config flags. I actually looked at the usage patterns of this, and it's not really a big deal that this is not inlined. All the important things (byte offsets) are still preserved as number literals, while the type enum is only really used for rare type comparisons. So it doesn't have a good reason to be a const enum to begin with.
| @@ -64,7 +64,7 @@ impl From<Browser> for wasm_pack::TestFlags { | |||
|
|
|||
| /// Lists members of given Cargo.toml workspace. | |||
| pub fn get_all_crates(repo_root: impl AsRef<Path>) -> Result<Vec<PathBuf>> { | |||
| let pattern = repo_root.as_ref().join("**/Cargo.toml"); | |||
| let pattern = repo_root.as_ref().join("[al][pi][pb]/**/Cargo.toml"); | |||
There was a problem hiding this comment.
yeah i don't know about this one... it'd technically match things like lip and apb, although to be fair it's unlikely for those folders to ever exist... but still if something like {app,lib} works i would definitely prefer that
There was a problem hiding this comment.
I would too, but seems like it's not a supported syntax. This code will soon be gone anyway, so I though this is the easiest way to go about it in the mean time.
Pull Request Description
Created a bazel-based build system, implemented a whole GUI build target within it.
Currently this PR open in order to develop and test the github actions workflow.