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

Need --cfg=libc_align in v0.3.16 but not v0.3.15; why? #130

Closed
wchargin opened this issue Nov 11, 2020 · 4 comments
Closed

Need --cfg=libc_align in v0.3.16 but not v0.3.15; why? #130

wchargin opened this issue Nov 11, 2020 · 4 comments

Comments

@wchargin
Copy link

Hi! I transitively use socket2 as part of a Rust project built with
Bazel. I’m noting that socket2 builds successfully for me at v0.3.15,
but not at version v0.3.16 (released today) unless I specify the extra
flag --cfg=libc_align. I’m a bit hesitant to do so because it feels
low-level and platform specific, so I wanted to check and see whether
this is expected or whether there’s a better solution?

Here is a repro repo. You do need to have Bazel to repro, but you
don’t need any additional setup. The error is:

INFO: From Compiling Rust lib socket2 v0.3.16 (7 files):
error[E0063]: missing field `__align` in initializer of `libc::in6_addr`
   --> external/raze__socket2__0_3_16/src/sockaddr.rs:243:25
    |
243 |         let sin6_addr = in6_addr {
    |                         ^^^^^^^^ missing `__align`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0063`.
ERROR: /HOMEDIR/.cache/bazel/_bazel_wchargin/e6b2d93af64d3456b38c9c9d2131e939/external/raze__socket2__0_3_16/BUILD.bazel:33:13: output 'external/raze__socket2__0_3_16/libsocket2--1560485693.rlib' was not created
ERROR: /HOMEDIR/.cache/bazel/_bazel_wchargin/e6b2d93af64d3456b38c9c9d2131e939/external/raze__socket2__0_3_16/BUILD.bazel:33:13: not all outputs were created or valid
Target @raze__socket2__0_3_16//:socket2 failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 0.707s, Critical Path: 0.20s
INFO: 2 processes: 1 internal, 1 linux-sandbox.
FAILED: Build did NOT complete successfully

I dug far enough into the libc structure to see the align/no_align
modules and guess that this cfg flag would probably fix it; it does.

The failing Bazel action, with directory structure and command line
invocation, is here (in a Gist because it has some long paths):
https://gist.github.com/wchargin/4da947cf43d2a241cfffb89873788a82

In all cases, the libc dependency is at version 0.2.80. But I note
that socket2 v0.3.15 depends on libc with default features, whereas
in v0.3.16 it depends on the align feature:

$ cargo tree -e features --manifest-path socket2-0.3.15/Cargo.toml
socket2-libc-align-demo v0.0.0 (/HOMEDIR/git/socket2-libc-align-demo/socket2-0.3.15)
└── socket2 feature "default"
    └── socket2 v0.3.15
        ├── cfg-if feature "default"
        │   └── cfg-if v0.1.10
        └── libc feature "default"
            ├── libc v0.2.80
            └── libc feature "std"
                └── libc v0.2.80
$ cargo tree -e features --manifest-path socket2-0.3.16-default/Cargo.toml
socket2-libc-align-demo v0.0.0 (/HOMEDIR/git/socket2-libc-align-demo/socket2-0.3.16-default)
└── socket2 feature "default"
    └── socket2 v0.3.16
        ├── cfg-if feature "default"
        │   └── cfg-if v0.1.10
        ├── libc feature "align"
        │   └── libc v0.2.80
        └── libc feature "default"
            ├── libc v0.2.80
            └── libc feature "std"
                └── libc v0.2.80
$ cargo tree -e features --manifest-path socket2-0.3.16-libc-align/Cargo.toml
socket2-libc-align-demo v0.0.0 (/HOMEDIR/git/socket2-libc-align-demo/socket2-0.3.16-libc-align)
└── socket2 feature "default"
    └── socket2 v0.3.16
        ├── cfg-if feature "default"
        │   └── cfg-if v0.1.10
        ├── libc feature "align"
        │   └── libc v0.2.80
        └── libc feature "default"
            ├── libc v0.2.80
            └── libc feature "std"
                └── libc v0.2.80

So I’m hoping that you can help me understand what caused this change,
why it requires me to build with --cfg=libc_align, (maybe) why it’s
needed with Bazel but not with Cargo (though I understand if that’s
outside your purview/expertise), and whether I should feel comfortable
doing so? I want to build for non-Linux platforms, too, eventually.

I’m on rustc 1.47.0 (18bf6b4f0 2020-10-07) on Debian-like Linux.

Thanks much!

@Thomasdezeeuw
Copy link
Collaborator

This was done in #120, maybe @faern can elaborate why it's needed a bit.

@faern
Copy link
Contributor

faern commented Nov 13, 2020

It is very strange that you get this on rustc 1.47.0. According to the libc build script it should automatically activate libc_align when you compile it on rustc 1.25 or newer: https://github.com/rust-lang/libc/blob/master/build.rs#L56-L59

And also having socket2 activate the feature should also make the need for the __align field go away.

I have never used Bazel. Maybe it does not support features or build scripts enough?

@faern
Copy link
Contributor

faern commented Nov 13, 2020

Or maybe it simply does not parse the build script output of cargo:rustc-cfg=libc_align!? That would make kind of sense since the line includes cargo:. But then again, would not almost everything break if it did not support those? :O

@wchargin
Copy link
Author

wchargin commented Nov 17, 2020

Ah! Thank you, thank you. Indeed, it can parse cfg=... outputs from
build.rs, but only if you specify gen_buildrs = true (by design, to
make it explicit where the build isn’t necessarily hermetic), and
I hadn’t done so for this crate. Silly of me; sorry for the trouble, and
thanks for the pointer. :-)

wchargin added a commit to tensorflow/tensorboard that referenced this issue Nov 17, 2020
Summary:
When setting up the transitive dep on `libc` in #4316, I had originally
passed `--cfg=libc_align`, because that fixed the build for me and on
CI, and I couldn’t figure out the right cross-platform fix. But @faern
[helpfully points out that we just need to run the build script][1],
which detects the OS and Rust compiler version to determine the
appropriate flags. Bazel `rules_rust` can handle this, if we just ask.

(I should have followed my own `DEVELOPMENT.md` suggestions!)

[1]: rust-lang/socket2#130 (comment)

Test Plan:
It still builds—`bazel build @raze__libc__0_2_80//:libc`—and the demo
server still works and accepts requests following #4318’s test plan.

wchargin-branch: rust-libc-gen-buildrs
wchargin-source: 4ddff8d0b822585a9e84c7a24c4577b97b9c5d6a
wchargin added a commit to tensorflow/tensorboard that referenced this issue Nov 17, 2020
Summary:
When setting up the transitive dep on `libc` in #4316, I had originally
passed `--cfg=libc_align`, because that fixed the build for me and on
CI, and I couldn’t figure out the right cross-platform fix. But @faern
[helpfully points out that we just need to run the build script][1],
which detects the OS and Rust compiler version to determine the
appropriate flags. Bazel `rules_rust` can handle this, if we just ask.

(I should have followed my own `DEVELOPMENT.md` suggestions!)

[1]: rust-lang/socket2#130 (comment)

Test Plan:
It still builds—`bazel build @raze__libc__0_2_80//:libc`—and the demo
server still works and accepts requests following #4318’s test plan.

wchargin-branch: rust-libc-gen-buildrs
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

No branches or pull requests

3 participants