-
Notifications
You must be signed in to change notification settings - Fork 13.3k
bitrig integration #21959
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
bitrig integration #21959
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @pcwalton (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see CONTRIBUTING.md for more information. |
Fixed the whitespace errors. |
RUSTC_CROSS_FLAGS_$(1)=-C linker=$$(call FIND_COMPILER,$$(CC_$(1))) \ | ||
-C ar=$$(call FIND_COMPILER,$$(AR_$(1))) $(RUSTC_CROSS_FLAGS_$(1)) | ||
|
||
RUSTC_FLAGS_$(1)=$$(RUSTC_CROSS_FLAGS_$(1)) $(RUSTC_FLAGS_$(1)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this keep the previous indentation?
r? @brson |
Let me fix the other issues and push again. |
I rebased to master too. |
I did. In ad45164. |
@bors: r+ fc147f1 |
pub st_ctime_nsec: c_long, | ||
pub st_atim: timespec, | ||
pub st_mtim: timespec, | ||
pub st_ctim: timespec, | ||
pub st_size: off_t, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not completely agreed with the conversion form time_t + c_long
to timespec
.
Under bitrig and openbsd, the timespec definition is under conditionnal definition:
#if __POSIX_VISIBLE >= 200809 || __BSD_VISIBLE
. Here, the mod is posix01
.
Note: the C size of timespec
is the same as time_t + c_long
, so it don't break code, only the ABI.
see https://github.com/bitrig/bitrig/blob/master/sys/sys/stat.h#L53
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point. 2001 < 2008
@Manishearth good catch! oops! |
Alright, all review changes are made and I rebased to master. |
The rollup should land in a few minutes; it has conflicts with your PR : try rebasing over https://github.com/Manishearth/rust/compare/rollup (d3732a1) |
@Manishearth I just rebased to d3732a1. |
Great, looks mergeable now. |
@@ -4130,7 +4149,7 @@ pub mod consts { | |||
pub const _SC_THREAD_THREADS_MAX : c_int = 90; | |||
pub const _SC_THREADS : c_int = 91; | |||
pub const _SC_TTY_NAME_MAX : c_int = 107; | |||
pub const _SC_ATEXIT_MAX : c_int = 46; | |||
pub const _SC_ATEXIT_MAX : c_int = 146; | |||
pub const _SC_XOPEN_CRYPT : c_int = 117; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the _SC_ATEXIT_MAX value is wrong for openbsd and for bitrig too.
please keep 46
.
see https://github.com/bitrig/bitrig/blob/master/include/unistd.h#L199
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch again! Thank you for taking the time to look at my patch so closely. I must have added the '1' accidentally when I was going through the constants.
@bors: retry |
⌛ Testing commit 5513040 with merge 14ac4d0... |
💔 Test failed - auto-win-32-nopt-t |
@bors: retry |
This patch adds the necessary pieces to support rust on Bitrig https://bitrig.org
💔 Test failed - auto-mac-32-opt |
@alexcrichton @brson again, the auto-mac-32-opt failures are unrelated to my work. the first error is a panic in ReaderRng.fill_bytes:
and then there are some test failures:
I'm not sure what to do about these? They are unrelated to my changes. Should these prevent my changes from landing? |
@alexcrichton @brson should I rebase to get updated code that might fix the unrelated test failures? |
@bors: retry Oh this just looks like a spurious timeout failure |
⌛ Testing commit 5513040 with merge 2f3f7d5... |
💔 Test failed - auto-mac-64-opt |
@alexcrichton @brson again, I'm not sure why this test is failing:
I'm certain that this is unrelated to my patch. |
would rebasing help here? |
@dhuseby the error you cite isn't the cause of the build failed. I think the error line come from The problem is a timeout in testing std crate (20 min without output). I am not sure a rebasing would help. it seems some buildbots have some instabilities (timeout, file creation problem...). |
@alexcrichton how many more times do we have to repeat this before we declare the testing system broken and land the patch anyway? |
Sorry our buildbots aren't always the most reliable builders :( |
@bors: retry Apparently there have been a lot of timeouts lately, but unfortunately we can't land anything without a successful test run. |
I don't see other mac timeouts from looking through the build history. @alexcrichton says we're having a lot of problems with windows right now. |
⌛ Testing commit 5513040 with merge 9efabd8... |
💔 Test failed - auto-win-64-opt |
@bors: retry |
This patch adds the necessary pieces to support rust on Bitrig https://bitrig.org
This patch adds the necessary pieces to support rust on Bitrig https://bitrig.org