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

std: Remove internal definitions of cfg_if! macro #61720

Merged
merged 1 commit into from
Jun 13, 2019

Conversation

alexcrichton
Copy link
Member

This is duplicated in a few locations throughout the sysroot to work
around issues with not exporting a macro in libstd but still wanting it
available to sysroot crates to define blocks. Nowadays though we can
simply depend on the cfg-if crate on crates.io, allowing us to use it
from there!

@rust-highfive
Copy link
Collaborator

r? @bluss

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 10, 2019
@@ -336,6 +336,9 @@ extern crate libc;
#[allow(unused_extern_crates)]
extern crate unwind;

#[macro_use]
extern crate cfg_if;
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that this is here because there's usage of a bare cfg_if! macro invocation in stdsimd. That needs to be updated to cfg_if::cfg_if! before this can be removed, but that can hopefully happen in parallel with this.

@alexcrichton
Copy link
Member Author

r? @sfackler

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_time:end:1792cf4d:start=1560180228390210893,finish=1560180229177715781,duration=787504888
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
$ export AWS_ACCESS_KEY_ID=AKIA46X5W6CZEJZ6XT55
---
travis_time:start:test_codegen
Check compiletest suite=codegen mode=codegen (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[01:02:59] 
[01:02:59] running 144 tests
[01:03:02] i..iii.....iii...iiii....i......................i..i.................i.....i..........ii.i..i..i.ii. 100/144
[01:03:04] test result: ok. 114 passed; 0 failed; 30 ignored; 0 measured; 0 filtered out
[01:03:04] 
[01:03:04]  finished in 4.505
[01:03:04] travis_fold:end:test_codegen
---
travis_time:start:test_assembly
Check compiletest suite=assembly mode=assembly (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[01:03:06] 
[01:03:06] running 9 tests
[01:03:06] iiiiiiiii
[01:03:06] 
[01:03:06]  finished in 0.147
[01:03:06] travis_fold:end:test_assembly

---
travis_time:start:test_debuginfo
Check compiletest suite=debuginfo mode=debuginfo-gdb+lldb (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[01:03:21] 
[01:03:21] running 122 tests
[01:03:46] .iiiii...i.....i..i...i..i.i.i..i.ii..i.i.....i..i....i..........iiii..........i...ii...i.......ii.i 100/122
[01:03:50] .i.i......iii.i.....ii
[01:03:50] 
[01:03:50]  finished in 28.999
[01:03:50] travis_fold:end:test_debuginfo

---
[01:19:38] 
[01:19:38] To learn more, run the command again with --verbose.
[01:19:38] 
[01:19:38] 
[01:19:38] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "test" "--target" "x86_64-unknown-linux-gnu" "-j" "4" "--release" "--locked" "--color" "always" "--features" "panic-unwind backtrace compiler-builtins-c" "--manifest-path" "/checkout/src/libstd/Cargo.toml" "-p" "std" "--" "--quiet"
[01:19:38] 
[01:19:38] 
[01:19:38] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:19:38] Build completed unsuccessfully in 1:15:43
---
166968 ./obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps
156504 ./src/llvm-project/clang
150572 ./obj/build/bootstrap/debug/incremental
135300 ./obj/build/bootstrap/debug/incremental/bootstrap-oxgzobynhmm1
135296 ./obj/build/bootstrap/debug/incremental/bootstrap-oxgzobynhmm1/s-fd1cypvdk7-ynen56-19ca4iajxr4qm
125304 ./obj/build/x86_64-unknown-linux-gnu/test/mir-opt
123656 ./src/llvm-project/llvm/test/CodeGen
117304 ./obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib
117248 ./obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

This is duplicated in a few locations throughout the sysroot to work
around issues with not exporting a macro in libstd but still wanting it
available to sysroot crates to define blocks. Nowadays though we can
simply depend on the `cfg-if` crate on crates.io, allowing us to use it
from there!
@sfackler
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jun 11, 2019

📌 Commit 8eb7f36 has been approved by sfackler

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 11, 2019
Centril added a commit to Centril/rust that referenced this pull request Jun 11, 2019
…fackler

std: Remove internal definitions of `cfg_if!` macro

This is duplicated in a few locations throughout the sysroot to work
around issues with not exporting a macro in libstd but still wanting it
available to sysroot crates to define blocks. Nowadays though we can
simply depend on the `cfg-if` crate on crates.io, allowing us to use it
from there!
Centril added a commit to Centril/rust that referenced this pull request Jun 11, 2019
…fackler

std: Remove internal definitions of `cfg_if!` macro

This is duplicated in a few locations throughout the sysroot to work
around issues with not exporting a macro in libstd but still wanting it
available to sysroot crates to define blocks. Nowadays though we can
simply depend on the `cfg-if` crate on crates.io, allowing us to use it
from there!
Centril added a commit to Centril/rust that referenced this pull request Jun 13, 2019
…fackler

std: Remove internal definitions of `cfg_if!` macro

This is duplicated in a few locations throughout the sysroot to work
around issues with not exporting a macro in libstd but still wanting it
available to sysroot crates to define blocks. Nowadays though we can
simply depend on the `cfg-if` crate on crates.io, allowing us to use it
from there!
bors added a commit that referenced this pull request Jun 13, 2019
Rollup of 5 pull requests

Successful merges:

 - #61598 (Handle index out of bound errors during const eval without panic)
 - #61720 (std: Remove internal definitions of `cfg_if!` macro)
 - #61757 (Deprecate ONCE_INIT in future 1.38 release)
 - #61766 (submodules: update clippy from c0dbd34 to bd33a97)
 - #61791 (Small cleanup in `check_pat_path`)

Failed merges:

r? @ghost
@bors bors merged commit 8eb7f36 into rust-lang:master Jun 13, 2019
@RalfJung
Copy link
Member

@alexcrichton any idea why this might break building libstd with xargo?

    Updating crates.io index
   Compiling core v0.0.0 (/home/travis/.rustup/toolchains/master/lib/rustlib/src/rust/src/libcore)
   Compiling compiler_builtins v0.1.16
   Compiling cc v1.0.37
   Compiling libc v0.2.58
   Compiling build_helper v0.1.0 (/home/travis/.rustup/toolchains/master/lib/rustlib/src/rust/src/build_helper)
   Compiling unwind v0.0.0 (/home/travis/.rustup/toolchains/master/lib/rustlib/src/rust/src/libunwind)
   Compiling cfg-if v0.1.9
error[E0463]: can't find crate for `core`
error: aborting due to previous error
For more information about this error, try `rustc --explain E0463`.
error: Could not compile `cfg-if`.
warning: build failed, waiting for other jobs to finish...
error: build failed
error: `"cargo" "build" "--release" "--manifest-path" "/tmp/xargo.EVc4fCLCAwza/Cargo.toml" "--target" "x86_64-unknown-linux-gnu" "-p" "std"` failed with exit code: Some(101)

@@ -15,6 +15,7 @@ crate-type = ["dylib", "rlib"]

[dependencies]
alloc = { path = "../liballoc" }
cfg-if = "0.1.8"
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this enable the rustc-dep-of-std feature?

Copy link
Member

Choose a reason for hiding this comment

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

Hm, that leads to a build failure?!?

error: failed to select a version for `cfg-if`.
    ... required by package `std v0.0.0 (/home/r/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd)`
    ... which is depended on by `sysroot v0.0.0 (/tmp/xargo.NWDhLbN6RoeP)`
versions that meet the requirements `^0.1.9` are: 0.1.9

the package `std` depends on `cfg-if`, with features: `rustc-dep-of-std` but `cfg-if` does not have these features.


failed to select a version for `cfg-if` which could resolve this conflict

But the feature is present since rust-lang/cfg-if@3e31141.

Copy link
Member

Choose a reason for hiding this comment

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

Looks like the crates.io version differs from the GH version in that parts of the toml are commented out...??

# [dependencies]
# core = { version = "1.0.0", optional = true, package = 'rustc-std-workspace-core' }
# compiler_builtins = { version = '0.1.2', optional = true }
#
# [features]
# rustc-dep-of-std = ['core', 'compiler_builtins']

Copy link
Member

Choose a reason for hiding this comment

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

Reported as rust-lang/cfg-if#22.

Copy link
Member

@RalfJung RalfJung Jun 14, 2019

Choose a reason for hiding this comment

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

And submitted hopefully fixing PR as #61828.

bors added a commit that referenced this pull request Jun 15, 2019
make sure we use cfg-if as a std dependency

xargo currently fails to build libstd because this feature is missing. My guess is that it works in rustc because the feature is enabled elsewhere, but that does not help for a libstd-only build.

Miri is currently in a state where it is shipped but broken, which makes CI fail for projects that are tested in Miri. So this is kind of urgent.

Cc @alexcrichton  #61720
@alexcrichton alexcrichton deleted the libstd-cfg-if-dep branch June 21, 2019 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants