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

Build system doesn't correctly rebuild submodules that depend on other submodules #210

Closed
burg opened this issue Nov 15, 2012 · 4 comments
Closed

Comments

@burg
Copy link

@burg burg commented Nov 15, 2012

In some scenarios, submodules that depend on other submodules will not necessarily rebuild when their dependencies are rebuilt. This is necessary if their API changes at all.

Sometimes this manifests as an ICE when compiling servo top-level modules, and other times it's a runtime dyld error.

@zmike
Copy link
Contributor

@zmike zmike commented Jan 14, 2014

If someone provides a list of what these dependencies should be, I can fix this.

@metajack
Copy link
Contributor

@metajack metajack commented Jan 14, 2014

I think these are all correct express in mk/sub.mk, but it is not enough to work quite right. The reason is that each individual submodule has it's own make, and we express dependencies only on targets.

For example, rust-azure depends on skia (as in, the rust-azure target depends on the skia target). This doesn't work in the following case:

  1. Run make and wait for servo to fully build
  2. Modify skia.
  3. Run make skia.
  4. Run make rust-azure.

Step 4 will not rebuild rust-azure because the Skia target is up to date.

The correct solution is that the target should not depend on a target, but on the actual output product it needs. We haven't fixed this yet because the Makefiles are ostensibly Servo independent and this would hardcode the directory structure we use into the build files. I'm hoping we can clean this up with CMake by addressing the problem in a way that doesn't hardcode our own structure into the build.

@zmike
Copy link
Contributor

@zmike zmike commented Jan 14, 2014

Okay, so this is like #472 which I already fixed. The real issue here is that the project uses handwritten Makefiles instead of something more robust, which means that the rules have to be written correctly in all cases to avoid issues like this. Switching to CMake is one option, but I'll gladly leave that task for someone else.

In the meanwhile, I'm operating on the assumption that since all these build-related tasks were opened a long time ago, they should probably be fixed using the current build system rather than waiting for a new one. I'll look at the deps listed in the file you mentioned.

ChrisParis pushed a commit to ChrisParis/servo that referenced this issue Sep 7, 2014
shadow-dom: Fixed indent style and performance improvement
@jdm
Copy link
Member

@jdm jdm commented Oct 21, 2014

Fixed by Cargo.

@jdm jdm closed this Oct 21, 2014
glennw pushed a commit to glennw/servo that referenced this issue Jan 16, 2017
Don't crash on complex clips.

Fixes browser.html.

r? @glennw
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.