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

chore(web): web build streamlining #9743

Merged
merged 1 commit into from
Oct 12, 2023
Merged

Conversation

jahorton
Copy link
Contributor

@jahorton jahorton commented Oct 11, 2023

Fixes #9670. Addresses #9670. While it does fix It fixes the biggest concern from the issue, though there are a few other nits that this doesn't outright address.

(As per comments received on this PR, I've updated this to "fix" the issue.)

  1. The android-harness build.sh is still called directly; it's not on a child-project path related to the build product that triggers it. That said... it's only called one time.

  2. The build scripts for the tools do still get called twice, although they at least don't "lose the environment" now.

[web/src/tools] child build, parameters: <build --builder-child --deps>
[web/src/tools/testing/bulk_rendering] child build, parameters: <build --builder-child --deps>
[web/src/tools/testing/bulk_rendering] ## build starting...
[web/src/tools/testing/bulk_rendering] ## build completed successfully
[web/src/tools/testing/recorder] child build, parameters: <build --builder-child --deps>
[web/src/tools/testing/recorder] ## build starting...
[common/web/recorder] dependency build, started by web/src/tools/testing/recorder
[common/web/recorder] build.sh parameters: <configure build --deps --builder-dep-parent web/src/tools/testing/recorder>
[common/web/recorder] skipping configure, up-to-date
[common/web/recorder] skipping build, up-to-date
[web/src/tools/testing/recorder] ## build completed successfully
[web/src/tools/building/sourcemap-root] child build, parameters: <build --builder-child --deps>
[web/src/tools/building/sourcemap-root] ## build starting...
[common/tools/sourcemap-path-remapper] dependency build, started by web/src/tools/building/sourcemap-root
[common/tools/sourcemap-path-remapper] build.sh parameters: <configure build --deps --builder-dep-parent web/src/tools/building/sourcemap-root>
[common/tools/sourcemap-path-remapper] skipping configure, up-to-date
[common/tools/sourcemap-path-remapper] skipping build, up-to-date
[web/src/tools/building/sourcemap-root] ## build completed successfully

Followed immediately by:

[web/src/test/manual] child build, parameters: <build --builder-child --deps>
[web/src/test/manual] ## build starting...
[common/web/sentry-manager] dependency build, started by web/src/test/manual
[common/web/sentry-manager] build.sh parameters: <configure build --deps --builder-dep-parent web/src/test/manual>
[common/web/sentry-manager] skipping configure, up-to-date
[common/web/sentry-manager] skipping build, up-to-date
[web/src/tools] dependency build, started by web/src/test/manual
[web/src/tools] build.sh parameters: <configure build --deps --builder-dep-parent web/src/test/manual>
[web/src/tools/testing/bulk_rendering] child build, parameters: <build --builder-child --deps>
[web/src/tools/testing/bulk_rendering] ## build starting...
[web/src/tools/testing/bulk_rendering] ## build completed successfully
[web/src/tools/testing/recorder] child build, parameters: <build --builder-child --deps>
[web/src/tools/testing/recorder] ## build starting...
[web/src/tools/testing/recorder] ## build completed successfully
[web/src/tools/building/sourcemap-root] child build, parameters: <build --builder-child --deps>
[web/src/tools/building/sourcemap-root] ## build starting...
[web/src/tools/building/sourcemap-root] ## build completed successfully
[web/src/test/manual] ## build completed successfully

Compare the later entries in both sections; they're nigh-identical.

First set of calls: as a child script, from the top-level build.sh.

Second set: as a dependency, from web/src/test/manual, which does not include web/src/tools as a viable child-project folder.

@keymanapp-test-bot skip

@jahorton jahorton added this to the A17S23 milestone Oct 11, 2023
@mcdurdin
Copy link
Member

Compare the later entries in both sections; they're nigh-identical.

First set of calls: as a child script, from the top-level build.sh.

Second set: as a dependency, from web/src/test/manual, which does not include web/src/tools as a viable child-project folder.

I reckon we should consider tweaking builder.inc.sh to handle the scenario of child vs dependency calls resulting in multiple calls.

Copy link
Member

@mcdurdin mcdurdin left a comment

Choose a reason for hiding this comment

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

These changes look good and go a long way in the right direction, enough to go ahead and mark it as closing the issue I think. And we might open a separate issue to mitigate the double-build scenario from dep vs child calls?

@jahorton jahorton merged commit d283165 into master Oct 12, 2023
16 checks passed
@jahorton jahorton deleted the chore/web/build-streamlining branch October 12, 2023 01:10
@keyman-server
Copy link
Collaborator

Changes in this pull request will be available for download in Keyman version 17.0.190-alpha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

chore(web): build scripts are calling child scripts directly, meaning we lose deps and build state
3 participants