Skip to content

Add dependencies for top-level build script #957

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

Merged
merged 1 commit into from
Aug 13, 2017

Conversation

alexcrichton
Copy link
Member

Helps avoid unnecessarily compiling the main library!

build.rs Outdated
@@ -8,6 +8,8 @@ use dotenv::dotenv;
use std::env;

fn main() {
println!("cargo:rerun-if-env-changed=TET_DATABASE_URL");
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this supposed to be TEST_DATABASE_URL ? :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops indeed!

Helps avoid unnecessarily compiling the main library!
@carols10cents
Copy link
Member

awesome!!! thanks!!

bors: r+

bors-voyager bot added a commit that referenced this pull request Aug 13, 2017
957: Add dependencies for top-level build script r=carols10cents

Helps avoid unnecessarily compiling the main library!
@bors-voyager
Copy link
Contributor

bors-voyager bot commented Aug 13, 2017

Build succeeded

@bors-voyager bors-voyager bot merged commit dd1523a into rust-lang:master Aug 13, 2017
@jtgeibel
Copy link
Member

I've just tested this locally, by: running cargo test, adding a migration, and running cargo test again. My new migration was not run.

I've added the following to the build script, which works for my simple test, but I'm not sure it is a complete solution:

println!("cargo:rerun-if-changed=migrations/");

Here are the reasons I think this may not be complete:

  • The build script documentation notes that the behavior of timestamps on directories can be platform specific. However, I'm not aware of what those differences may be.
  • The documentation notes that the directory is not traversed to check for additional timestamp changes. We can manually walk the tree and add a line for each directory and file, but I'm not certain that is necessary for this use case. If someone changes a migration during development (after running the initial migration), then they will need to run diesel migration redo before running cargo test anyway.

Thoughts?

@carols10cents
Copy link
Member

@jtgeibel I would like a pull request for that please, I think it'll be good enough for most of the cases!

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.

4 participants