-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Closed
Labels
component: build processissue: feature requestDescribes a new feature and why it should be addedDescribes a new feature and why it should be addedtype: cleanup
Description
This is a tracking bug for work to update our build system so that we can remove all checked-in build products from the google/blockly repository.
Background
The blockly repository contains not only source files but also build products, including the *_compressed.js (and .map.js) files. This is very convenient for being able to quickly fire up (e.g.) the Blockly Playground after doing a git clone, but creates some problems:
- Previously, every build would potentially modify these files, creating a lot of confusion about what should and shouldn't be included in any commit.
- This was fixed (for Closure Compiler output) by building to
build/and adding thenpm run checkincommand.
- This was fixed (for Closure Compiler output) by building to
- Now, we are in a situation where it is very easy to inadvertently be using stale checked-in build output rather than the freshly-generated copies.
- This has occasionally caused quite a lot of confusion, particularly around running the playground in compiled mode (which uses the checked-in copies at least until chore(tests): Use freshly-build files in compressed mode. #6218 is finished.
- Additionally, there has been some desire to check in additional build product, such as the wrapped language files.
It is probably preferable not to have any build output checked into the repository, but to generate it when required.
Outline Proposal
- Remove all generated files from the repository.
- Have
build/contain only ~temporary build products that are not part of the final release—e.g.:- The
.jsfiles output fromtsc. - The un-wrapped language files currently checked in at
msg/js/.
- The
- Have
dist/contain (the only copy of) each "final" build product (e.g.blockly_compressed.js). The contents ofdist/would be the canonical place to load blockly from:- Test/playground code in the Blockly repository would load Blockly from this directory directly.
- The contents of this directory will (continue to) be packaged as an NPM package.
- The contents of this directory could additionally be attached to the GitHub release, to provide an easy way for non-NPM users to download a ready-to-use Blockly as a
.zipfile.
- The only exception will be test/playgrounds running in uncompiled mode (for edit/compile/test velocity) that would continue to to use files in
build/where appropriate. npm run buildwould write some things that it currently writes tobuild/directly todist/.npm run packagewould add whatever additional files (e.g.package.json) that are needed for release but not for testing.npm run prepare, which happens automatically when doingnpm install, would be configured to donpm run build(or a suitable subset thereof) to ensure that previously checked-in build products have been built, allowing a "clone and go" approach to continue to work (just now "clone, install and go").- Additionally, to reduce the potential for human error when building, packaging and publishing Blockly, each step of the build / package / publish process would be configured to run all the previous steps. So for example in the following list each target includes all the previous targets (this is not a comprehensive list):
build:js(runstsc)build:deps(runsclosure-make-deps)build:compiled(runsgoogle-closure-compiler; this target will thus become synonymous withbuildand be removed.)package(copies files intodist/)publish(publishes contents ofdist/)
This is already the case forbuild:js/build:deps/buildtargets, so this is just extending the same principle to the full set of primary targets.
To Do list
- Modify gulp targets so that each runs all the ones it depends on, to avoid any inconsistency in build/packaged files.
- Consider whether to rename gulp scripts to reflect the new arrangements.
- For example, originally the
build:xyztargets were all separate non-overlapping parts of thebuildtarget, but that will no longer be the case, so a different naming system might be advisable; perhaps e.g. justtsc/deps/bundle.
- For example, originally the
- Consider whether to rename gulp scripts to reflect the new arrangements.
- Modify
packagetarget to run thecleanBuildDirandcleanReleaseDirtargets (i.e. the same asnpm run clean) before any of the other build steps to ensure that no extraneous files are inadvertently included indist/and thus the package. - Modify
publishscript to runnpm cibefore invoking the gulp script to ensure that dependencies are at the specified version. - Modify
tests/run_all_tests.shto just runnpm run packageonce at the start (for efficiency, sincepackagewill now invokebuild). - Modify gulp targets to write to
dist/instead ofbuild/where appropriate.- Remove obsolete copying code from
packagegup targets.
- Remove obsolete copying code from
- Modify tests to load
dist/msg/en.jsinstead ofmsg/messages.js. - Modify
tests/bootstrap.jsto load fromdist/(chore(tests): Use freshly-build files in compressed mode. #6218). - Remove build products from the repository:
-
*_compressed.jsand.js.map -
msg/js/* - (any others?)
-
- Update the procedures for pushing our GitHub Pages to include checking in the relevant build products to the
gh_pagesbranch so that our GitHub Pages will continue to work.
Taikono-HimazinTaikono-Himazin
Metadata
Metadata
Assignees
Labels
component: build processissue: feature requestDescribes a new feature and why it should be addedDescribes a new feature and why it should be addedtype: cleanup