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.

Sign up for GitHub

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

Jump to bottom

Don't try to determine sysroot. rustc_driver will use default value. #3257

Merged
merged 4 commits into from
Dec 6, 2018
Merged

Don't try to determine sysroot. rustc_driver will use default value. #3257

merged 4 commits into from
Dec 6, 2018

Conversation

o01eg
Copy link
Contributor

@o01eg o01eg commented Oct 4, 2018

Fix #2874

@oli-obk
Copy link
Contributor

oli-obk commented Oct 4, 2018

Windows is now failing with error[E0463]: can't find crate for std

@o01eg
Copy link
Contributor Author

o01eg commented Oct 4, 2018

Looks like it was necessary in windows.

@oli-obk
Copy link
Contributor

oli-obk commented Oct 4, 2018

It might not be necessary when installed via rustup. I'm not sure how to properly test that. Maybe we should copy the binary to .rustup/toolchain/whatev/bin instead of .cargo/bin and see how that goes

@o01eg o01eg mentioned this pull request Oct 4, 2018
Merged
@djc
Copy link
Contributor

djc commented Nov 14, 2018

So how do we make progress on this? It'd be nice to get this in.

@oli-obk
Copy link
Contributor

oli-obk commented Nov 16, 2018

Maybe we should copy the binary to .rustup/toolchain/whatev/bin instead of .cargo/bin and see how that goes

@o01eg can you try this?

@o01eg
Copy link
Contributor Author

o01eg commented Nov 16, 2018

@oli-obk Is it somewhere in the code?

@oli-obk
Copy link
Contributor

oli-obk commented Nov 16, 2018

Oh sorry. I completely misunderstood the issue. I assumed this was dogfood failing, but it's just our normal test suite.

You need to re-add the sysroot detection code that you removed, but only into our test suite, so it won't be in the final binary. I think it should suffice to have a check for windows and then run the sysroot detection right here and then append it to the target_rustcflags

@oli-obk
Copy link
Contributor

oli-obk commented Nov 16, 2018

You also don't need all the previous code, just the call to rustc: https://github.com/rust-lang-nursery/rust-clippy/pull/3257/files#diff-80f8b83dd8e9ced9366c4eabaf0bba64L42

@o01eg
Copy link
Contributor Author

o01eg commented Nov 22, 2018

Something odd broke again.

@oli-obk
Copy link
Contributor

oli-obk commented Nov 22, 2018

we seem to have lost appveyor, too XD

@hcpl
Copy link

hcpl commented Nov 22, 2018

@o01eg likely related to #3444 as both BASE_TESTS jobs failed due to dogfood tests.

@o01eg
Copy link
Contributor Author

o01eg commented Dec 6, 2018

Rebased after #3444 merged

@o01eg
Copy link
Contributor Author

o01eg commented Dec 6, 2018

Looks like it still failed. Should I add sysroot detection to dogfood tests as well?

@oli-obk
Copy link
Contributor

oli-obk commented Dec 6, 2018

Yea, that seems necessary sadly.

@phansch phansch added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Dec 6, 2018
@o01eg
Copy link
Contributor Author

o01eg commented Dec 6, 2018

I suppose it better to merge rustc_sysroot_paths but not sure where to place it.

oli-obk
oli-obk approved these changes Dec 6, 2018
View reviewed changes
Copy link
Contributor

@oli-obk oli-obk left a comment

Choose a reason for hiding this comment

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

Once we get appveyor back, r=me

@o01eg
Copy link
Contributor Author

o01eg commented Dec 6, 2018

I've launched AppVeyor by hand, it works: https://ci.appveyor.com/project/o01eg/rust-clippy/builds/20814346

@djc
Copy link
Contributor

djc commented Dec 6, 2018

@o01eg do you have a version of the patch that applies to 1.31.0?

@o01eg
Copy link
Contributor Author

o01eg commented Dec 6, 2018

I have it only for src/driver.rs because it need to be built on the Gentoo:
1.31.0-clippy-sysroot.patch.gz

@phansch phansch merged commit 041c49c into rust-lang:master Dec 6, 2018
@o01eg o01eg deleted the remove-sysroot branch December 6, 2018 21:21
@sinkuu sinkuu mentioned this pull request Dec 10, 2018
Closed
matthiaskrgr added a commit to matthiaskrgr/rust-clippy that referenced this pull request Dec 11, 2018
@matthiaskrgr
Revert "Merge pull request rust-lang#3257 from o01eg/remove-sysroot"
2375c31 
This reverts commit 041c49c, reversing
changes made to 278b94e.
matthiaskrgr added a commit to matthiaskrgr/rust-clippy that referenced this pull request Dec 11, 2018
@matthiaskrgr
Revert "Merge pull request rust-lang#3257 from o01eg/remove-sysroot"
77efc91 
This reverts commit 041c49c, reversing
changes made to 1df5766.

This broke clippy working when installed from the git repo via cargo install.

Fixes rust-lang#3523
Reopens rust-lang#2874
matthiaskrgr added a commit to matthiaskrgr/rust-clippy that referenced this pull request Dec 14, 2018
@matthiaskrgr
Revert "Merge pull request rust-lang#3257 from o01eg/remove-sysroot"
1d88229 
This reverts commit 041c49c, reversing
changes made to 1df5766.
matthiaskrgr added a commit to matthiaskrgr/rust-clippy that referenced this pull request Dec 14, 2018
@matthiaskrgr
Revert "Merge pull request rust-lang#3257 from o01eg/remove-sysroot"
a9509eb 
This reverts commit 041c49c, reversing
changes made to 1df5766.
bors added a commit that referenced this pull request Dec 19, 2018
@bors
Auto merge of #3546 - matthiaskrgr:fix_install, r=oli-obk
980bcd8 
Revert "Merge pull request #3257 from o01eg/remove-sysroot"

This reverts commit 041c49c, reversing
changes made to 1df5766.

The PR broke running a cargo-install'd clippy.
The installed clippy would not be able to find a crate for std.

Fixes #3523
Reopens #2874
o01eg added a commit to o01eg/gentoo-rust that referenced this pull request Dec 28, 2018
@o01eg
Fix clippy sysroot patch. Fix codegen search.
4db7d7f 
See rust-lang/rust#57101
and rust-lang/rust-clippy#3257
@o01eg o01eg mentioned this pull request Dec 28, 2018
Merged
@ghost ghost mentioned this pull request Dec 29, 2018
Merged
bors added a commit that referenced this pull request Dec 30, 2018
@bors
Auto merge of #3593 - mikerite:readme-syspath-2, r=phansch
61bfb39 
Update README local run command to remove syspath

Since #3257 was reverted, including the sysroot in RUSTFLAGS gives the
error `Option 'sysroot' given more than once`
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request May 5, 2020
@matthiaskrgr
submodules: update clippy from a416c5e to fc24fce
aca8f94 
Fixes clippy tool state

Changes:
````
FIXME > TODO
rustup rust-lang/rust#56992
Document map_clone known problems rust-lang#498
Remove header link
test: panic at map_unit_fn.rs:202 for map() without args
rm unused file map_unit_fn.stderr
panic at map_unit_fn.rs:202 for map() without args
Change contrib.md hierarchy, link to it from readme
Workaround rust-lang/rust#43081
Teach `suspicious_else_formatting` about `if .. {..} {..}`
Link to `rustc_driver` crate in plugin
mutex_atomic: Correct location of AtomicBool and friends
Update README local run command to specify syspath
Do not mark as_ref as useless if it's followed by a method call
Changes lint sugg to bitwise and operator `&`
Run update_lints after renaming
Rename lint to MODULE_NAME_REPETITIONS
Add renaming tests
Move renaming to the right place
Implements lint for order comparisons against bool
fix(module_name_repeat): Try to register renamed lint, not valid yet
Fix an endless loop in the tests.
Fix `implicit_return` false positives.
chore(moduel_name_repeat): Rename stutter lint to module_name_repeat to avoid ableist language
Make integration tests fail on 'E0463'
base tests: make sure cargo-clippy binary can be called directly
Revert "Merge pull request rust-lang#3257 from o01eg/remove-sysroot"
````
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oli-obk oli-obk oli-obk approved these changes

Assignees
No one assigned
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@o01eg @oli-obk @djc @hcpl @phansch