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

build-style/cargo.sh: add [ci skip] #2837

Merged
merged 40 commits into from
Oct 15, 2018
Merged

Conversation

Cogitri
Copy link
Contributor

@Cogitri Cogitri commented Sep 16, 2018

supersedes #2436

[ci skip]

@travankor
Copy link
Contributor

Does this work for cross-compiling? I thought cargo needs some coaxing to cross-compile packages?

@jnbr
Copy link
Contributor

jnbr commented Sep 17, 2018

Cross-compiling can be ignored for now because our rustc builds aren't able to cross-compile.

@Cogitri
Copy link
Contributor Author

Cogitri commented Sep 17, 2018

Cross-compiling can be ignored for now because our rustc builds aren't able to cross-compile.

Yup, can't test it without being able to cross compile rust. I do plan to work on that in the near future though, I'll add the changes to this build style then (I don't think we should stall this until then though).

@travankor
Copy link
Contributor

Can you add a commit documenting this build style in Manual.md, mentioning that nocross should be set for now?

Yup, can't test it without being able to cross compile rust. I do plan to work on that in the near future though, I'll add the changes to this build style then (I don't think we should stall this until then though).

Ok, sounds nice.

Copy link
Contributor

@maxice8 maxice8 left a comment

Choose a reason for hiding this comment

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

Please modify Manual.md to include a short explaination of this build_style like it is done with the others.

@Cogitri
Copy link
Contributor Author

Cogitri commented Sep 18, 2018

Please modify Manual.md to include a short explaination of this build_style like it is done with the others.

Sure. Seems like work for cross compiling rust is currently being done though, so I'll leave the stuff about nocross out for now

@@ -0,0 +1 @@
hostmakedepends+=" cargo rust"
Copy link
Member

Choose a reason for hiding this comment

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

cargo already depends on rust, so we can optimize it away.

common/build-style/cargo.sh Show resolved Hide resolved
common/build-style/cargo.sh Outdated Show resolved Hide resolved
@wpbirney
Copy link
Contributor

wpbirney commented Sep 24, 2018

why not use rustup for the builds so we can enable cross compiling?

Rustup supports a ton of targets and would make cross compiling a breeze

@Cogitri
Copy link
Contributor Author

Cogitri commented Sep 24, 2018

why not use rustup for the builds so we can enable cross compiling?

Rustup supports a ton of targets and would make cross compiling a breeze

Hm, I'd like to avoid that if possible. With rustup we'd have to download the rust release stuff for every build (I don't think we can save the tarballs in hostdir/sources), which would be rather cumbersome (especially if one's connection is pretty bad). Also, we still need to provide a normal package for rust, some people want a system install of rust (e.g. for development) - although we could (in theory) just repackage rustup's tarballs (not a big fan of that though).

@maxice8
Copy link
Contributor

maxice8 commented Sep 24, 2018 via email

@wpbirney
Copy link
Contributor

wpbirney commented Sep 24, 2018

@maxice8 every rustc is a cross compiler, we could enable the targets in our rustc build and have cross compilation, its in the Config.toml. I have not looked into the template for rustc but will do so tonight

@Cogitri Cogitri changed the title build-style/cargo.sh: add build-style/cargo.sh: add [ci skip] Oct 2, 2018
@Cogitri
Copy link
Contributor Author

Cogitri commented Oct 2, 2018

This enables the use of the cargo buildstyle for all appropriate packages and as such enables cross building for most of them. Depends on #3233. Currently am cross compiling all packages to all arches. Also has the rust commit for cross building in it to make it a little easier to test this, that commit has to be dropped before this gets merged of course.

@Cogitri
Copy link
Contributor Author

Cogitri commented Oct 2, 2018

Alright, everything but a few packages (which I've marked as broken/nocross) compile for aarch64, testing other platforms...

@Cogitri Cogitri force-pushed the cargo-buildstyle branch 3 times, most recently from 7d35463 to 52b7247 Compare October 2, 2018 18:23
Manual.md Outdated
@@ -734,6 +734,12 @@ The current list of available `build_style` scripts is the following:
`do_configure()`, `do_build()`, etc., and may overwrite default `do_fetch()` and
`do_extract()` that fetch and extract files defined in `distfiles` variable.

- `cargo` For packages written in rust that use Cargo for building. Configuration
arguments (such as `--features`) can be passed to cargo in `do_build` via
`configure_args`. Sets the `RUST_TARGET` variable, which can be useful for
Copy link
Member

Choose a reason for hiding this comment

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

Configuration arguments [...] can be passed to cargo in do_build via configure_args.

This looks ambiguous: Could mean you need to pass configure args IN do_build.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, how would you write it? I basically only want to express that one uses configure_args to configure the build (and as such they're used in do_build instead of the usual do_configure)

Copy link
Member

@Gottox Gottox Oct 8, 2018

Choose a reason for hiding this comment

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

(Disclaimer: Not a native speaker)

Configuration arguments (such as --features ...) can be defined in the variable configure_args which is applied as command line arguments to cargo during do_build.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, that does sound better, I'll use something along the lines of that. Did you have a chance to test any of the cross compiled binaries yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it, please take a 2nd look at it.

@Cogitri
Copy link
Contributor Author

Cogitri commented Oct 15, 2018

Also currently testing rust-std splitting, I guess I'll open a PR for that later

@Gottox
Copy link
Member

Gottox commented Oct 15, 2018

Failing on armv7hf-musl:

rustup oxipng amp way-cooler taizen pijul alacritty

@Cogitri
Copy link
Contributor Author

Cogitri commented Oct 15, 2018

Needs #3798 now

Currently testing all arches with that.

@Cogitri
Copy link
Contributor Author

Cogitri commented Oct 15, 2018

Failing on armv7hf-musl:

rustup oxipng amp way-cooler taizen pijul alacritty

way-cooler and alacritty already have nocross

@Gottox
Copy link
Member

Gottox commented Oct 15, 2018

Hows testing? :)

@Cogitri
Copy link
Contributor Author

Cogitri commented Oct 15, 2018

Currently building all packages affected by this PR, armv6l{,-musl} already worked. Should go a lot faster now since the builders have finished building rust rev 4 :)

@Cogitri
Copy link
Contributor Author

Cogitri commented Oct 15, 2018

Also currently building a rpi3 root FS to test the cross compiled binaries

Ah, that was easier than expected :)

@Cogitri
Copy link
Contributor Author

Cogitri commented Oct 15, 2018

Alright, at least the aarch64 binaries work AFAICS

@Gottox Gottox merged commit d38e0a6 into void-linux:master Oct 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants