Cargo Build System + Nightly Updates #285
Conversation
|
@posborne If you could give the I'll work on getting travis working again. |
|
Ok, I'm done screwing around with travis, it should work now but is being prevented from passing due to an upstream bug in Cargo rust-lang/cargo#1612. alexcrichton made a fix for it that works, so we'll just have to wait for that to get pulled in. If you actually go examine the travis builds though, all of the tests do pass, for what it's worth. |
|
@mcoffin, blink_pt is working on my device. A few notes:
I can manually build by reordering the arguments to gcc to have all object files at the end, like this... |
|
Whoa, that's one big diff! I'll take a closer look on Monday, so far it looks a different approach that I tried with multi-crate system and it also looks more manageable. |
|
If anybody else is getting that linking error, I can't even reproduce it in a VM yet so it's hard to debug. More info would be appreciated. EDIT: Travis-CI should report all is good once rust-lang/cargo#1617 lands in cargo-nightly (and therefore in travis builds) |
|
@mcoffin, thanks for taking a look. I'll take a deeper look and see if I can determine the root cause. |
|
Ok, so it appears that I had previously left a key piece of information out of what I had snipped... So, the link step is failing as |
|
Interestingly enough, my libgcc also defines So I'll have to see why I'm not getting that error. What versions of rustc / cargo are you running? |
I'm using rustc 1.0.0 and cargo from Alex's branch to fix the issue you reported. I believe I saw the same problem with cargo nightly, but I can retest this evening. I also tried using gcc 4.8 last night and saw the same results. I am running Ubuntu 14.04 LTS on that machine. The 4.8 toolchain is from the standard repos and the 4.9 toolchain is from https://launchpad.net/~terry.guo/+archive/ubuntu/gcc-arm-embedded, which is the same PPA used by travis. |
| @@ -28,14 +28,24 @@ Zinc supports only ARM at the moment. The primary development is focused on two | |||
|
|
|||
| Zinc is distributed under Apache-2.0, see LICENSE for more details. | |||
|
|
|||
| ## Current Status | |||
|
|
|||
| Zinc is currently undergoing some large build system changes to keep up with the latest developments in rust. Some things, most notably `platformtree` are not currently building, so note that some examples may not build. All non-platformtree examples should work fine though. | |||
There was a problem hiding this comment.
This is now fixed, correct?
There was a problem hiding this comment.
Also, lines should be wrapped at ~80 characters, I think.
|
@farcaller By the way, now that we have an external project working that uses this as a dependency, all the huge changes here should stabilize if you want to start reviewing. I guess I opened a little bit early. |
|
@farcaller can you fork bharrisau/rust-libcore into hackndev as part of this PR. Also need a rlib too, @mcoffin is there a reason you forked your own instead of bharrisau/rust-librlibc? I've included libcore in the cargo deps for mine (as it is technically required). |
|
Just a suggestion, but can we have a build.sh script instead of having autotools as a dep? That also allows for a build.bat for windows users if need be. At the end of the day, we are only covering 3 things:
Could we even just document these three command invokations and remove the autotools, or am I missing something? |
| @@ -51,3 +46,9 @@ pub fn main() { | |||
| wfi(); | |||
| } | |||
| } | |||
|
|
|||
| #[start] | |||
| fn start(_: isize, _: *const *const u8) -> isize { | |||
There was a problem hiding this comment.
Happy to 'keep it simple' and have this (because I know how annoying it is defining #start in Zinc). But for 1.0 we should really have pre-defined #start and it should be calling init_stack and init_data.
|
LGTM
|
|
On the |
AutotoolsThe autotools script is there for two purposes currently:
We could always remove the autotools script, and move the responsibility for that first purpose into the Makefile, but I didn't do it because then the user would have to patch the Makefile with their toolchain information again, and I think it feels more slick and standard to just run a SpacingAgh. I just realized how much this is scattered everywhere. The official rust style guide recommends 4 spaces... maybe we should consider converting the project rather than the commit to follow the standard? If not I can go through and convert my work to 2 space indentation. StartI agree that the user shouldn't have to actually write the For non-platformtree consumers, we might want to just provide a dummy rlibc ForkI did this because I had originally been building libcore out of tree (which meant I was using the crates.io version of rlibc). When I moved libcore building back in to the project, I forgot someone had already done it. It looks like you might be a little bit behind upstream changes too (my fork too, strangely enough). I really really want to get rid of the manual dependency on libcore, but I just don't see a good way to go about it just yet. |
|
Re: spacing. I've always been easy on 2 vs 4, as long as we stick to one type (my vim defaults to 4 spaces, but I just override it for zinc projects). I think last I talked to @farcaller he was amenable to moving to 4 spaces, but only if someone else did it (waiting for a rustfmt like utility). I'm not sure what his current thinking is. Ok, I'm fine with autotools. If build.sh can map from Yep, let's keep it simple and deal with |
| @@ -0,0 +1,3 @@ | |||
| [target.@TARGET@] | |||
There was a problem hiding this comment.
@bharrisau This is why the platform -> target mapping is currently done in configure. If we move it to the Makefile, then we have to manually define all the targets in this file... and then I don't know how I would go about looking for every toolchain.
There was a problem hiding this comment.
If you've looked at the alternative and this is the best option, then I'm happy to go with it.
|
Fixed the spacing issues. I'll squash things in some meaningful way once I get the go-ahead. |
|
On spacing, I too would be interested in seeing the move to 4-spaces. The emacs mode for rust also (and rightly) defaults to 4-space indentation now that this is part of the style guide. I wouldn't gate this PR on it, however but might be nice to merge before we have too many people spun up on Zinc again (I'm ignoring other open PRs as this change will break most of them anyway). This does seem to be undergoing active development. Might not work for zinc with all of the stuff we do with syntax extensions, but could be worth a shot: https://github.com/nrc/rustfmt |
|
Honestly, I'm going to play with it a little bit tomorrow, but there's only a few things that break with just using rust.vim and doing a good old One of them is quoting in syntax extensions... and the other is when you have a newline immediately before the return value of a function. It's enough that I don't want to deal with it though. |
|
First of all, thanks for keeping up this talk active. I am sorry for not paying much attention, I am stuck with family issues I need to solve. AutotoolsI think it's reasonable to use them here. They are more standard and expected to work fine on all the *nix variants. SpacingNow that rust style guide is settling down we can refactor all the code to follow. I don't like project-scale formatting refactoring as git blame becomes useless, but we break enough of zinc now to not care much I guess.
Done |
|
@bharrisau @posborne that should do it. Eventually we may want to consider allowing for unwinding, but that will drastically increase binary size. |
|
Unwinding for us is hitting the fault handler really. Stack overflow will
|
|
Well in that case, then what I have above should duplicate what was going on before. |
|
@mcoffin, I tested the fix and the UART app builds and spits out bytes at 115200 on the UART. |
|
Okay, I'm going to land this and try to fix the build sizes reporting on ci. Once again, thanks for a great job. |
Refactored build system and fixed issues with current nightly
|
|
|
@farcaller How so? It tries to compile the examples by default (which won't work without the right linker script / flags since its trying to compile for the host platform). Try |
|
I mean, there's no straightforward way to run tests for subcrates, and those should be part of travis workflow. |
|
I just added them to travis manually; it wasn't that big of a pita tbh. |
|
Oh, I totally missed that part, thanks for pointing it out. |
Spelling problem in Cargo.toml Fallout from hackndev#285
Don't pull this directly as it has a TON of stupidly small commits. I'll squash them when we deem it ready.Done.This uses cargo to do most of the heavy lifting as far as the compilation of rust (and even the final linking). I tried to make it a lot less of a fully custom build system and more of a small Makefile built on top of cargo to get us the last few steps of the way.
I also fixed a lot of the fallout from rust updates on nightly, some of which were easier than others.
Most notably: this also means that people can create their own cargo projects and just list zinc, platformtree, etc as dependencies.