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.

Sign up for GitHub

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

Jump to bottom

close file before removing it in check writeable #3636

Merged
merged 1 commit into from
Jan 27, 2017
Merged

close file before removing it in check writeable #3636

merged 1 commit into from
Jan 27, 2017

Conversation

whyrusleeping
Copy link
Member

Should half-close #3632 (along with a flatfs update)

License: MIT
Signed-off-by: Jeromy jeromyj@gmail.com

@whyrusleeping
close file before removing it in check writeable
7fc9c30 
License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
@whyrusleeping whyrusleeping added the status/in-progress In progress label Jan 27, 2017
@whyrusleeping whyrusleeping added this to the ipfs 0.4.5 milestone Jan 27, 2017
Kubuxu
Kubuxu approved these changes Jan 27, 2017
View reviewed changes
Copy link
Member

@Kubuxu Kubuxu left a comment

Choose a reason for hiding this comment

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

I am not really clear why it is necessary but if it resolves the issue, LGTM.

@whyrusleeping
Copy link
Member Author

@Kubuxu in windows, the Remove call was erroring because we had an open valid file descriptor for the file.

@Kubuxu
Copy link
Member

Kubuxu commented Jan 27, 2017

Ahh, 100% LGTM then. I've forgot about Windows.

@whyrusleeping
Copy link
Member Author

CHANGELOG:
Fix 'Writeable' helper method behavior on windows.

@whyrusleeping whyrusleeping merged commit c99a82d into master Jan 27, 2017
@whyrusleeping whyrusleeping deleted the fix/writeable-file-close branch January 27, 2017 17:13
@whyrusleeping whyrusleeping removed the status/in-progress In progress label Jan 27, 2017
@Kubuxu Kubuxu added the RFM label Jan 27, 2017
kevina
kevina reviewed Jan 27, 2017
View reviewed changes
Copy link
Contributor

@kevina kevina left a comment

Choose a reason for hiding this comment

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

LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kevina kevina kevina left review comments

@Kubuxu Kubuxu Kubuxu approved these changes

Assignees
No one assigned
Labels
RFM
Projects
None yet
Milestone
ipfs 0.4.5
Development

Successfully merging this pull request may close these issues.

Flatfs initialization bombs on windows
3 participants
@whyrusleeping @Kubuxu @kevina