Skip to content

Lock Cache before downloading Dart SDK on Windows#7931

Merged
goderbauer merged 2 commits intoflutter:masterfrom
goderbauer:cacheLockWin
Feb 7, 2017
Merged

Lock Cache before downloading Dart SDK on Windows#7931
goderbauer merged 2 commits intoflutter:masterfrom
goderbauer:cacheLockWin

Conversation

@goderbauer
Copy link
Member

fixes #7921

@goderbauer goderbauer requested a review from Hixie February 7, 2017 19:40
bin/.gitignore Outdated
@@ -1,2 +1,2 @@
cache

flutter.lock
Copy link
Contributor

Choose a reason for hiding this comment

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

we have a lockfile already, i would just use the same one that we have for linux

Copy link
Member Author

Choose a reason for hiding this comment

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

On Linux, the bash script itself is used as the lock file [1]. This doesn't work on Windows because a file that's open for write cannot be opened for read: Trying to start a flutter.bat instance while another flutter.bat is running and holding a write lock on flutter.bat results in an (uncatchable) OS-level error.

Maybe you are thinking about the lockfile we use on all platforms in dart land [2]. Re-using that one would change the semantics slightly because now dart land and the batch script would share the same lock. That's not necessary, though, because the batch script's download of the dart-sdk in one instance of flutter_tools and dart land's download of engine artifacts in another instance of flutter_tools can happen in parallel without problems.

[1] https://github.com/flutter/flutter/blob/master/bin/flutter#L79
[2] https://github.com/flutter/flutter/blob/master/packages/flutter_tools/lib/src/cache.dart#L57

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, fair enough. In that case I would create a second file next to the first specifically for the batch file.

The point of having the file be on the filesystem at all times is to discourage people from thinking that the file's existence is relevant to whether things are locked. It should be trivially provable that everyone should always have the file, and thus the lock can't be based on the existence of the file.

Copy link
Member Author

Choose a reason for hiding this comment

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

Talked in person. The other lockfile is also never checked in. We decided to move this lock file into the cache dir, though.

@Hixie
Copy link
Contributor

Hixie commented Feb 7, 2017

RSLGTM

This is some dark magic right there.

@goderbauer goderbauer merged commit e01470a into flutter:master Feb 7, 2017
@goderbauer goderbauer deleted the cacheLockWin branch February 7, 2017 23:02
@goderbauer goderbauer added the platform-windows Building on or for Windows specifically label Oct 17, 2017
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

platform-windows Building on or for Windows specifically

Projects

None yet

Development

Successfully merging this pull request may close these issues.

flutter.bat doesn't lock the cache directory before downloading artifacts

3 participants