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

using the DepInfo backend still uses -Z save-analysis #106

Closed
pvdrz opened this issue Nov 23, 2021 · 5 comments · Fixed by #107
Closed

using the DepInfo backend still uses -Z save-analysis #106

pvdrz opened this issue Nov 23, 2021 · 5 comments · Fixed by #107

Comments

@pvdrz
Copy link
Contributor

pvdrz commented Nov 23, 2021

Hi, I'm not sure if this is intended or not but I found an upstream bug (binary-dep-depinfo) that causes an ICE on rustc with the -Z save-analysis flag on and by extension while calling cargo +nightly udeps: rust-lang/rust#88536.

I know that there is the --backend depinfo flag but it seems that the command executed by the compiler still includes the save-anaysis flag. Is this the intended behavior?

@est31
Copy link
Owner

est31 commented Nov 23, 2021

I'm not entirely sure if this flag should stay. There might be some caching implications. That is, if you don't pass the flag, run cargo udeps, and then pass it, cargo might think it already ran that command and thus not execute it. But I'm not entirely sure. It can be figured out experimentally, and I can't think of any other issue than that one.

@pvdrz
Copy link
Contributor Author

pvdrz commented Nov 23, 2021

I have a local copy of the repo removing the flag when the depinfo backend is being used. I'll test if your caching issues happen or not.

@pvdrz
Copy link
Contributor Author

pvdrz commented Nov 23, 2021

I did this experiment:

  • Run cargo +nightly udeps --backend=depinfo in a crate without dependencies.
  • Add a dependency to the manifest and don't use it.
  • Run cargo +nightly udeps --backend=depinfo again.

The unused dependency is detected in the second run. Is that OK or do you think I should try something more extensive?

@est31
Copy link
Owner

est31 commented Nov 23, 2021

Is that OK or do you think I should try something more extensive?

My concern was more what if you did, after removing the target dir, run cargo +nightly udeps --backend=depinfo first and then ran cargo +nightly udeps. Does it then still warn about an unused dependency that you added before removing the target dir?

@pvdrz
Copy link
Contributor Author

pvdrz commented Nov 23, 2021

it still warns me in both runs but the first run does the checks from scratch ( as expected ) and then the second run reuses the json.

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 a pull request may close this issue.

2 participants