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

Automatically find workspaces of all local dependencies and watch them. #216

Merged
merged 1 commit into from
Dec 29, 2022

Conversation

Blei
Copy link
Contributor

@Blei Blei commented Nov 13, 2022

This watches the workspace roots of all dependencies which are local, plus the workspace root of the current project. This has no effect if --watch/-w is manually specified.

Fixes #117.

It's very possible I'm missing some nuances though! Maybe this breaks in certain constellations of where cargo is invoked? Please let me know if I'm missing something.

@passcod passcod changed the base branch from main to 8.x November 13, 2022 21:05
@passcod passcod closed this Nov 13, 2022
@passcod passcod reopened this Nov 13, 2022
@passcod
Copy link
Member

passcod commented Nov 13, 2022

Closed/reopened to trigger the builds after I changed the base branch.

@passcod
Copy link
Member

passcod commented Nov 14, 2022

Okay, that's looking good, even if it's somewhat different from #117 (but probably covers the original usecase).

If you could add this behaviour to the readme/usage/manpage as needed, I'll merge and release this asap!

@Blei
Copy link
Contributor Author

Blei commented Nov 18, 2022

I've pushed a new version of my change. Changes since last time:

  • I've added a flag to disable the new behaviour
  • I've updated some help output
  • I've updated the README file (by dumping cargo help output) and the man file.

You mention this change is not what was discussed in the feature request. What do you think is different? I'm happy to change the PR into a form you think is useful.

@passcod
Copy link
Member

passcod commented Nov 19, 2022

Yeah, actually, upon further reflection, this PR changes the current behaviour quite a bit.

Say we have the following structure:

projectA/
  Cargo.toml -- workspace manifest
  pkg1/Cargo.toml -- depends on pkg2
  pkg2/Cargo.toml
  pkg3/Cargo.toml
  1. run cargo watch from projectA/:

    • Current behaviour (8.x): all of projectA/ is watched
    • This PR: all of projectA/ is watched
    • so far so good
  2. run cargo watch from projectA/pkg1/:

    • Current behaviour (8.x): projectA/pkg1/ is watched
    • This PR: all of projectA/ is watched
    • Wanted in 117: both projectA/pkg1/ and projectA/pkg2/ are watched

Now consider this other structure:

projectB/
  Cargo.toml -- workspace manifest
  pkg1/Cargo.toml -- depends on pkg3
  pkg2/Cargo.toml

projectC/
  Cargo.toml -- workspace manifest
  pkg3/Cargo.toml
  pkg4/Cargo.toml
  1. run cargo watch from projectB/:

    • Current behaviour (8.x): all of projectB/ is watched
    • This PR: all of projectB/ and all of projectC/ are watched
    • Wanted in 117: unclear, but probably:
      • all of projectB/
      • and projectC/pkg3/
  2. run cargo watch from projectB/pkg1/:

    • Current behaviour (8.x): projectB/pkg1/ is watched
    • This PR: all of projectB/ and all of projectC/ are watched
    • Wanted in 117: both projectB/pkg1/ and projectC/pkg3/ are watched
  3. run cargo watch from projectB/pkg2/:

    • Current behaviour (8.x): projectB/pkg2/ is watched
    • This PR: all of projectB/ and all of projectC/ are watched
    • Wanted in 117: as there's no dependencies, presumably just projectB/pkg2/ is watched

@Blei
Copy link
Contributor Author

Blei commented Nov 19, 2022

I see. I thought I'd simplify the logic a bit because watching only the workspace root is probably fine :) I'll write up a new version which is more precise in what it tracks.

@Blei
Copy link
Contributor Author

Blei commented Nov 19, 2022

I think the latest version should fit what you describe.

This watches the package directories of all dependencies which are local,
plus the package directory of the current project. This has no effect if
--watch/-w is manually specified.

Add --skip-local-deps which disables this new behaviour.

Fixes #117.
@Blei
Copy link
Contributor Author

Blei commented Nov 20, 2022

Fixed the compile error.

@passcod
Copy link
Member

passcod commented Nov 22, 2022

Sorry for the delay! The code looks good, I just want to check a few more things and I haven't gotten to it yet. Next few days at the latest :) I did check things and they did look good, but the merge didn't happen because I got busy at work and forgot.

@passcod passcod merged commit 9dc829a into watchexec:8.x Dec 29, 2022
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.

Take local dependencies into account
2 participants