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

Add the TARGET_LIBS environment variable for rustc CI testing #4786

Merged
merged 2 commits into from
Nov 8, 2019

Conversation

msizanoen1
Copy link
Contributor

@msizanoen1 msizanoen1 commented Nov 7, 2019

Needed to fix the test failure in rust-lang/rust#66158.

See rust-lang/rust#66158 (comment)

r? @Manishearth

changelog: none

@tesuji
Copy link
Contributor

tesuji commented Nov 7, 2019

Ping @flip1995
@msizanoen1 you might need to rebase against upstream master.

@msizanoen1 msizanoen1 force-pushed the target-libs branch 2 times, most recently from 12b46b9 to ac4e589 Compare November 7, 2019 14:34
tests/compile-test.rs Outdated Show resolved Hide resolved
tests/compile-test.rs Outdated Show resolved Hide resolved
@msizanoen1 msizanoen1 force-pushed the target-libs branch 2 times, most recently from 59b43e8 to fa8d9db Compare November 7, 2019 14:48
@tesuji
Copy link
Contributor

tesuji commented Nov 7, 2019

build failed because of format check. Could you run ./util/dev fmt?

@Manishearth
Copy link
Member

Do you think this will work when cross compiling? I'm strongly inclined to just disable these tests when running rustc ci.

@mati865
Copy link
Contributor

mati865 commented Nov 7, 2019

Tools tests aren't ran when cross compiling right now.
In fact they are ran only on x86_64-gnu-tools and x86_64-msvc-tools which are native but user could enable them for local build.

@Manishearth
Copy link
Member

Oh, right!

Anyway, I found a better fix for this in #4784, we really shouldn't be linking to clippy_lints in those tests anyway.

@msizanoen1
Copy link
Contributor Author

Actually even with that PR you will still get the regex and serde crate not found error.

@msizanoen1
Copy link
Contributor Author

About the tools are tested only when building natively assumption, the rustbuild build system always pass the --target flags so the target rlibs are put in a separate directory.

@msizanoen1
Copy link
Contributor Author

msizanoen1 commented Nov 8, 2019

Cross testing is not a goal of this PR.

@@ -27,6 +27,11 @@ fn host_libs() -> PathBuf {
}
}

#[must_use]
Copy link
Member

Choose a reason for hiding this comment

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

the must_use isn't necessary

@Manishearth
Copy link
Member

@Brors r+

will land for now and maybe make minor fixes later

@msizanoen1
Copy link
Contributor Author

msizanoen1 commented Nov 8, 2019

@Brors r+

Who's @Brors? You probably mean @bors

@Manishearth
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Nov 8, 2019

📌 Commit 7d2e813 has been approved by Manishearth

@bors
Copy link
Contributor

bors commented Nov 8, 2019

⌛ Testing commit 7d2e813 with merge af3090c...

bors added a commit that referenced this pull request Nov 8, 2019
Add the TARGET_LIBS environment variable for rustc CI testing

Needed to fix the test failure in rust-lang/rust#66158.

See rust-lang/rust#66158 (comment)

r? @Manishearth
@bors
Copy link
Contributor

bors commented Nov 8, 2019

💔 Test failed - checks-travis

@flip1995
Copy link
Member

flip1995 commented Nov 8, 2019

@bors retry

Changelog was missing

bors added a commit that referenced this pull request Nov 8, 2019
Add the TARGET_LIBS environment variable for rustc CI testing

Needed to fix the test failure in rust-lang/rust#66158.

See rust-lang/rust#66158 (comment)

r? @Manishearth

changelog: none
@bors
Copy link
Contributor

bors commented Nov 8, 2019

⌛ Testing commit 7d2e813 with merge 692b260...

@bors
Copy link
Contributor

bors commented Nov 8, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: Manishearth
Pushing 692b260 to master...

@bors bors merged commit 7d2e813 into rust-lang:master Nov 8, 2019
@msizanoen1 msizanoen1 deleted the target-libs branch November 9, 2019 04:21
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

Successfully merging this pull request may close these issues.

6 participants