-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Fix dragonfly casting #1237
Fix dragonfly casting #1237
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (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. Due to 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 the contribution instructions for more information. |
It seems to be failing on lint errors that are recreated on the file after running rustfmt. I suppose I could disable rust-mode in my Emacs in order to not trigger rustfmt on save, but I think the inconsistency in requirements should be documented. rustfmt shouldn't intentionally create something that will fail linting here and vice versa. |
This project does not support rustfmt (as you have discovered). Could you strip that commit from the PR ? If you want to add rustfmt support for libc, it is better to do so in a different PR. You are going to need to add a custom |
072c3e4
to
452f027
Compare
Ok. I've removed the formatting commit and left the one that allows it to correctly build and run on DragonFly. |
I think this overlaps with #1235 , going to hold on this until that one is merged. |
@strangelittlemonkey after #1235 lands could you please try running libc-test locally on your Dragonfly machine? It adds a regression test for |
I've tested against master, with the PR merged, it still has the exact same error that my PR fixes. |
What I meant it "do the runtime tests added in that PR pass after you merge your compile fixes"? |
@strangelittlemonkey would it be possible to rebase this on top of master ? Also, are there any cross-compile dragonflybsd targets on linux that might be missing on ci/build.sh ? |
452f027
to
4ccd317
Compare
I've rebased against master. I don't know that you'd support any ci stuff for dragonfly given it's tier III and you don't produce any binaries for it. |
I thought we might be able to cross-compile to it from Linux if we use |
@bors: r+ |
📌 Commit 4ccd317 has been approved by |
…zlbg Fix dragonfly casting This fixes a recent break in the casting on DragonFly and also a separate commit to bring the newer code into the same style with rustfmt. fixes #1238
💔 Test failed - checks-travis |
☔ The latest upstream changes (presumably #1254) made this pull request unmergeable. Please resolve the merge conflicts. |
I've disabled the android build jobs that were making bors fail, after rebase this should merge without issues. Sorry for the CI churn. |
It’s probably also because the use line should be moved up according to style, I can fix that shortly.
… On Feb 13, 2019, at 9:05 AM, gnzlbg ***@***.***> wrote:
I've disabled the android build jobs that were making bors fail, after rebase this should merge without issues. Sorry for the CI churn.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
4ccd317
to
2c33181
Compare
I addressed the line length and ordering issue that travis style complained about, I think it will pass now. |
Some conflicts poped up, this has to be rebased (sorry). |
2c33181
to
c32c80c
Compare
Ok, I've rebased your changes in, which ended up removing the need to have the dox::mem at the top now. |
Well, Travis failure this time says style but not related to me as it was because of inability to find rustfmt. |
Did you try running libc-test locally on Dragonfly? It doesn't run in CI. |
My servers are almost exclusively DragonFly BSD. I'm editing this and compiling it on DragonFly BSD systems. zach@dev ~/libc> cargo build running 0 tests test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out Doc-tests libc running 0 tests test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out zach@dev ~/libc> uname I'm also using my patched version for user and group libraries, so I know it works. |
Eh, I guess the test is kinda useless looking at it running 0 tests. |
Hmm, this fails. zach@dev ~/l/libc-test> cargo test
|
Oh wow, that's worse than I thought. The undefined symbols like |
I made some progress on it last night, clearly my test applications aren't making full use of the library. In any event, I got it down to two warnings, I'll see if I can get it clean over the weekend. |
@strangelittlemonkey any progress on this? I don't know if I should merge #1263 as soon as its ready, or wait for this to be fixed. Rebasing either of the PRs on top of the other shouldn't be too much work since the changes should be the same. |
Test that targets without a shipping libcore / libstd build properly This PR tests that `libc` builds on targets that currently don't ship a `libcore`/`libstd` by cross-compiling to them from Linux and MacOSX using `Xargo`. This fixes the build on DragonflyBSD (superseeds #1237), Haiku, bitrig, and OpenBSD (superseeds #1259). cc @asomers @strangelittlemonkey @semarie
Test that targets without a shipping libcore / libstd build properly This PR tests that `libc` builds on targets that currently don't ship a `libcore`/`libstd` by cross-compiling to them from Linux and MacOSX using `Xargo`. This fixes the build on DragonflyBSD (superseeds #1237), Haiku, bitrig, and OpenBSD (superseeds #1259). cc @asomers @strangelittlemonkey @semarie
Test that targets without a shipping libcore / libstd build properly This PR tests that `libc` builds on targets that currently don't ship a `libcore`/`libstd` by cross-compiling to them from Linux and MacOSX using `Xargo`. This fixes the build on DragonflyBSD (superseeds #1237), Haiku, bitrig, and OpenBSD (superseeds #1259). cc @asomers @strangelittlemonkey @semarie
Test that targets without a shipping libcore / libstd build properly This PR tests that `libc` builds on targets that currently don't ship a `libcore`/`libstd` by cross-compiling to them from Linux and MacOSX using `Xargo`. This fixes the build on DragonflyBSD (superseeds #1237), Haiku, bitrig, and OpenBSD (superseeds #1259). cc @asomers @strangelittlemonkey @semarie
☔ The latest upstream changes (presumably #1263) made this pull request unmergeable. Please resolve the merge conflicts. |
@strangelittlemonkey any progress on this? I'm on Zulip and Discord in case you encounter some errors that aren't clear enough and need help. |
Note the type signatures in the error message:
Note the last type is |
Sorry, I've been a bit busy with personal things of late. I'm not sure how or where the Zulip/Discord things are. Maybe I'm old, but I idle on IRC a lot. |
So I will hang out on IRC for the next couple of days in case you have questions :) (the only reason I use discord or zulip more is that i get notified when somebody pings me when i'm offline without needing a bouncer or similar) |
This needs to be rebased. |
@strangelittlemonkey ping :) let me know if there is a way I can help you with here! |
I'm going to close this as inactive and the build works now. Thanks! |
This fixes a recent break in the casting on DragonFly and also a separate commit to bring the newer code into the same style with rustfmt.
fixes #1238