Closed
Bug 912438
Opened 11 years ago
Closed 11 years ago
Make sure CSRCS only points to existing files
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla27
People
(Reporter: Ms2ger, Assigned: Ms2ger)
References
Details
Attachments
(3 files)
(deleted),
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
mshal
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
mshal
:
review+
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #912099 +++
Mostly vpath issues here.
Assignee | ||
Comment 1•11 years ago
|
||
Attachment #811083 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 2•11 years ago
|
||
The sorted() bits are for (hopefully) easier review, they go away in the next patch.
Attachment #811084 -
Flags: review?(mshal)
Assignee | ||
Comment 3•11 years ago
|
||
Attachment #811085 -
Flags: review?(mshal)
Comment 4•11 years ago
|
||
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 5•11 years ago
|
||
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•11 years ago
|
Attachment #811085 -
Flags: review?(mshal) → review+
Assignee | ||
Comment 6•11 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.
Comment 7•11 years ago
|
||
(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•11 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.
Comment 9•11 years ago
|
||
I don't follow.
Assignee | ||
Comment 10•11 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 | ||
Comment 11•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6b7ec3be1e7e
https://hg.mozilla.org/mozilla-central/rev/eac86a85a8da
https://hg.mozilla.org/mozilla-central/rev/2864587e0c7c
Flags: in-testsuite-
Comment 12•11 years ago
|
||
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•11 years ago
|
||
https://tbpl.mozilla.org/ doesn't show any build bustage. Maybe this is breaking stuff in your own code?
Comment 14•11 years ago
|
||
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•11 years ago
|
||
(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•11 years ago
|
||
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: 11 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•11 years ago
|
Target Milestone: --- → mozilla27
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•