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

Fix #180: Handle workspaces more similar to cargo build/run/test #182

Merged
merged 1 commit into from
Jan 1, 2022

Conversation

ede1998
Copy link
Contributor

@ede1998 ede1998 commented Dec 24, 2021

About

This PR fixes the reproducible issues mentioned in #180. Namely this is the different behavior between standard cargo commands and cargo flamegraph when using workspaces. The default cargo commands by default only take into account crates in the working directory while cargo flamegraph was working on the entire working space.

The PR tries to amend that by changing cargo flamegraph behavior in the following ways:

  • cargo flamegraph chooses a target only from the current working directory unless --package or --manifest-path are used (instead of the entire workspace).
  • cargo flamegraph suggests targets only from the current working directory unless --package or --manifest-path are used (instead of the entire workspace).

I tested the new version with several different dummy crates and workspaces as well as ripgrep. It seems to work fine and just like the standard cargo commands.

Breaking changes (Not sure if this is relevant for anyone though.)

As described above, cargo flamegraph no longer works on an entire workspace but only on the crate in the current working directory. This means that calling cargo flamegraph in a crate within a workspace tries to execute the binary within this crate instead of the workspace binary.

Example: calling cargo flamegraph in ripgrep/crates/printer (a library crate) now errors because it cannot find a binary instead of executing rg (the workspace executable).

Relevant issues

Copy link
Contributor

@djc djc left a comment

Choose a reason for hiding this comment

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

This mostly looks good to me, thanks for improving the situation here!

…/run/test`

use the crate in the current working directory unless otherwise
specified by --manifest-path or --package
@ede1998 ede1998 force-pushed the handle-workspaces-like-cargo branch from 265a11b to 760ca0f Compare January 1, 2022 19:31
@djc djc merged commit fc5f6bb into flamegraph-rs:main Jan 1, 2022
@djc
Copy link
Contributor

djc commented Jan 1, 2022

Awesome, thanks!

@ede1998 ede1998 deleted the handle-workspaces-like-cargo branch January 2, 2022 12:43
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.

2 participants