912438 - Make sure CSRCS only points to existing files

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

+++ This bug was initially created as a clone of Bug #912099 +++

Mostly vpath issues here.

Comment 1

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part a: build/unix/elfhack/inject

Comment 2

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part b: fix paths

The sorted() bits are for (hopefully) easier review, they go away in the next patch.

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part c: sort

Comment 4

[Mike Hommey [:glandium]]

Comment on attachment 811083 [details] [diff] [review]
Part a: build/unix/elfhack/inject

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

::: build/unix/elfhack/inject/Makefile.in
@@ +13,5 @@
>  CPU := arm
>  else
>  CPU := $(TARGET_CPU)
>  endif
>  endif

Can you remove this block and change $(CPU)-noinit.$(OBJ_SUFFIX) to something like $(filter %-noinit.$(OBJ_SUFFIX),$(OBJS))?

Comment 5

[Michael Shal [:mshal]]

Comment on attachment 811084 [details] [diff] [review]
Part b: fix paths

Looks good - I'd have been fine with reviewing the sorting all in one, too :)

Comment 6

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

(In reply to Mike Hommey [:glandium] from comment #4)
> Comment on attachment 811083 [details] [diff] [review]
> Part a: build/unix/elfhack/inject
> 
> Review of attachment 811083 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: build/unix/elfhack/inject/Makefile.in
> @@ +13,5 @@
> >  CPU := arm
> >  else
> >  CPU := $(TARGET_CPU)
> >  endif
> >  endif
> 
> Can you remove this block and change $(CPU)-noinit.$(OBJ_SUFFIX) to
> something like $(filter %-noinit.$(OBJ_SUFFIX),$(OBJS))?

I don't follow.

Comment 7

[Mike Hommey [:glandium]]

(In reply to :Ms2ger from comment #6)
> I don't follow.

diff --git a/build/unix/elfhack/inject/Makefile.in b/build/unix/elfhack/inject/Makefile.in
--- a/build/unix/elfhack/inject/Makefile.in
+++ b/build/unix/elfhack/inject/Makefile.in
@@ -1,30 +1,20 @@
 #
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 INTERNAL_TOOLS = 1
 NO_PROFILE_GUIDED_OPTIMIZE = 1
 
-ifneq (,$(filter %86,$(TARGET_CPU)))
-CPU := x86
-else
-ifneq (,$(filter arm%,$(TARGET_CPU)))
-CPU := arm
-else
-CPU := $(TARGET_CPU)
-endif
-endif
-
 include $(topsrcdir)/config/rules.mk
 
 export:: $(CSRCS:.c=.$(OBJ_SUFFIX))
 
 $(CSRCS): %.c: ../inject.c
        cp $< $@
 
 GARBAGE += $(CSRCS)
 
 DEFINES += -DBITS=$(if $(HAVE_64BIT_OS),64,32)
 CFLAGS := -O2 -fno-stack-protector $(filter -m% -I%,$(CFLAGS))
-$(CPU)-noinit.$(OBJ_SUFFIX): DEFINES += -DNOINIT
+$(filter %-noinit.$(OBJ_SUFFIX),$(OBJS)): DEFINES += -DNOINIT

Comment 8

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

(In reply to Mike Hommey [:glandium] from comment #7)
> (In reply to :Ms2ger from comment #6)
> > I don't follow.
> 
> diff --git a/build/unix/elfhack/inject/Makefile.in
> b/build/unix/elfhack/inject/Makefile.in
> --- a/build/unix/elfhack/inject/Makefile.in
> +++ b/build/unix/elfhack/inject/Makefile.in
> @@ -1,30 +1,20 @@
>  #
>  # This Source Code Form is subject to the terms of the Mozilla Public
>  # License, v. 2.0. If a copy of the MPL was not distributed with this
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
>  INTERNAL_TOOLS = 1
>  NO_PROFILE_GUIDED_OPTIMIZE = 1
>  
> -ifneq (,$(filter %86,$(TARGET_CPU)))
> -CPU := x86
> -else
> -ifneq (,$(filter arm%,$(TARGET_CPU)))
> -CPU := arm
> -else
> -CPU := $(TARGET_CPU)
> -endif
> -endif
> -
>  include $(topsrcdir)/config/rules.mk
>  
>  export:: $(CSRCS:.c=.$(OBJ_SUFFIX))
>  
>  $(CSRCS): %.c: ../inject.c
>         cp $< $@
>  
>  GARBAGE += $(CSRCS)
>  
>  DEFINES += -DBITS=$(if $(HAVE_64BIT_OS),64,32)
>  CFLAGS := -O2 -fno-stack-protector $(filter -m% -I%,$(CFLAGS))
> -$(CPU)-noinit.$(OBJ_SUFFIX): DEFINES += -DNOINIT
> +$(filter %-noinit.$(OBJ_SUFFIX),$(OBJS)): DEFINES += -DNOINIT

That doesn't work; the sources can't be in moz.build after this bug because they aren't in the srcdir.

Comment 9

[Mike Hommey [:glandium]]

I don't follow.

Comment 10

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

The goal of this bug is to enforce that CSRCS in moz.build only contains files that exist in the source dir; $(CPU).c and $(CPU)-noinit.c don't, as they're both copied from ../inject.c. Once these are back in the Makefile.in, I don't see how I can remove the block that defines CPU.

Comment 11

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/6b7ec3be1e7e
https://hg.mozilla.org/mozilla-central/rev/eac86a85a8da
https://hg.mozilla.org/mozilla-central/rev/2864587e0c7c

Comment 12

[Diego Wilson [:diego]]

Looks like this caused some build breakage. See bug 923405. Was this change landed on m-c without going through m-i first?

Comment 13

[Michael Wu [:mwu]]

https://tbpl.mozilla.org/ doesn't show any build bustage. Maybe this is breaking stuff in your own code?

Comment 14

[Diego Wilson [:diego]]

Looks like this breaks DMD (see bug 923489) which is only optional for B2G. I'll turn it off in our builds for now.

Comment 15

[Ed Morley [:emorley]]

(In reply to Diego Wilson [:diego] from comment #12)
> Looks like this caused some build breakage. See bug 923405. Was this change
> landed on m-c without going through m-i first?

mozilla-inbound and mozilla-central on the whole run the same set of builds and tests (apart from a couple of edge cases that don't make any difference here), so "skipping inbound" wouldn't make any difference :-)

Comment 16

[Mike Hommey [:glandium]]

In practice, the combination of various bugs that landed recently take care of verifying that sources files exist. The remaining parts is to move those source definitions in moz.build, and there are bugs around for that. Let's close this one.