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

README.md: Add note that rerun-if-changed is not emitted. #706

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

EvanTheB
Copy link

Issue #230 is the issue for this, this patch just documents the
current behaviour.

Issue rust-lang#230 is the issue for this, this patch just documents the
current behaviour.
@dot-asm
Copy link
Contributor

dot-asm commented Jul 31, 2022

Customarily the decision whether or not a package gets rebuilt depends on whether or not any of the files in corresponding cargo package --list [--allow-dirty] output are modified. Customarily you would make your .c sources to be a part of the package, so that emitting explicit cargo:rerun-if-changed directives wouldn't be normally required. I suppose question is if it's some unusual case, then it might be more appropriate to discuss the specifics of when it would be actually required to emit the directives.

@dot-asm
Copy link
Contributor

dot-asm commented Aug 1, 2022

There might be some misunderstanding on my[?] side. You'd use cargo:rerun-if-changed in case you find that rebuilding upon any change in cargo package --list excessive. But either way, the suggested "Note that cc-rs does not automatically tell cargo to rebuild when the c files are changed" still doesn't adequately reflect the current state of affairs. I mean in most common situation cargo will follow modification timestamps without explicit if-changed directives.

@EvanTheB
Copy link
Author

EvanTheB commented Aug 3, 2022

I need to recheck as I dont understand this very well. I had a c file in my src/ folder, which does show up in cargo package --list. However modifying that file did not cause a rebuild. Searching the issue brought me to the mentioned issue, and adding rerun-if-changed made the behaviour what I expect (rebuild when I modify the c file). There might be something else at play.

Because pkg_config is emitting rerun-if commands, the default behaviour you mention may not be as expected. Its not clear what is the 'right' way to solve this. But it would have saved me half a day of debugging to have this mentioned in the cc docs somehow...

extern crate cc;

fn main() {
    // pkg_config is needed only to pick up the include path for log.c to use.
    // libflashrom-sys tells cargo how to link to libflashrom.
    let flashrom = pkg_config::Config::new()
        .cargo_metadata(false)
        .probe("flashrom")
        .unwrap();
    let mut log_c = cc::Build::new();
    log_c.file("src/log.c");
    for p in flashrom.include_paths {
        log_c.include(p);
    }
    log_c.compile("log.o");
    println!("cargo:rerun-if-changed=src/log.c");
}

@dot-asm
Copy link
Contributor

dot-asm commented Aug 3, 2022

Because pkg_config is emitting rerun-if commands, the default behaviour you mention may not be as expected.

Bingo! If you have one rerun-if, you need all of them. And that's what the note should say instead of "cargo never tells." I suppose it would be appropriate to mention the possibility of build-dependency crates adding rerun-if without you realizing it in advance...

Copy link
Member

@thomcc thomcc left a comment

Choose a reason for hiding this comment

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

As discussed in this issue, this isn't quite right (or is at least misleading). I think it would be good to document the behavior, but we should be somewhat more precise about how this works, since in the common case you don't need to provide these.

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.

3 participants