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

Make bower concurrent friendly, fixes #933 #1211

Merged
merged 1 commit into from
Apr 7, 2014

Conversation

sheerun
Copy link
Contributor

@sheerun sheerun commented Apr 4, 2014

Read my comment: #933 (comment)

This is the only reasonable solution to concurrency issues in bower.

@desandro
Copy link
Member

desandro commented Apr 4, 2014

Thank you for this patch! We'll be taking a look at this in a bit. We've got a bit of a back log at the moment, to it might take a while. Your patience is appreciated 🍌


// Check if directory exists
// Check if destination directory exists to prevent issuing lock at all times
return Q.nfcall(fs.stat, dir)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I think about it there is one scenario that can go wrong here:

  1. One worker checks that destination cache doesn't exist and begins copying from tmp
  2. Second worker checks that destination exists and returns cache location
  3. While first worker have not finished moving tmp to cache, second worker copies cache to bower_components, copying only half of files or so.

This is given bower doesn't wait for all downloads to be finished before copying them to bower_components.

If bower first resolves all downloads and only then copies to bower_components, it is not a problem.

If it doesn't wait, the solution is to use write lock at all times and remove this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can see from source that bower waits with dissection until all packages are fetched.
So it should be ok..

@sheerun
Copy link
Contributor Author

sheerun commented Apr 5, 2014

Additionally the --force behavior of missing the cache doesn't make sense with my change as cache is never overwritten (unless you use bower cache clean).

This means the --force causes component to be downloaded, but not copied to cache anyway.

I suggest removing --force behavior of skipping the cache (it's not documented nor tested).

Unfortunately bower cannot not use cache right now (it always copies from cache to bower_components, even if --force is used). So if one wants to skip global cache he can set cache location to .bower-cache for example.

@paulirish
Copy link
Member

I suggest removing --force behavior of skipping the cache

@sheerun seems reasonable. If you want, open a PR on that. Seems good.

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

Successfully merging this pull request may close these issues.

4 participants