Closed Bug 912438 Opened 7 years ago Closed 6 years ago

Make sure CSRCS only points to existing files

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla27

People

(Reporter: Ms2ger, Assigned: Ms2ger)

References

Details

Attachments

(3 files)

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

Mostly vpath issues here.
Blocks: 875013
Attachment #811083 - Flags: review?(mh+mozilla)
The sorted() bits are for (hopefully) easier review, they go away in the next patch.
Attachment #811084 - Flags: review?(mshal)
Attached patch Part c: sortSplinter Review
Attachment #811085 - Flags: review?(mshal)
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))?
Attachment #811083 - Flags: review?(mh+mozilla) → review+
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 :)
Attachment #811084 - Flags: review?(mshal) → review+
Attachment #811085 - Flags: review?(mshal) → review+
(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.
(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
(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.
I don't follow.
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.
Depends on: 923489
Looks like this caused some build breakage. See bug 923405. Was this change landed on m-c without going through m-i first?
https://tbpl.mozilla.org/ doesn't show any build bustage. Maybe this is breaking stuff in your own code?
Looks like this breaks DMD (see bug 923489) which is only optional for B2G. I'll turn it off in our builds for now.
(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 :-)
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.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.