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

Document stat.st_blksize changes on DragonFly BSD #2487

Merged
merged 1 commit into from
Nov 6, 2021

Conversation

rtzoeller
Copy link
Contributor

@rtzoeller rtzoeller commented Oct 30, 2021

The tests are currently failing on DragonFly BSD, due to changes to stat.st_blksize among other reasons.

st_blksize is now an i64, and has moved to take up a previously reserved struct member. Deprecate the struct, to indicate it will be modified in a subsequent release.

@rust-highfive
Copy link

r? @Amanieu

(rust-highfive has picked a reviewer for you, use r? to override)

@Amanieu
Copy link
Member

Amanieu commented Oct 30, 2021

What is the long-term solution for this? I don't see how the deprecation warning is helpful since there is no action anyone can do in their code to resolve it.

@rtzoeller
Copy link
Contributor Author

@Amanieu in a future release, we should update the struct definition to match DragonFly's. This is just an announcement of that future breaking change.

My understanding was that this was the correct course of action, per the breaking change policy and previous conversations with @JohnTitor.

If we should handle this differently, let me know and I can update the PR.

@Amanieu
Copy link
Member

Amanieu commented Oct 30, 2021

I will let @JohnTitor review this since you have previously discussed it with him.

r? @JohnTitor

@rust-highfive rust-highfive assigned JohnTitor and unassigned Amanieu Oct 30, 2021
Copy link
Member

@JohnTitor JohnTitor left a comment

Choose a reason for hiding this comment

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

Yeah, let's deprecate first so that users can notice the change and prepare their own struct if they want to use an older one.
Left some comments about version and note.

@rtzoeller rtzoeller force-pushed the dfly_stat_deprecation branch 2 times, most recently from 9a0074e to 1c126a2 Compare November 2, 2021 18:13
@rtzoeller rtzoeller requested a review from JohnTitor November 2, 2021 18:26
@JohnTitor
Copy link
Member

Thanks! @bors r+

@bors
Copy link
Contributor

bors commented Nov 2, 2021

📌 Commit 1c126a2 has been approved by JohnTitor

@bors
Copy link
Contributor

bors commented Nov 2, 2021

⌛ Testing commit 1c126a2 with merge 5fca7a7...

bors added a commit that referenced this pull request Nov 2, 2021
Document stat.st_blksize changes on DragonFly BSD

The tests are currently failing on DragonFly BSD, due to [changes to `stat.st_blksize`](DragonFlyBSD/DragonFlyBSD@34c6728) among other reasons.

`st_blksize` is now an `i64`, and has moved to take up a previously reserved struct member. Deprecate the struct, to indicate it will be modified in a subsequent release.
@bors
Copy link
Contributor

bors commented Nov 2, 2021

💔 Test failed - checks-actions

@JohnTitor
Copy link
Member

CI should be fixed now, @bors retry

@bors
Copy link
Contributor

bors commented Nov 3, 2021

⌛ Testing commit 1c126a2 with merge 4fbd84e...

bors added a commit that referenced this pull request Nov 3, 2021
Document stat.st_blksize changes on DragonFly BSD

The tests are currently failing on DragonFly BSD, due to [changes to `stat.st_blksize`](DragonFlyBSD/DragonFlyBSD@34c6728) among other reasons.

`st_blksize` is now an `i64`, and has moved to take up a previously reserved struct member. Deprecate the struct, to indicate it will be modified in a subsequent release.
@bors
Copy link
Contributor

bors commented Nov 3, 2021

💔 Test failed - checks-actions

@JohnTitor
Copy link
Member

Some functions need #[allow(deprecated)].

@rtzoeller rtzoeller force-pushed the dfly_stat_deprecation branch from 1c126a2 to 9d11d8d Compare November 6, 2021 03:30
@rtzoeller
Copy link
Contributor Author

@JohnTitor fixed! For some reason I couldn't see the actual error in the log on GitHub, and had to download it and view it locally.

@JohnTitor
Copy link
Member

For some reason I couldn't see the actual error in the log on GitHub, and had to download it and view it locally.

Hm, it's displayed fine on my env, maybe loading the log failed at that time?

Anyway, the build should be fixed, @bors r+

@bors
Copy link
Contributor

bors commented Nov 6, 2021

📌 Commit 9d11d8d has been approved by JohnTitor

@bors
Copy link
Contributor

bors commented Nov 6, 2021

⌛ Testing commit 9d11d8d with merge b916b48...

@bors
Copy link
Contributor

bors commented Nov 6, 2021

☀️ Test successful - checks-actions, checks-cirrus-freebsd-11, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13
Approved by: JohnTitor
Pushing b916b48 to master...

@bors bors merged commit b916b48 into rust-lang:master Nov 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants