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

libc-misc test freebsd fixes attempt #3177

Merged
merged 1 commit into from
Nov 20, 2023

Conversation

devnexen
Copy link
Contributor

No description provided.

@devnexen devnexen force-pushed the fix_pthread_misc_fbsd branch 4 times, most recently from 9867993 to 15bf480 Compare November 18, 2023 22:48
@devnexen devnexen marked this pull request as ready for review November 18, 2023 23:15
@devnexen devnexen force-pushed the fix_pthread_misc_fbsd branch 2 times, most recently from fd01ea5 to e47c669 Compare November 19, 2023 07:35
@devnexen devnexen force-pushed the fix_pthread_misc_fbsd branch 3 times, most recently from ac7b2f5 to bf66c21 Compare November 19, 2023 16:25
@devnexen
Copy link
Contributor Author

@rustbot review

@rustbot rustbot added the S-waiting-on-review Status: Waiting for a review to complete label Nov 19, 2023
let result = this.ftruncate64(fd, length)?;
this.write_scalar(result, dest)?;
}
"ftruncate" => {
Copy link
Member

Choose a reason for hiding this comment

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

If you're adding this for all targets now we should probably have a test for it... libc-fs.rs sound like the right file.

We've historically been pretty bad with libc tests for our FS APIs though. And test_posix_mkstemp should probably be in libc-fs.rs as well. So if you prefer we can open an issue to track fixing this in the future.

Copy link
Member

Choose a reason for hiding this comment

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

There's still no test for this function, is there?

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 did not add purposely, can be done separately.

Copy link
Member

@RalfJung RalfJung Nov 20, 2023

Choose a reason for hiding this comment

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

I said it should either be done here or have an issue to track it. :)
I created an issue: #3179

@devnexen devnexen force-pushed the fix_pthread_misc_fbsd branch 6 times, most recently from 1df561d to 553db9a Compare November 19, 2023 20:20
@devnexen
Copy link
Contributor Author

devnexen commented Nov 19, 2023

@rustbot review

@@ -1,4 +1,5 @@
//@ignore-target-windows: no libc on Windows
//@ignore-target-freebsd
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
//@ignore-target-freebsd
//@ignore-target-freebsd: FIXME needs foreign function `stat@FBSD_1.0`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note could be fixed in "all fs" next PR eventually.

@devnexen devnexen force-pushed the fix_pthread_misc_fbsd branch 2 times, most recently from cf2a07f to ea353db Compare November 20, 2023 11:41
@devnexen devnexen force-pushed the fix_pthread_misc_fbsd branch from ea353db to 2472e70 Compare November 20, 2023 12:10
@RalfJung
Copy link
Member

Looks good, thanks!
@bors r+

@bors
Copy link
Contributor

bors commented Nov 20, 2023

📌 Commit 2472e70 has been approved by RalfJung

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Nov 20, 2023

⌛ Testing commit 2472e70 with merge c71353d...

@bors
Copy link
Contributor

bors commented Nov 20, 2023

☀️ Test successful - checks-actions
Approved by: RalfJung
Pushing c71353d to master...

@bors bors merged commit c71353d into rust-lang:master Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Waiting for a review to complete
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants