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

Add OsString from/to bytes helper functions #993

Merged
merged 12 commits into from
Oct 23, 2019
Merged

Add OsString from/to bytes helper functions #993

merged 12 commits into from
Oct 23, 2019

Conversation

pvdrz
Copy link
Contributor

@pvdrz pvdrz commented Oct 13, 2019

Related issue: #989

r? @RalfJung

This was referenced Oct 13, 2019
Merged
Merged
Merged
@pvdrz pvdrz closed this Oct 14, 2019
@pvdrz pvdrz reopened this Oct 14, 2019
RalfJung
RalfJung reviewed Oct 16, 2019
View reviewed changes
src/helpers.rs Outdated Show resolved Hide resolved
RalfJung
RalfJung reviewed Oct 17, 2019
View reviewed changes
src/helpers.rs Outdated Show resolved Hide resolved
RalfJung
RalfJung reviewed Oct 17, 2019
View reviewed changes
src/helpers.rs Outdated Show resolved Hide resolved
RalfJung
RalfJung reviewed Oct 17, 2019
View reviewed changes
src/helpers.rs Outdated Show resolved Hide resolved
RalfJung
RalfJung reviewed Oct 17, 2019
View reviewed changes
src/helpers.rs Outdated Show resolved Hide resolved
@bors
Copy link
Contributor

bors commented Oct 18, 2019

☔ The latest upstream changes (presumably #1004) made this pull request unmergeable. Please resolve the merge conflicts.

RalfJung
RalfJung reviewed Oct 18, 2019
View reviewed changes
src/helpers.rs Outdated Show resolved Hide resolved
@bors
Copy link
Contributor

bors commented Oct 18, 2019

☔ The latest upstream changes (presumably #1005) made this pull request unmergeable. Please resolve the merge conflicts.

@RalfJung
Copy link
Member

@christianpoveda please don't delete comments, rather mark then as resolved when you resolved them. I got a notification about getcwd breakage and that seems like useful information to keep in the history, even if you managed to fix it later.

RalfJung
RalfJung reviewed Oct 19, 2019
View reviewed changes
src/helpers.rs Outdated Show resolved Hide resolved
RalfJung
RalfJung reviewed Oct 19, 2019
View reviewed changes
src/helpers.rs Outdated Show resolved Hide resolved
RalfJung
RalfJung reviewed Oct 19, 2019
View reviewed changes
src/helpers.rs Outdated Show resolved Hide resolved
RalfJung
RalfJung reviewed Oct 19, 2019
View reviewed changes
src/helpers.rs Outdated Show resolved Hide resolved
@pvdrz
Copy link
Contributor Author

pvdrz commented Oct 19, 2019 •
edited
Loading

@christianpoveda please don't delete comments, rather mark then as resolved when you resolved them. I got a notification about getcwd breakage and that seems like useful information to keep in the history, even if you managed to fix it later.

Just for the future us (and other minds doomed to suffer the same fate as us :P): The problem was that I was trying to write using Allocation:write_bytes and it was failing to write all the bytes causing an invalid memory access in other places. I changed to Allocation::get_bytes_mut and everything is working nice and dandy now.

Edit: This was my fault all the time... see: #993 (comment)

@bors
Copy link
Contributor

bors commented Oct 21, 2019

☔ The latest upstream changes (presumably #991) made this pull request unmergeable. Please resolve the merge conflicts.

@RalfJung RalfJung mentioned this pull request Oct 21, 2019
Closed
3 tasks
@bors
Copy link
Contributor

bors commented Oct 22, 2019

☔ The latest upstream changes (presumably #1014) made this pull request unmergeable. Please resolve the merge conflicts.

@RalfJung
Copy link
Member

RalfJung commented Oct 22, 2019 via email

@pvdrz
Copy link
Contributor Author

pvdrz commented Oct 22, 2019

Ok then we are good to go

RalfJung
RalfJung reviewed Oct 22, 2019
View reviewed changes
src/shims/env.rs Outdated Show resolved Hide resolved
RalfJung
RalfJung reviewed Oct 22, 2019
View reviewed changes
src/helpers.rs Outdated Show resolved Hide resolved
RalfJung
RalfJung reviewed Oct 23, 2019
View reviewed changes
src/helpers.rs Outdated Show resolved Hide resolved
@pvdrz pvdrz requested a review from RalfJung October 23, 2019 13:58
@RalfJung
Copy link
Member

Thanks for the endurance, this is great. :)

@bors r+

@bors
Copy link
Contributor

bors commented Oct 23, 2019

📌 Commit de12cbc has been approved by RalfJung

@bors
Copy link
Contributor

bors commented Oct 23, 2019

⌛ Testing commit de12cbc with merge a05f2aa...

bors added a commit that referenced this pull request Oct 23, 2019
@bors
Auto merge of #993 - christianpoveda:os_string_helper, r=RalfJung
a05f2aa 
Add OsString from/to bytes helper functions

Related issue: #989

r? @RalfJung
@pvdrz
Copy link
Contributor Author

pvdrz commented Oct 23, 2019

OK now I can start with the other stuff :)

@bors
Copy link
Contributor

bors commented Oct 23, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: RalfJung
Pushing a05f2aa to master...

@bors bors merged commit de12cbc into rust-lang:master Oct 23, 2019
@RalfJung RalfJung mentioned this pull request Oct 24, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oli-obk oli-obk oli-obk left review comments

@RalfJung RalfJung Awaiting requested review from RalfJung

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pvdrz @bors @RalfJung @oli-obk