Skip to content
This repository has been archived by the owner on Dec 28, 2021. It is now read-only.

Fix build failing when executing run watch on a clean build. #1395

Merged
merged 4 commits into from
Mar 31, 2021

Conversation

MichaelMauderer
Copy link
Contributor

@MichaelMauderer MichaelMauderer commented Mar 28, 2021

Pull Request Description

Fix a concurrency issue between our different build tasks when running the watch command.

For the watch command to work, we need to set up the language server, run it, execute the watch process of the JS build (which includes doing the initial build) and executing the watch process for our rust build (which includes doing the initial rust build). The old setup did ((Build Language Server . Run Language Server. Watch Rust) | (Initialize JS Dependencies . Watch JS)) (CSS notation) but since setting up the language server requires the JS dependencies this meant interference of the build process. This was not a process in other commands as they never allowed running of these steps in parallel. Also, the timing between starting the watch process for JS and Rust is important because if they start in the wrong order the process ends up hanging and requires restarting.

The new sequence is now (Initialize JS Dependencies . Build Language Server . Run Language Server . Build Rust . Build JS . (Watch Rust | Watch JS)) , which adds more build time but starts up correctly.

Important Notes

We should consider moving to a task-based build system (e.g., cargo make) that has better support for specifying task dependencies while avoiding the pitfalls of having to code the build in JS and makes setup more re-usable.

The correct way to do this efficiently would be to separate out the language server (and other build steps) as a pre-step to starting the watch processes. We could then independently do (Build JS | Build Rust | Set Up Language Server) . Start Language Server . Start Backend . Start IDE

Checklist

Please include the following checklist in your PR:

  • The CHANGELOG.md was updated with the changes introduced in this PR.
  • The documentation has been updated if necessary.
  • All code conforms to the Rust style guide.
  • All code has automatic tests where possible.
  • All code has been profiled where possible.
  • All code has been manually tested in the IDE.
  • All code has been manually tested in the "debug/interface" scene.
  • All code has been manually tested by the PR owner against our test scenarios.
  • All code has been manually tested by at least one reviewer against our test scenarios.

@MichaelMauderer MichaelMauderer added Priority: Highest Should be completed ASAP Type: Bug A bug in Enso IDE Category: Build System The build scripts and CI. labels Mar 28, 2021
@MichaelMauderer MichaelMauderer self-assigned this Mar 28, 2021
@MichaelMauderer MichaelMauderer force-pushed the feature/wip/fix-initial-clean-watch branch from 9cd3ee1 to 19dedf8 Compare March 28, 2021 12:41
@MichaelMauderer MichaelMauderer force-pushed the feature/wip/fix-initial-clean-watch branch from 19dedf8 to ed3bfe2 Compare March 28, 2021 12:57
@MichaelMauderer MichaelMauderer marked this pull request as ready for review March 28, 2021 12:59
Copy link
Member

@wdanilo wdanilo left a comment

Choose a reason for hiding this comment

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

I dont have any comments but I really like the solution here. Very clean approach. Nice one!

@MichaelMauderer MichaelMauderer merged commit c4008d4 into develop Mar 31, 2021
@MichaelMauderer MichaelMauderer deleted the feature/wip/fix-initial-clean-watch branch March 31, 2021 14:31
mwu-tow pushed a commit to enso-org/enso that referenced this pull request Oct 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Category: Build System The build scripts and CI. Priority: Highest Should be completed ASAP Type: Bug A bug in Enso IDE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants