-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Run travis on rustc-master instead of nightly #2941
Conversation
appveyor.yml
Outdated
@@ -17,6 +17,11 @@ install: | |||
- rustup-init.exe -y --default-host %TARGET% --default-toolchain nightly | |||
- set PATH=%PATH%;C:\Users\appveyor\.cargo\bin;C:\Users\appveyor\.rustup\toolchains\nightly-%TARGET%\bin | |||
- if defined MSYS2_BITS set PATH=%PATH%;C:\msys64\mingw%MSYS2_BITS%\bin | |||
- cargo install rustup-toolchain-install-master || echo "rustup-toolchain-install-master already installed" | |||
- set RUSTC_HEAD=`git ls-remote https://github.com/rust-lang/rust.git HEAD | tr -d ",." | tr " \t" "\n" | grep -e "HEAD" -v` |
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.
let's use rustup-toolchain-install-master -n master -f
from kennytm/rustup-toolchain-install-master#6 when it merges
though I'm okay with merging this first and switching over later
hmm
|
Could Anyway AppVeyor error looks worse:
|
hmm... the macro imports aren't working. |
Probably means it's not using the toolchain -- maybe it's obeying the
rust-toolchain file?
…On Fri, Jul 20, 2018, 2:17 AM Oliver Schneider ***@***.***> wrote:
hmm... the macro imports aren't working.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#2941 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABivSL0_LHiZLSvXpMSp8pStZqWgdMh4ks5uIaApgaJpZM4VXpQR>
.
|
Rebased and updated to use new rustup-toolchain-install-master |
Primary build should succeed if you apply this workaround #3001 |
I re-triggered the build after #3001 was merged. The master installer still sometimes fails with a Forbidden from the GitHub API. Perhaps we are hitting some rate limit?
Looks like there are also different AppVeyor errors now:
and https://ci.appveyor.com/project/llogiq/rust-clippy/build/1.0.228#L265 |
Appveyor might be failing due to msvc vs mingw. Not sure if we are taking any care in choosing the correct thing |
@oli-obk |
So.. travis is failing due to
I'm not sure where the master-toolchain's libs are hidden, but we probably need to add it to the |
@oli-obk you are not installing toolchain components, see kennytm/rustup-toolchain-install-master#5 |
are you sure? the error looks like it's not finding the rustc libs, which rustc itself definitely needs (and rustc works) |
Please disregard my previous comment, I went too far with new component stuff. This repository needs to build Clippy everytime... To debug this you can add following commands:
It could be taking libs from the other toolchains. |
My "debug" build:
Can we dump sysroot path from |
The master-toolchain libs can be found at |
The appveyor linking error seems to be related with MinGW: alexcrichton/xz2-rs@eceb287 I'm not sure why, but without MinGW the linking error disappears. But after that the dogfood test fails appveyor. I got travis to work, except sometimes fetching master commit hash... thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error { kind: ClientError(Forbidden), url: Some("https://api.github.com/repos/rust-lang/rust/commits/master") } https://github.com/flip1995/rust-clippy/commits/prs Should I push this, to this branch? |
Windows build system seems really borked then.
|
GitHub will reject API requests from the CI since the IP is shared. You need to create a GitHub token. @mati865 See https://github.com/kennytm/rustup-toolchain-install-master/blob/master/.travis.yml for an example (the |
Thanks @kennytm! Just tested it on my fork and it works like a charm! We now need someone to create the token for Clippy and add it to travis and appveyor. |
After adding the If no |
@matthiaskrgr has found out the reason for this weird issue:
In other words: |
@mati865 we're already doing this in Travis and in appveyor. The appveyor error is somehow connected to the master toolchain. Even when you change nothing in the appveyor script, except the toolchain (from nightly to master) , this error appears. Also adding |
Oops my bad, guess it's the time to get some sleep. Looking at AppVeyor error again:
It just means the handle to the file is still open. On Windows files cannot be renamed/moved until very last handle is closed. |
Do we want to move forward with this? The next steps would be:
|
Just realized that I can add new environment variables on travis, too. I will give it a try now. I don't have access to AppVeyor, though. |
Yeah, just fix it for travis and land this, let me know what we need to do for appveyor and I'll do it. I don't know how to grant access to appveyor. |
Did a merge instead of a rebase because the For AppVeyor, the docs say that there's an 'Environment Variables' tab in the AppVeyor project settings. So essentially, you create a new Token with |
Done |
Feel free to merge when ready. |
Travis looks fine now, but I think AppVeyor is not detecting the |
Yeah not sure what's going on. Feel free to merge and open a new issue for
appveyor, at least we'll have _some_ reliable CI.
…On Sun, Sep 2, 2018, 3:27 PM Philipp Hansch ***@***.***> wrote:
Travis looks fine now, but I think AppVeyor is not detecting the
GITHUB_TOKEN correctly and doesn't install rust from master:
https://ci.appveyor.com/project/rust-lang-libs/rust-clippy/build/1.0.1536#L29
I guess it should show the output of the master installer?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#2941 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABivSLke9TYzsxKrNBlkBmdlJROq2DIdks5uW6uKgaJpZM4VXpQR>
.
|
It seems that no env var @Manishearth Is there maybe a typo in the env var name? Lower case, instead of upper case? 🤔 |
Oh, I added it to Manishearth/rust. I don't have access to rust-lang-nursery, but I pinged nrc to get it added. Relatedly: What should we do for PRs then? They don't have access to these env vars, right? Should we use a different method for getting the master hash? |
You could clone the rust-lang/rust repo and find the (full 40 digits) hash. I think |
Yeah, I really want to avoid that but it might be the only option |
Anyway, merging this since at least master gets good CI |
I'm going to add a CGI script for this on my server so we don't need the token. |
Yeah, shallow cloning rust is the other option but i thought it was terrible amd maybe slow so didn't pick it. But really both suck, i don't know which to pick. I guess cloning is nicer since it's less infra I have to maintain. |
`git ls-remote https://github.com/rust-lang/rust.git master` with some
parsing works.
I'll incorporate this into this PR (we can also make this part of
rustup-toolchain-install-master)
…On Mon, Sep 3, 2018, 8:08 AM kennytm ***@***.***> wrote:
You could clone the rust-lang/rust repo and find the (full 40 digits)
hash. I think git clone isn't rate limited by IP (otherwise these CI
services simply won't work). Cloning would be slower though.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#2941 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABivSOYeaKIajCbFZIOurevBNFzEVFWaks5uXJYogaJpZM4VXpQR>
.
|
No description provided.