1242663 - Removing a Makefile.in requires a clobber

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Michael Shal [:mshal]]

When removing a Makefile.in, an incremental build dies with:

INFO -  gmake[2]: *** No rule to make target `/builds/slave/m-in-l64-d-0000000000000000000/build/src/testing/mochitest/manifests/Makefile.in', needed by `backend.RecursiveMakeBackend'.  Stop.

The problem is that the backend dependencies were changed in bug 1239217, and this no longer includes the stub targets for handling file removal.

Comment 1

[Michael Shal [:mshal]]

Attachment: 0001-Bug-1242663-Add-targets-for-each-backend-dependency.patch

This seems to fix it while testing locally. Is there any reason we would need to add a separate line for each file rather than putting them all in one rule? eg:

foo/Makefile.in:
bar/Makefile.in:

vs:

foo/Makefile.in bar/Makefile.in:

(This patch does the latter)

Comment 2

[Mike Hommey [:glandium]]

Comment on attachment 8711810 [details] [diff] [review]
0001-Bug-1242663-Add-targets-for-each-backend-dependency.patch

Review of attachment 8711810 [details] [diff] [review]:
-----------------------------------------------------------------

::: Makefile.in
@@ +114,5 @@
>  	@$(TOUCH) $@
>  
>  define build_backend_rule
>  $(1): $$(shell cat $(1).in)
> +$$(shell cat $(1).in):

That makes us spawn a shell to read the file twice. Use a "temporary" variable?

Comment 3

[Michael Shal [:mshal]]

Attachment: 0001-Bug-1242663-Add-targets-for-each-backend-dependency.patch

Ahh, good call.

Comment 4

[(not currently active) Ted Mielczarek]

Comment on attachment 8712199 [details] [diff] [review]
0001-Bug-1242663-Add-targets-for-each-backend-dependency.patch

Review of attachment 8712199 [details] [diff] [review]:
-----------------------------------------------------------------

Stealing because I got bitten by this trying to land a patch today.

::: Makefile.in
@@ +113,5 @@
>  Makefile: $(BUILD_BACKEND_FILES)
>  	@$(TOUCH) $@
>  
>  define build_backend_rule
> +files := $$(shell cat $(1).in)

This might want to be $(1)_files := ...

because we might be evaluating build_backend_rule more than once.

Comment 5

[Michael Shal [:mshal]]

Attachment: 0001-Bug-1242663-Add-targets-for-each-backend-dependency-.patch

Updated to use $(1)_files, r+ carried forward.

Note that we do currently have multiple backends (recursive make & faster make).  The use of 'files' in the previous patch is expanded during the eval, so each backend is evaluated with its own set of files. However, using a separate variable for each is much more clear when dumping the database with 'make -p', since you can see the list of files retrieved for each backend, rather than just the last backend.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f8fc7a05f96b

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f8fc7a05f96b