Make sure CSRCS only points to existing files

RESOLVED FIXED in mozilla27

Status

Firefox Build System
General
RESOLVED FIXED
5 years ago
5 months ago

People

(Reporter: Ms2ger, Assigned: Ms2ger)

Tracking

Trunk
mozilla27
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Assignee)

Description

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

Mostly vpath issues here.
(Assignee)

Updated

5 years ago
Blocks: 875013
(Assignee)

Comment 1

5 years ago
Created attachment 811083 [details] [diff] [review]
Part a: build/unix/elfhack/inject
Attachment #811083 - Flags: review?(mh+mozilla)
(Assignee)

Comment 2

5 years ago
Created attachment 811084 [details] [diff] [review]
Part b: fix paths

The sorted() bits are for (hopefully) easier review, they go away in the next patch.
Attachment #811084 - Flags: review?(mshal)
(Assignee)

Comment 3

5 years ago
Created attachment 811085 [details] [diff] [review]
Part c: sort
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+

Updated

5 years ago
Attachment #811085 - Flags: review?(mshal) → review+
(Assignee)

Comment 6

5 years ago
(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
(Assignee)

Comment 8

5 years ago
(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.
(Assignee)

Comment 10

5 years ago
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.
(Assignee)

Updated

5 years ago
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?

Comment 13

5 years ago
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
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Updated

5 years ago
Target Milestone: --- → mozilla27

Updated

5 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.