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

Implement a simple "build.lock" file. #780

Closed
wants to merge 9 commits into from

Conversation

matanlurey
Copy link
Contributor

Fixes #352.

Let me know if you think we should add anything else (tests, etc). I did a couple of tests locally with the e2e_example package, and it seems to work pretty well.

@googlebot googlebot added the cla: yes Google is happy with the PR contributors label Dec 23, 2017
@matanlurey
Copy link
Contributor Author

Boy this is tricky. I might revert all changes but the lock file implementation itself until I can get either of your help. Let me know.

@jakemac53
Copy link
Contributor

Yes, this is trickier than it appears - even if you properly clean up the lockfile after the builds then you still might miss the case where somebody kills the task manually in the middle of a build. I also would prefer the lockfile to be in the .dart_tool directory - but that would be harder to clean up on errors.

One option that comes to mind is never actually trying to delete the lockfile at all. Instead you just put the pid of the current build task in it. Then when another build tries to run it sees if there is an active task that looks like a build script (the hard part...) with that pid. If not it replaces the file with its own pid and does a build and otherwise it errors or possibly waits to start a build.

Some other thoughts:

  • We would like to be able to run tests while you have a server going - today the test command always does a build which would error if there was a server running since it would have the lockfile.
    • We could possibly run tests in --pub-serve mode in this case, if we add similar hacks into the server to serve the html files that the test package expects.
    • The test command could possibly skip the build in this case, and assume everything is up to date.
    • Or possibly the file could contain some additional information about if a build is currently happening.
    • This stuff all only works well in the context of full builds - which likely makes it not a good path for the future where you can serve/build specific directories/files.
  • We could only take ownership of the lockfile during an actual build - so watch/serve would release it after each build. This would get complicated though because their state would not get updated if other builds happened so you would end up doing extra work.
  • Another option could be to run more as a build server - the lockfile would contain a host/port of an active server to schedule builds with or something. This gets a lot more complicated but would likely result in a better experience overall... except that it would be difficult to debug.

@matanlurey matanlurey closed this Jan 23, 2018
@matanlurey matanlurey deleted the feat-lock branch January 23, 2018 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google is happy with the PR contributors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants