-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
appveyor: Use VS2017 for all our images #55935
Conversation
This was [recommended by AppVeyor][1] to see if it has any impact on our build times, hopefully on the beneficial side of things! This shouldn't affect our binary compatibility for generated compilers like it would normally do for Linux. [1]: https://help.appveyor.com/discussions/questions/29832-did-recent-changes-apply-to-possibly-slow-down-builds#comment_46484879
r? @aturon (rust_highfive has picked a reviewer for you, use r? to override) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r=me with the digest restored
@@ -1,5 +1,8 @@ | |||
environment: | |||
SCCACHE_DIGEST: f808afabb4a4eb1d7112bcb3fa6be03b61e93412890c88e177c667eb37f46353d7ec294e559b16f9f4b5e894f2185fe7670a0df15fd064889ecbd80f0c34166c |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like an unintentional change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh sorry forgot to comment on this but it's intentional, this is long since not used at all
@bors: r=Mark-Simulacrum |
📌 Commit 008e5dc has been approved by |
appveyor: Use VS2017 for all our images This was [recommended by AppVeyor][1] to see if it has any impact on our build times, hopefully on the beneficial side of things! This shouldn't affect our binary compatibility for generated compilers like it would normally do for Linux. [1]: https://help.appveyor.com/discussions/questions/29832-did-recent-changes-apply-to-possibly-slow-down-builds#comment_46484879
Rollup of 16 pull requests Successful merges: - #54906 (Reattach all grandchildren when constructing specialization graph.) - #55182 (Redox: Update to new changes) - #55211 (Add BufWriter::buffer method) - #55507 (Add link to std::mem::size_of to size_of intrinsic documentation) - #55530 (Speed up String::from_utf16) - #55556 (Use `Mmap` to open the rmeta file.) - #55622 (NetBSD: link libstd with librt in addition to libpthread) - #55827 (A few tweaks to iterations/collecting) - #55901 (fix various typos in doc comments) - #55926 (Change sidebar selector to fix compatibility with docs.rs) - #55930 (A handful of hir tweaks) - #55932 (core/char: Speed up `to_digit()` for `radix <= 10`) - #55935 (appveyor: Use VS2017 for all our images) - #55936 (save-analysis: be even more aggressive about ignorning macro-generated defs) - #55948 (submodules: update clippy from d8b4269 to 7e0ddef) - #55956 (add tests for some fixed ICEs)
I suspect this PR caused the strange "Command exited with code 259" in https://ci.appveyor.com/project/rust-lang/rust/builds/20316715/job/kx57v0vi9ginm1po ( |
Hm that does seem suspicious, let's see if that happens again when this hits bors "for real" (non-rollup) |
appveyor: Use VS2017 for all our images This was [recommended by AppVeyor][1] to see if it has any impact on our build times, hopefully on the beneficial side of things! This shouldn't affect our binary compatibility for generated compilers like it would normally do for Linux. [1]: https://help.appveyor.com/discussions/questions/29832-did-recent-changes-apply-to-possibly-slow-down-builds#comment_46484879
Rollup of 25 pull requests Successful merges: - #55562 (Add powerpc- and powerpc64-unknown-linux-musl targets) - #55564 (test/linkage-visibility: Ignore on musl targets) - #55827 (A few tweaks to iterations/collecting) - #55834 (Forward the ABI of the non-zero sized fields of an union if they have the same ABI) - #55857 (remove unused dependency) - #55862 (in which the E0618 "expected function" diagnostic gets a makeover) - #55867 (do not panic just because cargo failed) - #55894 (miri enum discriminant handling: Fix treatment of pointers, better error when it is undef) - #55916 (Make miri value visitor usfeful for mutation) - #55919 (core/tests/num: Simplify `test_int_from_str_overflow()` test code) - #55923 (reword #[test] attribute error on fn items) - #55935 (appveyor: Use VS2017 for all our images) - #55949 (ty: return impl Iterator from Predicate::walk_tys) - #55952 (Update to Clang 7 on CI.) - #55953 (#53488 Refactoring UpvarId) - #55962 (rustdoc: properly calculate spans for intra-doc link resolution errors) - #55963 (Stress test for MPSC) - #55968 (Clean up some non-mod-rs stuff.) - #55970 (Miri backtrace improvements) - #56007 (CTFE: dynamically make sure we do not call non-const-fn) - #56011 (Replace data.clone() by Arc::clone(&data) in mutex doc.) - #56012 (avoid shared ref in UnsafeCell::get) - #56016 (Add VecDeque::resize_with) - #56027 (docs: Add missing backtick in object_safety.rs docs) - #56043 (remove "approx env bounds" if we already know from trait) Failed merges: r? @ghost
appveyor: Use VS2017 for all our images This was [recommended by AppVeyor][1] to see if it has any impact on our build times, hopefully on the beneficial side of things! This shouldn't affect our binary compatibility for generated compilers like it would normally do for Linux. [1]: https://help.appveyor.com/discussions/questions/29832-did-recent-changes-apply-to-possibly-slow-down-builds#comment_46484879
☀️ Test successful - status-appveyor, status-travis |
I think we should revert this PR, this is likely causing the spurious error in #55906 (comment) |
@kennytm definitely feel free, I'll investigate this later on my own time. I won't have a chance for a but to post the revert myself, but I'll get to it if it's not already up by the time I'm back on! |
Submitted #56201 :) |
Revert "appveyor: Use VS2017 for all our images" This reverts commit 008e5dc (#55935) We suspect this causes the spurious failure in #55906 (comment) and #55915 (comment). r? @alexcrichton
Originally added in rust-lang#55935 to test build times, this was reverted in rust-lang#56201 due to a belief that it caused the exit code 259 spurious errors. We've since learned, however, that the 259 exit code is likely not related to this image update as we're getting it in a number of locations now. VS2017 looks like it may be required to compile LLVm in the near future, notably discovered by rust-lang#58408 where we attempted to update LLVM.
…-Simulacrum appveyor: Use VS2017 for all our images Originally added in rust-lang#55935 to test build times, this was reverted in rust-lang#56201 due to a belief that it caused the exit code 259 spurious errors. We've since learned, however, that the 259 exit code is likely not related to this image update as we're getting it in a number of locations now. VS2017 looks like it may be required to compile LLVm in the near future, notably discovered by rust-lang#58408 where we attempted to update LLVM.
…-Simulacrum appveyor: Use VS2017 for all our images Originally added in rust-lang#55935 to test build times, this was reverted in rust-lang#56201 due to a belief that it caused the exit code 259 spurious errors. We've since learned, however, that the 259 exit code is likely not related to this image update as we're getting it in a number of locations now. VS2017 looks like it may be required to compile LLVm in the near future, notably discovered by rust-lang#58408 where we attempted to update LLVM.
…-Simulacrum appveyor: Use VS2017 for all our images Originally added in rust-lang#55935 to test build times, this was reverted in rust-lang#56201 due to a belief that it caused the exit code 259 spurious errors. We've since learned, however, that the 259 exit code is likely not related to this image update as we're getting it in a number of locations now. VS2017 looks like it may be required to compile LLVm in the near future, notably discovered by rust-lang#58408 where we attempted to update LLVM.
…-Simulacrum appveyor: Use VS2017 for all our images Originally added in rust-lang#55935 to test build times, this was reverted in rust-lang#56201 due to a belief that it caused the exit code 259 spurious errors. We've since learned, however, that the 259 exit code is likely not related to this image update as we're getting it in a number of locations now. VS2017 looks like it may be required to compile LLVm in the near future, notably discovered by rust-lang#58408 where we attempted to update LLVM.
This was recommended by AppVeyor to see if it has any impact on our
build times, hopefully on the beneficial side of things! This shouldn't
affect our binary compatibility for generated compilers like it would
normally do for Linux.