-
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
build: improve script and travis config #4675
Conversation
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.
I'm really no bash expert, so some questions might seem silly. Most of the changes LGTM.
Oh and thanks for all the work on the build system lately! ❤️ |
aa256c2
to
1ce44aa
Compare
|
||
# Make sure this isn't set - clippy-driver should cope without it | ||
unset CARGO_MANIFEST_DIR | ||
|
||
# Run a lint and make sure it produces the expected output. It's also expected to exit with code 1 | ||
# XXX How to match the clippy invocation in compile-test.rs? | ||
! ./target/debug/clippy-driver -Dwarnings -Aunused -Zui-testing --emit metadata --crate-type bin tests/ui/cstring.rs 2> cstring.stderr |
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.
I don't understand what this command does. Could someone explain it to me?
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 whole block checks if clippy-driver
can be run without cargo.
This command is just a test, if the output of clippy-driver
is the same as the output of the ui-test. Why cstring.rs
was chosen for this test, I have no idea.
I guess all these flags are there to simulate the behavior of compiletest-rs
.
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.
The PR that added this has some more context. Maybe include the PR link in the comment?
#3665
1ce44aa
to
6b3ba64
Compare
27e88b6
to
16c2681
Compare
Ready for review! |
☔ The latest upstream changes (presumably #4659) made this pull request unmergeable. Please resolve the merge conflicts. |
d4f2bba
to
33cf02c
Compare
33cf02c
to
e6cb66a
Compare
☔ The latest upstream changes (presumably #4702) made this pull request unmergeable. Please resolve the merge conflicts. |
osx ) | ||
# See <https://github.com/nteract/nteract/issues/1523#issuecomment-301623519> | ||
sudo mkdir -p /usr/local/lib | ||
sudo find "$SYSROOT/lib" -maxdepth 1 -name '*.dylib' -exec ln -s {} /usr/local/lib \; | ||
;; |
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 wasn't necessary before. What changed?
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.
Honestly, I don't know. I tried to set LD_LIBRARY_PATH, LIBRARY_PATH, DYLD_LIBRARY_PATH. All of that still made this PR failed.
f2ffc3d
to
44c3c9c
Compare
* fix a diff failure on windows See https://travis-ci.com/rust-lang/rust-clippy/jobs/245971932#L1625 for an example. * use cmp instead of diff > /dev/null * clone single branch instead of clone then checking out * do not decrypt key if have no diff change
Build succeed. I wanna more eyes and thoughts on #4675 (comment). |
8fdbbcd
to
312c761
Compare
Now the travis CI build time is fast enough to not cause build timeout.
312c761
to
088d528
Compare
@bors try |
build: improve script and travis config * fix a diff failure on windows See https://travis-ci.com/rust-lang/rust-clippy/jobs/245971932#L1625 for an example. * use cmp instead of diff > /dev/null * clone single branch instead of clone then checking out * do not decrypt key if have no diff change changelog: none
Note that in the first try, the cache will be invalidated because of removing BASE_TESTS env. |
☀️ Try build successful - checks-travis, status-appveyor |
@bors try |
build: improve script and travis config * fix a diff failure on windows See https://travis-ci.com/rust-lang/rust-clippy/jobs/245971932#L1625 for an example. * use cmp instead of diff > /dev/null * clone single branch instead of clone then checking out * do not decrypt key if have no diff change changelog: none
☀️ Try build successful - checks-travis, status-appveyor |
Changes look good to me. I can't say anything about the dylib stuff, but it seems to work fine 🤷♂️ Thanks for all your build improvements! @bors r+ |
📌 Commit 088d528 has been approved by |
build: improve script and travis config * fix a diff failure on windows See https://travis-ci.com/rust-lang/rust-clippy/jobs/245971932#L1625 for an example. * use cmp instead of diff > /dev/null * clone single branch instead of clone then checking out * do not decrypt key if have no diff change changelog: none
☀️ Test successful - checks-travis, status-appveyor |
See https://travis-ci.com/rust-lang/rust-clippy/jobs/245971932#L1625
for an example.
use cmp instead of diff > /dev/null
clone single branch instead of clone then checking out
do not decrypt key if have no diff change
changelog: none