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

Parse depfiles with multiple targets #113

Merged
merged 1 commit into from
Mar 5, 2024

Conversation

Colecf
Copy link
Contributor

@Colecf Colecf commented Feb 29, 2024

This is needed for android. It just treats all the deps of all targets in the depfile as deps for the Build.

In task.rs there is a TODO to verify that the targets refer to the outputs of the Build, but adding that verification breaks the android build. In android's case, there are some depfiles that contain foo.o: path/to/some/dep.asm where it should instead be path/to/foo.o: path/to/some/dep.asm.

With this change, you can do a full build of android with n2.

This is needed for android. It just treats all the deps of all targets
in the depfile as deps for the Build.

In task.rs there is a TODO to verify that the targets refer to the
outputs of the Build, but adding that verification breaks the android
build. In android's case, there are some depfiles that contain
`foo.o: path/to/some/dep.asm` where it should instead be
`path/to/foo.o: path/to/some/dep.asm`.
@Colecf Colecf force-pushed the depfiles_with_multiple_targets branch from e2f54f3 to 4c78be8 Compare February 29, 2024 22:08
@evmar
Copy link
Owner

evmar commented Mar 5, 2024

In task.rs there is a TODO to verify that the targets refer to the outputs of the Build, but adding that verification breaks the android build.

It looks like Ninja does something here:
https://github.com/ninja-build/ninja/blob/ab510c7a8cccbea0ea2c82531dc23893b551d55e/src/graph.cc#L693
so it must somehow work for Android.

I also found ninja-build/ninja@8a9edb1 while skimming the code in this area, seems an area fraught with peril :(

@evmar evmar merged commit 8881a66 into evmar:main Mar 5, 2024
2 checks passed
@Colecf Colecf deleted the depfiles_with_multiple_targets branch March 5, 2024 19:30
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.

2 participants