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

Rebuild if changed #765

Open
orenbenkiki opened this issue Dec 19, 2022 · 10 comments
Open

Rebuild if changed #765

orenbenkiki opened this issue Dec 19, 2022 · 10 comments

Comments

@orenbenkiki
Copy link

When using the cc crate...

what is the proper way to set up build.rs and/or Cargo.toml so that cargo will automatically recompile and relink the C/C++ files if they are changed since the last compilation? Maybe even list the local header files so changing them will trigger recompilation/relink as well?

Alternatively, what is the proper way to force cargo to recompile all the C/C++ files?

Right now I can always remove the target/{debug,release} directories to force recompilation but it seems like a hack.

@dot-asm
Copy link
Contributor

dot-asm commented Dec 20, 2022

build.rs is executed whenever any file from the cargo package --list output is changed. So if you didn't list your C files in the include directive in your Cargo.toml yet, it might be time. This is assuming that you'd eventually have to do that, as you're likely to publish your crate. If not, then you would find println!("cargo:rerun-if-changed=dir/ect/ory"); in your build.rs useful.

It should be noted that none of this is actually cc-rs-specific, it's how cargo works in general.

@orenbenkiki
Copy link
Author

orenbenkiki commented Dec 22, 2022

It sounds as though I should expect the C compiler to be invoked if any of the files in cargo package --list were changed. Did I understand this right?

Two points then:

  • Even if it did work this way, that would be sub-optimal.

    In principle there should be a separate list of files which trigger C compilation (which would make this a specific issue for the cc crate). Of course "ideally" there should be a list of dependencies for each separate .c file to be compiled (that is, the list of .h files it includes), so not all the .c files would be compiled, just these that needed to. I guess it is a matter of deciding "how much like make do we want cc to be".

    It isn't a huge deal as long as these files are small. I can live with "all C object files depend on all the package files".

  • More importantly, it doesn't seem to work for me.

    I ensured that cargo package --list did list the .c files. It did even without an explicit include directive in Cargo.toml, but I added one anyway and also listed the .h files in it.

    Regardless, if I touch any of the .c/.h files, then cc does not trigger re-compilation.

    I'm attaching a small with_c.zip file which demonstrates the problem - see RUNME.sh and the results I get in RUNME.log.

@dot-asm
Copy link
Contributor

dot-asm commented Dec 22, 2022

I should expect the C compiler to be invoked if any of the files in cargo package --list were changed.

As already implied, the question is when does cargo execute the build.rs. That's all to it. The fact that build.rs invokes C compiler is immaterial to the said decision.

it doesn't seem to work for me.

Ah! It ought to be one of the recent modifications that broke it. It does work if you specify the dependency as cc = "<=1.0.73"... Not that I insist on doing this, but at least there is an explanation for why it stopped working the described way.

Even if it did work this way, that would be sub-optimal.

It's by cargo's design, we have to accept it.

I guess it is a matter of deciding "how much like make do we want cc to be".

It sounds like you'd be better off using cmake-rs. Idea would be to have cargo detect a change and hand it off to cmake to identify what actually needs to be done. Replicating the make functionality in cc-rs would probably be deemed out of this project's scope...

@dot-asm
Copy link
Contributor

dot-asm commented Dec 22, 2022

one of the recent modifications that broke it.

More specifically emission of cargo:rerun-if-env-changed=<this-n-that> in 1.0.74. Collateral problem here is that one single rerun-if apparently puts aside the internal cargo decision making mechanism. You can suppress the referred directives with .emit_rerun_if_env_changed(false) method.

@orenbenkiki
Copy link
Author

Thanks for the clarification. As long as my C code is trivial, cc will do the trick. For larger, more complex projects, cmake-rs might be better. Good to know the options.

Yes, using 1.0.73 does fix the issue for me. Thanks!

@thomcc
Copy link
Member

thomcc commented Dec 22, 2022

I’m on vacation now, but I’ll investigate what broke in 1.0.74 when I get back.

Note that pinning a version like that will cause problems for your users down the line, since if another crate in the dependency graph depends on a later version of cc, cargo will error (it only duplicates versions if they would be semver incompatible).

@orenbenkiki
Copy link
Author

Noted - thanks for the warning.

Happy holidays!

@bmwebster
Copy link

Is there anything more on this? I just ran into it in my project and it's quite a surprise when c files don't recompile (especially since they sometimes do - if you have errors and compilation fails, it will re-run on a new cargo build, but otherwise I need to cargo clean and rebuild all the rust code as well to get c to recompile).

I've tried adding .emit_rerun_if_env_changed(false) to the cc::Build chain in my build.rs but it doesn't seem to make any difference.

@bmwebster
Copy link

If I add println!("cargo:rerun-if-changed=src"); to build.rs this does trigger a rebuild on changes (all my .c and .h files are in src).

@NobodyXu
Copy link
Collaborator

Maybe we could add a new function emit_rerun_if_src_changed for this that is set to false by default for backwards compatibility?

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

No branches or pull requests

5 participants