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

crossbeam-utils: check only the architecture, not the whole target string #751

Closed
wants to merge 1 commit into from

Conversation

kanavin
Copy link

@kanavin kanavin commented Nov 3, 2021

There can be custom targets in use, and it's not possible to make a list
of them; for the check only the first item in the target string is actually
relevant (the architecture of the target, which is generally consistent
between standard and custom targets).

Signed-off-by: Alexander Kanavin alex@linutronix.de

Copy link
Member

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

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

In some cases, the kinds of atomic operations supported depend on the OS. For example:

Currently, the recommended way to build for custom targets is to use rustflags to pass the cfgs.
I would accept a patch to improve the situation, but we need to handle the edge cases mentioned above (in a way that does not manually maintain a list of such targets, whenever possible).

…ring

There can be custom targets in use, and it's not possible to make a list
of them; for the check only the first item in the target string is actually
relevant (the architecture of the target, which is generally consistent
between standard and custom targets).

Signed-off-by: Alexander Kanavin <alex@linutronix.de>
@kanavin
Copy link
Author

kanavin commented Nov 3, 2021

Currently, the recommended way to build for custom targets is to use rustflags to pass the cfgs. I would accept a patch to improve the situation, but we need to handle the edge cases mentioned above (in a way that does not manually maintain a list of such targets, whenever possible).

Thanks. The context is that this patch comes from the efforts to cross-compile librsvg in the Yocto project, where crossbeam is included into the librsvg tarball as a vendored library. For various reasons Yocto cannot use the standard rust targets (e.g. the compiler binary is hardcoded in them to 'cc' which is something we cannot use).

I guess I need to find out how to convince librsvg's build system to pass those custom flags to crossbeam, instead of running build.rs - if you can take the librsvg tarball and check what it does, I'd appreciate:
https://download.gnome.org/sources/librsvg/2.52/

Just for reference, you can see our configuration ('recipe' in yocto parlance) for librsvg here:
http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/tree/meta/recipes-gnome/librsvg/librsvg_2.52.3.bb?h=akanavin/package-version-updates

@kanavin
Copy link
Author

kanavin commented Nov 4, 2021

So yes, it's actually easier to just pass the needed flags via RUSTFLAGS on architectures where it's needed:

RUSTFLAGS:append:mips = " --cfg crossbeam_no_atomic_64"
RUSTFLAGS:append:powerpc = " --cfg crossbeam_no_atomic_64"

Thanks!

@kanavin kanavin closed this Nov 4, 2021
halstead pushed a commit to openembedded/openembedded-core that referenced this pull request Nov 5, 2021
Do not try to mangle the upstream list of targets;
after discussion with upstream it turns out it's neither
necessary nor upstreamable:
crossbeam-rs/crossbeam#751

Signed-off-by: Alexander Kanavin <alex@linutronix.de>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
kraj pushed a commit to YoeDistro/poky-old that referenced this pull request Nov 5, 2021
Do not try to mangle the upstream list of targets;
after discussion with upstream it turns out it's neither
necessary nor upstreamable:
crossbeam-rs/crossbeam#751

(From OE-Core rev: e54f9d1b40b14038dced0cd071a3022a3ea9dff4)

Signed-off-by: Alexander Kanavin <alex@linutronix.de>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
kraj pushed a commit to YoeDistro/openembedded-core that referenced this pull request Nov 6, 2021
Do not try to mangle the upstream list of targets;
after discussion with upstream it turns out it's neither
necessary nor upstreamable:
crossbeam-rs/crossbeam#751

Signed-off-by: Alexander Kanavin <alex@linutronix.de>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
halstead pushed a commit to openembedded/openembedded-core that referenced this pull request Nov 7, 2021
Do not try to mangle the upstream list of targets;
after discussion with upstream it turns out it's neither
necessary nor upstreamable:
crossbeam-rs/crossbeam#751

Signed-off-by: Alexander Kanavin <alex@linutronix.de>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
seambot pushed a commit to seamapi/poky that referenced this pull request Nov 7, 2021
Do not try to mangle the upstream list of targets;
after discussion with upstream it turns out it's neither
necessary nor upstreamable:
crossbeam-rs/crossbeam#751

(From OE-Core rev: 97562668e1a76710acd2be7c8cdbcc1deb80bd9f)

Signed-off-by: Alexander Kanavin <alex@linutronix.de>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
bors bot added a commit that referenced this pull request Nov 17, 2022
922: When building for linux, replace all vendors with 'unknown' r=taiki-e a=kanavin

Some build systems such as the Yocto project define custom rust targets, of the form 'arch-somevendor-linux-{gnu|musl}' (only the 'somevendor' part is custom, the rest matches rust's definitions).

This causes mismatches with crossbeam's lists of targets which are built from the standard rust target list, and always have 'unknown' as the vendor.

This change simply replaces such custom vendors with 'unknown' when the third component of the target is 'linux'.

---
Note from Alex: this is a reworked and hopefully better/more precise/acceptable version of #751

926: crossbeam-epoch: Bump memoffset to 0.7 r=taiki-e a=glittershark

mostly to keep the number of dupes of crates in my dependency graph down

Co-authored-by: Alexander Kanavin <alex@linutronix.de>
Co-authored-by: Griffin Smith <root@gws.fyi>
daregit pushed a commit to daregit/yocto-combined that referenced this pull request May 22, 2024
…per-target

Do not try to mangle the upstream list of targets;
after discussion with upstream it turns out it's neither
necessary nor upstreamable:
crossbeam-rs/crossbeam#751

(From OE-Core rev: 97562668e1a76710acd2be7c8cdbcc1deb80bd9f)

Signed-off-by: Alexander Kanavin <alex@linutronix.de>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
daregit pushed a commit to daregit/yocto-combined that referenced this pull request May 22, 2024
…per-target

Do not try to mangle the upstream list of targets;
after discussion with upstream it turns out it's neither
necessary nor upstreamable:
crossbeam-rs/crossbeam#751

(From OE-Core rev: 97562668e1a76710acd2be7c8cdbcc1deb80bd9f)

Signed-off-by: Alexander Kanavin <alex@linutronix.de>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants