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

Need a way to ask cargo if the current package needs to be recompiled. #1989

Open
Kimundi opened this issue Sep 17, 2015 · 10 comments
Open

Need a way to ask cargo if the current package needs to be recompiled. #1989

Kimundi opened this issue Sep 17, 2015 · 10 comments
Labels
A-rebuild-detection Area: rebuild detection and fingerprinting C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-report S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.

Comments

@Kimundi
Copy link
Member

Kimundi commented Sep 17, 2015

I'm currently trying to write an exact file system watcher for a cargo project, so that any change that would cause a cargo build to actually end up recompiling the library can be detected. However, I'm stuck at figuring out just what file path changes to watch and react to:

  • If I listen to all changes in src, then I'll get false positives by temporary files or source files not part of the crate.
  • If I listen to all changes to *.rs files in src, I'll get false positves by source files not part of the crate, or false negatives for source files that don't have a *.rs ending (the latter might be irrelevant in practice, but is still technical possible).
  • If I don't listen to anywhere outside src, I miss changed or not yet downloaded dependencies
  • If I don't listen to anywhere outside src, I miss changed rustc or cargo executables (say a new nightly).

So I need a way to ask cargo what paths to listen to, including:

  • source of current package
  • manifest of current package
  • rustc and cargo executables
  • path dependencies

Furthermore I need a high-performance way to actually ask cargo wether a change in any of those paths will cause a recompile. I could just shell out to cargo build each time any change is detected, but thats going to be wasteful if its just temporary files of an text editor or harmless other changes that won't trigger a recompilation.

If using cargo as a library already supports any of those usecases in a robust way, I've not been able to find it yet.


If its not currently possible to do these things with cargo I'd be willing to add the necessary library features, though I'd need guidance about how cargo is currently structured internally 😄 .

I have limited information about how this could be best implemented, but right now I'm thinking about an API like this:

struct FsWatchList {
    paths: Vec<PathBuf>,
    /// some state so that it can keep track of needing a rebuild without doing IO
    needs_rebuild: bool,
}

impl FsWatchList {
    /// Looks for a package in p and gathers the lists of filesystem paths
    /// it could trigger a rebuild on. 
    fn new(p: &Path) -> Self { ... }

    /// incorporate all the changed paths, and decide wether
    /// they will cause a rebuild
    fn merge_changes(&mut self, changed_paths: &[PathBuf]) { ... }

    fn needs_rebuild(&self) -> bool { ... }

   // ... accessors for the path list, etc
}

Using that API, a filesystem watcher could crawl the contents of a package once with FsWatchList::new(), setup a filesystem notifcator for all relevant paths, and then call merge_changes() and needs_rebuild() each time it receives an event, triggering an rebuild only if needed.

Ideally there would also be a trigger_rebuild() method that does the same as cargo build without shelling out at all, but thats a different issue.

@alexcrichton
Copy link
Member

I'd totally be down for something like this in terms of a library API, and this would also be a great inclusion as part of a field in some sort of "local package metadata" command.

In terms of implementation most of this stuff is tracked in fingerprint.rs but it's definitely not in an "easily usable" state right now in terms of a consumer being able to leverage it. I'd be more than welcome to patches to tweak the API though!

@durka
Copy link
Contributor

durka commented Oct 7, 2015

I would really like a command like this, some kind of cargo what-changed to debug those cases where Cargo rebuilds a crate which I'll swear up and down has no changes. Can I help?

@Kimundi
Copy link
Member Author

Kimundi commented Oct 7, 2015

Of course! :)
I'm still in the phase where I'm getting my head around of how the execution of a cargo command ends up checking modification times. So far I've got the following "call stack" for a cargo build:

-> main()                                 bin/cargo.rs:60
  -> execute_main_without_stdin()         lib.rs:74
    -> process()                          lib.rs:99
      -> || callback                      lib.rs:82
        -> call_main_without_stdin()      lib.rs:87
          -> fn execute                   bin/cargo.rs:95
            -> call_main_without_stdin()  lib.rs:87
              -> fn build_execute()       bin/build.rs:61
                -> ops::compile()         ops/cargo_compile.rs:84

-> ops::compile()                         ops/cargo_compile.rs:84
  -> Package::for_path()                  core/package.rs:62
    -> SourceId::for_path()               core/source.rs:163
    -> ops::read_package()                ops/cargo_read_manifest.rs:22
      -> read_manifest
      -> Package::new
  -> compile_pkg()                        ops/cargo_compile.rs:100
    -> PackageRegistry::new()             core/registry.rs
    -> compile_targets()                  ops/cargo_rustc.rs:57
      -> compile()                        ops/cargo_rustc.rs:163
        // starts calling fingerprint stuff

Additional pairs of eyes digging around that code and thinking about how to restructure it so that checking modifications are decoupled from performing the action would be welcome.

@alexcrichton
Copy link
Member

If you're just trying to answer the question "why is this rebuilding", could you try out the new support landed in c447e9d? (only available in nightlies).

You can test it out via RUST_LOG=cargo::ops::cargo_rustc::fingerprint=info, and hopefully it should pretty precisely pinpoint what went wrong!

@Nemo157
Copy link
Member

Nemo157 commented Feb 24, 2016

If I listen to all changes to *.rs files in src, I'll get false positves by source files not part of the crate, or false negatives for source files that don't have a *.rs ending (the latter might be irrelevant in practice, but is still technical possible).

Definitely not irrelevant, in one of my projects I'm using include_bytes! for some static data and would want the build re-run when any of these files change (an issue with cargo-watch).

@Kimundi
Copy link
Member Author

Kimundi commented Feb 25, 2016

Yeah, thats an issue. It would probably help if rustc could track what source files it depends on outside of just the rust modules, and emit that info as part of the dep-info output.

@luser
Copy link
Contributor

luser commented Jun 21, 2016

We've discussed wanting something similar for invoking Cargo as part of the Gecko build system. My proposal was for Cargo to accept an argument specifying a GNU Make-style dependency file, similar to how rustc accepts --emit dep-info. It would possibly have to have every rustc invocation emit dep info, and then generate a file that included all of them, or aggregate them all somehow.

@alexcrichton
Copy link
Member

@luser just to confirm, but is this a hard constraint for using Cargo in Gecko for now? The approximation of "if any Rust code changes rerun Cargo" is probably what this would amount to for Gecko so it may not buy much in the long term, but it's certainly something we can try to implement soon if needed. (it may also be worth just always invoking Cargo if that's easy as it should be fast)

Note that Cargo definitely has all this info on hand, so it won't be too difficult to implement.

@luser
Copy link
Contributor

luser commented Jun 22, 2016

Oh, no, we had chatted about this not long ago and agreed that Gecko can probably live with "always rerun cargo" for now. I was just looking at the issues list to try to avoid filing a dupe and ran into this.

@carols10cents carols10cents added A-rebuild-detection Area: rebuild detection and fingerprinting C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` labels Sep 10, 2017
@weihanglo weihanglo added the S-triage Status: This issue is waiting on initial triage. label May 14, 2023
@epage
Copy link
Contributor

epage commented Sep 19, 2023

#2904 covers the cargo what-changed to help understand why a rebuild happened. That would likely live under cargo report. I could see us also having a cargo report whats-checked to say what files caused a rebuild last run which could then be used by a watch.

Furthermore I need a high-performance way to actually ask cargo wether a change in any of those paths will cause a recompile. I could just shell out to cargo build each time any change is detected, but thats going to be wasteful if its just temporary files of an text editor or harmless other changes that won't trigger a recompilation.

This is the part I don't quite understand. If you had a precise set of paths to watch (that is updated after every successful build), what difference is there between running cargo build -will-you and cargo build?

@epage epage added S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. and removed S-triage Status: This issue is waiting on initial triage. labels Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rebuild-detection Area: rebuild detection and fingerprinting C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-report S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
Projects
None yet
Development

No branches or pull requests

8 participants