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

Attempt to cache using a temp file if home is unwriteable #30

Merged
merged 1 commit into from
Dec 8, 2015

Conversation

elldritch
Copy link
Contributor

This helps alleviate #22 and #19, by attempting to use a temporary file to cache if the home directory is unwriteable. Previously, a temporary file was only used if the home directory could not be found.

@tkellen
Copy link

tkellen commented Nov 30, 2015

Thanks @ilikebits!

Can you add a passing test for this that fails without this addition?

@phated
Copy link
Member

phated commented Nov 30, 2015

@tkellen isn't this similar to the patch that caused all the havoc not too long ago?

@elldritch
Copy link
Contributor Author

@phated, that patch (#21) attempted to create new cache directories; this one fixes current cache behaviour to be as described. When $HOME is writeable, it behaviour is the same as it is now -- at the very least, I can't see this causing more problems.

@tkellen, working on tests now. There seem to be a couple of bugs with existing tests e.g. files written to temporary directories are not correctly cleaned up after each test because any invalid path passed to fs.unlinkSync or path.resolve will throw an error and prevent later (and possibly valid) paths from being removed. Is there any reason os is required at the test level instead of the file level?

@elldritch
Copy link
Contributor Author

@tkellen Tests finished. I simulate an unwriteable $HOME by setting it to a non-existent directory.

@tkellen
Copy link

tkellen commented Dec 4, 2015

Thanks @ilikebits! This looks good to me. Can you please squash your commits? Also, if I give you commit and npm publish access for this repo, would you be willing to triage any issues that result?

@elldritch
Copy link
Contributor Author

Sure thing; this bug blocks me from deploying on my PaaS provider, and I'd be happy to help out others with the same issue. I'm @ilikebits on npm as well.

tkellen pushed a commit that referenced this pull request Dec 8, 2015
Attempt to cache using a temp file if home is unwriteable
@tkellen tkellen merged commit 6fe4c2e into gulpjs:master Dec 8, 2015
@tkellen
Copy link

tkellen commented Dec 8, 2015

Sounds good @ilikebits! Matster is all set to npm publish the next version and you have author access on npm. Cut a release when ready :)

@tkellen
Copy link

tkellen commented Dec 17, 2015

Hey @ilikebits! Are you going to publish this?

@elldritch
Copy link
Contributor Author

Sorry for the delay; it's finals crunch time right now, planning to release this weekend post-finals so I can address any bugs that come up.

phated pushed a commit that referenced this pull request Dec 17, 2018
Attempt to cache using a temp file if home is unwriteable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants