870406 - Move CSRCS to moz.build

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Comment 1

[Joey Armstrong [:joey]]

Attachment: move CSRCS to moz.build (logic)

Comment 2

[Joey Armstrong [:joey]]

Comment on attachment 751872 [details] [diff] [review]
move CSRCS to moz.build (logic)

Add CSRCS as a passthrough variable.

Comment 3

[Joey Armstrong [:joey]]

Inbound push:

changeset: 132691:e3faa44c33e4
https://hg.mozilla.org/integration/mozilla-inbound/rev/e3faa44c33e4

Comment 4

[Ed Morley [:emorley]]

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

Comment 5

[Joey Armstrong [:joey]]

Attachment: move to moz.build (config)

Comment 6

[Joey Armstrong [:joey]]

Comment on attachment 753535 [details] [diff] [review]
move to moz.build (config)

First batch of CSRCS variables to convert.
intl/uconv/moz.build - added missing tools/ directory to compile umaptable.
Try job pending.

Comment 7

[Joey Armstrong [:joey]]

Attachment: move to moz.build (config batch #1)

Comment 8

[Joey Armstrong [:joey]]

Comment on attachment 753559 [details] [diff] [review]
move to moz.build (config batch #1)

Same patch as before (initial set of converted CSRCS).
Removed 'tools' from intl/uconv/moz.build.

Another bug to be opened, platform filtering or linkage problems need to be resolved.

Comment 9

[Michael Shal [:mshal]]

Comment on attachment 753559 [details] [diff] [review]
move to moz.build (config batch #1)

>-SIMPLE_PROGRAMS	= $(CSRCS:.c=$(BIN_SUFFIX))
>+SIMPLE_PROGRAMS	= $(DISABLED_CSRCS:.c=$(BIN_SUFFIX))

As we discussed in person, can't you still use CSRCS here? The deferred resolution should work to pick up CSRCS from backend.mk, until we move SIMPLE_PROGRAMS to moz.build

>diff --git a/modules/libmar/tool/moz.build b/modules/libmar/tool/moz.build
>--- a/modules/libmar/tool/moz.build
>+++ b/modules/libmar/tool/moz.build
>@@ -3,8 +3,12 @@
> # 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/.
> 
> MODULE = 'mar'
> 
> if CONFIG['MOZ_ENABLE_SIGNMAR']:
>     PROGRAM = 'signmar'
>+
>+# bug 875549: Temporarly break dep on HOST_CSRCS= so CSRCS can convert.
>+host_csrcs = ['mar.c']
>+CSRCS += host_csrcs

Why does the patch for bug 870406 have changes for bug 875549?

>diff --git a/netwerk/dns/moz.build b/netwerk/dns/moz.build
>--- a/netwerk/dns/moz.build
>+++ b/netwerk/dns/moz.build
>@@ -16,8 +16,9 @@ XPIDL_SOURCES += [
> XPIDL_MODULE = 'necko_dns'
> 
> MODULE = 'necko'
> 
> EXPORTS.mozilla.net += [
>     'DNS.h',
> ]
> 
>+CSRCS += ['nameprep.c', 'punycode.c', 'race.c']

Please use the standard one-per-line moz.build convention (here and everywhere else in this patch) -

CSRCS += [
    'nameprep.c',
    'punycode.c',
    'race.c',
]

Comment 10

[Joey Armstrong [:joey]]

(In reply to Michael Shal [:mshal] from comment #9)
> Comment on attachment 753559 [details] [diff] [review]
> move to moz.build (config batch #1)
> 
> >-SIMPLE_PROGRAMS	= $(CSRCS:.c=$(BIN_SUFFIX))
> >+SIMPLE_PROGRAMS	= $(DISABLED_CSRCS:.c=$(BIN_SUFFIX))
> 
> As we discussed in person, can't you still use CSRCS here? The deferred
> resolution should work to pick up CSRCS from backend.mk, until we move
> SIMPLE_PROGRAMS to moz.build

CSRCS was empty on an earlier build attempt.


> 
> >diff --git a/modules/libmar/tool/moz.build b/modules/libmar/tool/moz.build
> >--- a/modules/libmar/tool/moz.build
> >+++ b/modules/libmar/tool/moz.build
> >@@ -3,8 +3,12 @@
> > # 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/.
> > 
> > MODULE = 'mar'
> > 
> > if CONFIG['MOZ_ENABLE_SIGNMAR']:
> >     PROGRAM = 'signmar'
> >+
> >+# bug 875549: Temporarly break dep on HOST_CSRCS= so CSRCS can convert.
> >+host_csrcs = ['mar.c']
> >+CSRCS += host_csrcs
> 
> Why does the patch for bug 870406 have changes for bug 875549?

[info: 875549 is a followup bug]
This will allow the conversions to remain independent.

HOST_CSRCS is defined by Makefile.in.  moz.build::CSRCS will not be able to access that value because backend.mk will be included before HOST_CSRCS is defined.  To grab that value the HOST_CSRCS conversion would come into play which then requires landing both at the same time {increasing the chance of removal due to test failures :)}.

> 
> >diff --git a/netwerk/dns/moz.build b/netwerk/dns/moz.build
> >--- a/netwerk/dns/moz.build
> >+++ b/netwerk/dns/moz.build
> >@@ -16,8 +16,9 @@ XPIDL_SOURCES += [
> > XPIDL_MODULE = 'necko_dns'
> > 
> > MODULE = 'necko'
> > 
> > EXPORTS.mozilla.net += [
> >     'DNS.h',
> > ]
> > 
> >+CSRCS += ['nameprep.c', 'punycode.c', 'race.c']
> 
> Please use the standard one-per-line moz.build convention (here and
> everywhere else in this patch) -

todo

Do you know if the standards are written down anywhere ?  If not we probably need a section for this in the style guide:
https://developer.mozilla.org/en-US/docs/Developer_Guide/Coding_Style



> CSRCS += [
>     'nameprep.c',
>     'punycode.c',
>     'race.c',
> ]

Comment 11

[Joey Armstrong [:joey]]

(In reply to Joey Armstrong [:joey] from comment #10)
> (In reply to Michael Shal [:mshal] from comment #9)
> > Comment on attachment 753559 [details] [diff] [review]
> > move to moz.build (config batch #1)
> > 
> > >-SIMPLE_PROGRAMS	= $(CSRCS:.c=$(BIN_SUFFIX))
> > >+SIMPLE_PROGRAMS	= $(DISABLED_CSRCS:.c=$(BIN_SUFFIX))
> > 
> > As we discussed in person, can't you still use CSRCS here? The deferred
> > resolution should work to pick up CSRCS from backend.mk, until we move
> > SIMPLE_PROGRAMS to moz.build

Oh also bug 632224 is already open for the SIMPLE_PROGRAMS conversion so this will go away in the near future.

Comment 12

[Joey Armstrong [:joey]]

try results: https://tbpl.mozilla.org/?tree=Try&rev=756ae8cc8efb

Comment 13

[Joey Armstrong [:joey]]

Attachment: move to moz.build (config batch #1)

Comment 14

[Joey Armstrong [:joey]]

Comment on attachment 753599 [details] [diff] [review]
move to moz.build (config batch #1)

nit cleanup, split file lists into one per line.

Comment 15

[Joey Armstrong [:joey]]

Try results for config edits, batch #1:
https://tbpl.mozilla.org/?tree=Try&rev=756ae8cc8efb

Comment 16

[Michael Shal [:mshal]]

Comment on attachment 753599 [details] [diff] [review]
move to moz.build (config batch #1)


>diff --git a/netwerk/dns/moz.build b/netwerk/dns/moz.build
>--- a/netwerk/dns/moz.build
>+++ b/netwerk/dns/moz.build
>@@ -16,8 +16,13 @@ XPIDL_SOURCES += [
> XPIDL_MODULE = 'necko_dns'
> 
> MODULE = 'necko'
> 
> EXPORTS.mozilla.net += [
>     'DNS.h',
> ]
> 
>+CSRCS += [
>+    'nameprep.c',
>+    'punycode.c',
>+    'race.c',
>+    ]

nit: It looks like we align the closing ']' with the first character of the variable name, rather than indenting it. This shows up in a few other places in this patch.

Comment 17

[Joey Armstrong [:joey]]

Attachment: move to moz.build (config batch #1)

Comment 18

[Joey Armstrong [:joey]]

Attachment: CSRCS variable conversion, first batch of files

File batch #1 - nit fixes, align ']'.  Will have to revisit this post conversion, default auto-indent for python at least with emacs aligns with the value list -vs- var name definition. r=mshal carried forward.

Comment 19

[Joey Armstrong [:joey]]

push to inbound:
https://bugzilla.mozilla.org/attachment.cgi?id=753973

changeset:   132965:773937adfcac
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/773937adfcac

Comment 20

[Joey Armstrong [:joey]]

Comment on attachment 753973 [details] [diff] [review]
CSRCS variable conversion, first batch of files

re-post to attach info to the appropriate patch.

push to inbound:
https://bugzilla.mozilla.org/attachment.cgi?id=753973

changeset:   132965:773937adfcac
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/773937adfcac

Comment 21

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/773937adfcac

Comment 22

[Joshua Cranmer [:jcranmer]]

Attachment: Migrate CSRCS in comm-central

The files in mailnews/mapi/mapihook/build are autogenerated by the MIDL, so I'm slightly concerned about moving it to moz.build. It does appear to work on try though:
https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=3c22399a614c

Comment 23

[Joey Armstrong [:joey]]

Attachment: move CSRCS to moz.build (file bundle #2)

convert media/ and memory/ makefiles.

Comment 24

[Michael Shal [:mshal]]

Comment on attachment 756101 [details] [diff] [review]
move CSRCS to moz.build (file bundle #2)

>diff --git a/memory/jemalloc/moz.build b/memory/jemalloc/moz.build
>--- a/memory/jemalloc/moz.build
>+++ b/memory/jemalloc/moz.build
>@@ -1,8 +1,39 @@
> # -*- Mode: python; c-basic-offset: 4; indent-tabs-mode: nil; tab-width: 40 -*-
> # vim: set filetype=python:
> # 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/.
> 
> MODULE = 'jemalloc'
> 
>+CSRCS += [
>+    'arena.c',
>+    'atomic.c',
>+    'base.c',
>+    'bitmap.c',
>+    'chunk.c',
>+    'chunk_dss.c',
>+    'chunk_mmap.c',
>+    'ckh.c',
>+    'ctl.c',
>+    'extent.c',
>+    'hash.c',
>+    'huge.c',
>+    'jemalloc.c',
>+    'mb.c',
>+    'mutex.c',
>+    'prof.c',
>+    'quarantine.c',
>+    'rtree.c',
>+    'stats.c',
>+    'tcache.c',
>+    'tsd.c',
>+    'util.c',
>+]
>+
>+# Only OSX needs the zone allocation implementation,
>+# but only if replace-malloc is not enabled.
>+if CONFIG['OS_TARGET'] == Darwin or CONFIG['MOZ_REPLACE_MALLOC'] == 'Darwin':
>+    CSRCS += [
>+        'zone.c',
>+    ]

I don't think this if statement is correct. The first Darwin is not in quotes, so wouldn't that be a variable? I think this should be:

if CONFIG['OS_TARGET'] == 'Darwin' and not CONFIG['MOZ_REPLACE_MALLOC']:
    CSRCS += [
        'zone.c',
    ]

(Please double check)

>diff --git a/memory/jemalloc/src/moz.build b/memory/jemalloc/src/moz.build
>new file mode 100644
>--- /dev/null
>+++ b/memory/jemalloc/src/moz.build
>@@ -0,0 +1,35 @@
>+# -*- Mode: python; c-basic-offset: 4; indent-tabs-mode: nil; tab-width: 40 -*-
>+# vim: set filetype=python:
>+# 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/.
>+
>+CSRCS += [
>+    'src/arena.c',
>+    'src/atomic.c',
>+    'src/base.c',
>+    'src/bitmap.c',
>+    'src/chunk.c',
>+    'src/chunk_dss.c',
>+    'src/chunk_mmap.c',
>+    'src/ckh.c',
>+    'src/ctl.c',
>+    'src/extent.c',
>+    'src/hash.c',
>+    'src/huge.c',
>+    'src/jemalloc.c',
>+    'src/mb.c',
>+    'src/mutex.c',
>+    'src/prof.c',
>+    'src/quarantine.c',
>+    'src/rtree.c',
>+    'src/stats.c',
>+    'src/tcache.c',
>+    'src/tsd.c',
>+    'src/util.c',
>+]
>+
>+if CONFIG['ABI'] == 'macho':
>+    CSRCS += [
>+        'src/zone.c',
>+    ]

Is ABI an actual config variable? It doesn't show up in my autoconf.mk, but maybe it is platform specific.

Everything else looks good to me - I can r+ when the if-statement is fixed.

Comment 25

[Joey Armstrong [:joey]]

Comment on attachment 756101 [details] [diff] [review]
move CSRCS to moz.build (file bundle #2)

try results: https://tbpl.mozilla.org/?tree=Try&rev=364948d0fee3

Comment 26

[Joey Armstrong [:joey]]

(In reply to Michael Shal [:mshal] from comment #24)
> Comment on attachment 756101 [details] [diff] [review]
> move CSRCS to moz.build (file bundle #2)
> 
> >diff --git a/memory/jemalloc/moz.build b/memory/jemalloc/moz.build
> >--- a/memory/jemalloc/moz.build
> >+++ b/memory/jemalloc/moz.build
> >@@ -1,8 +1,39 @@
> > # -*- Mode: python; c-basic-offset: 4; indent-tabs-mode: nil; tab-width: 40 -*-
> > # vim: set filetype=python:
> > # 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/.
> > 
> > MODULE = 'jemalloc'
> > 
> >+CSRCS += [
> >+    'arena.c',
> >+    'atomic.c',
> >+    'base.c',
> >+    'bitmap.c',
> >+    'chunk.c',
> >+    'chunk_dss.c',
> >+    'chunk_mmap.c',
> >+    'ckh.c',
> >+    'ctl.c',
> >+    'extent.c',
> >+    'hash.c',
> >+    'huge.c',
> >+    'jemalloc.c',
> >+    'mb.c',
> >+    'mutex.c',
> >+    'prof.c',
> >+    'quarantine.c',
> >+    'rtree.c',
> >+    'stats.c',
> >+    'tcache.c',
> >+    'tsd.c',
> >+    'util.c',
> >+]
> >+
> >+# Only OSX needs the zone allocation implementation,
> >+# but only if replace-malloc is not enabled.
> >+if CONFIG['OS_TARGET'] == Darwin or CONFIG['MOZ_REPLACE_MALLOC'] == 'Darwin':
> >+    CSRCS += [
> >+        'zone.c',
> >+    ]
> 
> I don't think this if statement is correct. The first Darwin is not in
> quotes, so wouldn't that be a variable? I think this should be:
> 
> if CONFIG['OS_TARGET'] == 'Darwin' and not CONFIG['MOZ_REPLACE_MALLOC']:
>     CSRCS += [
>         'zone.c',
>     ]
> 
> (Please double check)

Hmmm, the file is not processed while using --enable-jemalloc but --enable-dmd was enough to display the error.  To be fixed shortly.

> 
> >diff --git a/memory/jemalloc/src/moz.build b/memory/jemalloc/src/moz.build
> >new file mode 100644
> >--- /dev/null
> >+++ b/memory/jemalloc/src/moz.build
> >@@ -0,0 +1,35 @@
>
> >+if CONFIG['ABI'] == 'macho':
> >+    CSRCS += [
> >+        'src/zone.c',
> >+    ]
> 
> Is ABI an actual config variable? It doesn't show up in my autoconf.mk, but
> maybe it is platform specific.

Will convert verbatim and open another bug to check on the ABI logic.  fyi> ABI=[@abi@] if searching config was not case insensitive.
 
> Everything else looks good to me - I can r+ when the if-statement is fixed.

ok

Comment 27

[Joey Armstrong [:joey]]

Attachment: move CSRCS to mozbuild (config batch #2)

Comment 28

[Joey Armstrong [:joey]]

Comment on attachment 757468 [details] [diff] [review]
move CSRCS to mozbuild (config batch #2)

Quote 'Darwin' in the jemalloc conditional.
Opened bug 870406 to verify ABI/config usage.

last try results: https://tbpl.mozilla.org/?tree=Try&rev=364948d0fee3

Comment 29

[Michael Shal [:mshal]]

Comment on attachment 757468 [details] [diff] [review]
move CSRCS to mozbuild (config batch #2)

>diff --git a/media/libcubeb/src/moz.build b/media/libcubeb/src/moz.build
>--- a/media/libcubeb/src/moz.build
>+++ b/media/libcubeb/src/moz.build
>@@ -1,8 +1,46 @@
> # -*- Mode: python; c-basic-offset: 4; indent-tabs-mode: nil; tab-width: 40 -*-
> # vim: set filetype=python:
> # 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/.
> 
> MODULE = 'cubeb'
> 
>+CSRCS += [
>+    'cubeb.c',
>+]
>+
>+if CONFIG['MOZ_ALSA']:
>+    CSRCS += [
>+        'cubeb_alsa.c',
>+    ]
>+
>+if CONFIG['MOZ_PULSEAUDIO']:
>+    CSRCS += [
>+        'cubeb_pulse.c',
>+    ]
>+
>+if CONFIG['OS_ARCH'] == 'OpenBSD':
>+    CSRCS += [
>+        'cubeb_sndio.c',
>+    ]
>+
>+

nit: Remove the extra line of whitespace

>+# Only OSX needs the zone allocation implementation,
>+# but only if replace-malloc is not enabled.
>+if CONFIG['OS_TARGET'] == 'Darwin' or CONFIG['MOZ_REPLACE_MALLOC'] == 'Darwin':
>+    CSRCS += [
>+        'zone.c',
>+    ]

This still looks wrong to me. MOZ_REPLACE_MALLOC won't get set to 'Darwin' - it is either '1' or not defined. I think you want:

if CONFIG['OS_TARGET'] == 'Darwin' and not CONFIG['MOZ_REPLACE_MALLOC']:

Once this is fixed I can r+.

Comment 30

[Joey Armstrong [:joey]]

Attachment: move CSRCS to mozbuild (config batch #2)

Comment 31

[Joey Armstrong [:joey]]

Comment on attachment 757539 [details] [diff] [review]
move CSRCS to mozbuild (config batch #2)

nit fixes

Comment 32

[Michael Shal [:mshal]]

Comment on attachment 757539 [details] [diff] [review]
move CSRCS to mozbuild (config batch #2)

Looks good to me now! If try is happy, I think it's ready.

Comment 33

[Joey Armstrong [:joey]]

Attachment: move CSRCS to mozbuild (config batch #2)

Comment 34

[Joey Armstrong [:joey]]

Comment on attachment 757943 [details] [diff] [review]
move CSRCS to mozbuild (config batch #2)

Same patch but removed memory/jemalloc/src/{Makefile.in,moz.build} per bug # 878866.
Earlier try job covered all but jemalloc/src::Darwin edit - http://tbpl.mozilla.org/?tree=Try&rev=364948d0fee3
r=mshal carried forward.

Comment 35

[Joey Armstrong [:joey]]

Comment on attachment 757943 [details] [diff] [review]
move CSRCS to mozbuild (config batch #2)

Inbound push
committed changeset 133925:e5b6545901b0
http://hg.mozilla.org/integration/mozilla-inbound/rev/e5b6545901b0

Comment 36

[Ryan VanderMeulen [:RyanVM]]

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

Comment 37

[Mark Banner (:standard8)]

Comment on attachment 754485 [details] [diff] [review]
Migrate CSRCS in comm-central

Looks fine to me, I can't think of any issues at the moment, as it all gets translated back into makefile anyway.

Comment 38

[Joshua Cranmer [:jcranmer]]

Comment on attachment 754485 [details] [diff] [review]
Migrate CSRCS in comm-central

https://hg.mozilla.org/comm-central/rev/0c54a5703d29

Comment 39

[Joey Armstrong [:joey]]

Attachment: move CSRCS to moz.build (file batch #3).

Comment 40

[Joey Armstrong [:joey]]

Attachment: move CSRCS to mozbuild (file batch #3)-stw

Comment 41

[Joey Armstrong [:joey]]

Attachment: move CSRCS to mozbuild (file batch #3).

Comment 42

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

Comment on attachment 770821 [details] [diff] [review]
move CSRCS to mozbuild (file batch #3).

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

Please fix the emptied if blocks in the makefiles?

::: tools/trace-malloc/moz.build
@@ +17,5 @@
> +simplecsrcs = [
> +    'leakstats.c',
> +    'tmstats.c',
> +]
> +        

Trailing ws

::: widget/gtk2/moz.build
@@ +61,5 @@
> +    CSRCS += [
> +        'maiRedundantObjectFactory.c',
> +    ]
> +
> +if CONFIG['NATIVE_THEME_SUPPORT']:

This is always defined in the makefile, and nowhere else.* Just remove the conditional.

[*] And it has been this way since bug 142334, http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/gfx/src/gtk/Attic&command=DIFF_FRAMESET&file=Makefile.in&rev2=1.86&rev1=1.85

@@ +66,5 @@
> +    if CONFIG['MOZ_ENABLE_GTK2']:
> +        CSRCS += [
> +            'gtk2drawing.c',
> +        ]
> +    elif CONFIG['MOZ_ENABLE_GTK2']:

This should be an else

Comment 43

[Joey Armstrong [:joey]]

(In reply to :Ms2ger from comment #42)
> Comment on attachment 770821 [details] [diff] [review]
> move CSRCS to mozbuild (file batch #3).
> 
> Review of attachment 770821 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: widget/gtk2/moz.build
> @@ +61,5 @@
> > +    CSRCS += [
> > +        'maiRedundantObjectFactory.c',
> > +    ]
> > +
> > +if CONFIG['NATIVE_THEME_SUPPORT']:
> 
> This is always defined in the makefile, and nowhere else.* Just remove the
> conditional.
> 
> [*] And it has been this way since bug 142334,
> http://bonsai.mozilla.org/cvsview2.
> cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/gfx/
> src/gtk/Attic&command=DIFF_FRAMESET&file=Makefile.in&rev2=1.86&rev1=1.85

These conversions are mostly automated and will be moving content over verbatim.
They are not attempting to interpret logic or optimize away logic.  That would be an exercise for after conversions are complete.

Also the patch was uploaded for safekeeping, it has not been assigned for review yet.

Comment 44

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

I'm aware; just pointing out a bug to avoid you having to look for it later.

Comment 45

[Joey Armstrong [:joey]]

Attachment: move CSRCS to mozbuild (file batch #3).

Comment 46

[Joey Armstrong [:joey]]

Attachment: move CSRCS to mozbuild (file batch #3)

Rebase, fixed conflict and stray whitespace insertion in security/manager/ssl/src/moz.build.

Try results with a lot of orange:
https://tbpl.mozilla.org/?tree=Try&rev=114204240591

Comment 47

[Michael Shal [:mshal]]

Comment on attachment 774704 [details] [diff] [review]
move CSRCS to mozbuild (file batch #3)

># HG changeset patch
># User Joey Armstrong <[e-mail]>
># Date 1372864769 14400
># Node ID 4fdd5bcd73000612a33a89339ada45d0ac3f65b9
># Parent 4dda210929fb56003dba4de87b19c6c5d5cb7d5a
>bug 870406: move CSRCS to mozbuild (file batch #3).
>
>diff --git a/mozglue/build/Makefile.in b/mozglue/build/Makefile.in
>--- a/mozglue/build/Makefile.in
>+++ b/mozglue/build/Makefile.in
>@@ -91,17 +91,16 @@ ifeq (android, $(MOZ_WIDGET_TOOLKIT))
> # Add Android specific code
> EXTRA_DSO_LDOPTS += $(MOZ_ZLIB_LIBS)
> SHARED_LIBRARY_LIBS += $(call EXPAND_LIBNAME_PATH,android,../android)
> # To properly wrap jemalloc's pthread_atfork call.
> EXTRA_DSO_LDOPTS += -Wl,--wrap=pthread_atfork
> endif
> 
> ifeq (gonk, $(MOZ_WIDGET_TOOLKIT))
>-CSRCS += cpuacct.c
> endif

The whole ifeq..endif block can go away.

>diff --git a/tools/trace-malloc/moz.build b/tools/trace-malloc/moz.build
>--- a/tools/trace-malloc/moz.build
>+++ b/tools/trace-malloc/moz.build
>@@ -1,13 +1,33 @@
> # -*- Mode: python; c-basic-offset: 4; indent-tabs-mode: nil; tab-width: 40 -*-
> # vim: set filetype=python:
> # 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/.
> 
>+extracsrcs = [
>+    'tmreader.c',
>+]
>+
>+progcsrcs = [
>+    'formdata.c',
>+    'spacecategory.c',
>+    'spacetrace.c',
>+]
>+
>+simplecsrcs = [
>+    'leakstats.c',
>+    'tmstats.c',
>+]
>+        

nit: the last blank line here has a bunch of space characters in it

> if not CONFIG['MOZ_PROFILE_GENERATE']:
>     PROGRAM = 'spacetrace'
>+    CSRCS += progcsrcs
> 
> CPP_SOURCES += [
>     '$(EXTRACPPSRCS)',
>     '$(SIMPLECPPSRCS)',
> ]
>+
>+CSRCS += extracsrcs
>+CSRCS += simplecsrcs
>+CSRCS.sort()

Why do we need to sort() here? I don't think it's necessary, so it should probably just go away.

>diff --git a/widget/gtk2/Makefile.in b/widget/gtk2/Makefile.in
>--- a/widget/gtk2/Makefile.in
>+++ b/widget/gtk2/Makefile.in
>@@ -18,32 +18,26 @@ endif
> 
> EXPORT_LIBRARY	= 1
> LIBXUL_LIBRARY = 1
> 
> NATIVE_THEME_SUPPORT = 1
> 
> 
> 
>-CSRCS		= \
>-		mozcontainer.c \
>-		$(NULL)
> 
> ifdef ACCESSIBILITY
>-CSRCS += maiRedundantObjectFactory.c
> endif

Please remove the whole ifdef block.

> # build our subdirs, too
> 
> SHARED_LIBRARY_LIBS = ../xpwidgets/libxpwidgets_s.a
> 
> ifdef NATIVE_THEME_SUPPORT
> ifdef MOZ_ENABLE_GTK2
>-CSRCS		+= gtk2drawing.c
> else
>-CSRCS		+= gtk3drawing.c
> endif

The whole ifdef MOZ_ENABLE_GTK2 block can go away too.

>diff --git a/widget/gtk2/moz.build b/widget/gtk2/moz.build
>--- a/widget/gtk2/moz.build
>+++ b/widget/gtk2/moz.build
>@@ -48,10 +48,26 @@ if CONFIG['NS_PRINTING']:
>     ]
> 
> if CONFIG['MOZ_X11']:
>     CPP_SOURCES += [
>         'nsClipboard.cpp',
>         'nsDragService.cpp',
>     ]
> 
>-CPP_SOURCES += [
>+CSRCS += [
>+    'mozcontainer.c',
> ]
>+
>+if CONFIG['ACCESSIBILITY']:
>+    CSRCS += [
>+        'maiRedundantObjectFactory.c',
>+    ]
>+
>+if CONFIG['NATIVE_THEME_SUPPORT']:

NATIVE_THEME_SUPPORT is not a CONFIG variable - it is a local variable in the Makefile.in that is hard-coded to 1. Unless there's a good reason to keep that around, I recommend just deleting NATIVE_THEME_SUPPORT.

>diff --git a/widget/gtkxtbin/Makefile.in b/widget/gtkxtbin/Makefile.in
>--- a/widget/gtkxtbin/Makefile.in
>+++ b/widget/gtkxtbin/Makefile.in
>@@ -10,19 +10,16 @@ srcdir		= @srcdir@
> VPATH		= @srcdir@
> 
> include $(DEPTH)/config/autoconf.mk
> 
> EXPORT_LIBRARY = 1
> LIBXUL_LIBRARY = 1
> 
> ifdef MOZ_ENABLE_GTK2
>-CSRCS		= \
>-		gtk2xtbin.c \
>-		$(NULL)
> endif

This ifdef block can go away.

This looks almost ready - the r- is primarily for the NATIVE_THEME_SUPPORT issue.

Comment 48

[Joey Armstrong [:joey]]

(In reply to Michael Shal [:mshal] from comment #47)
> Comment on attachment 774704 [details] [diff] [review]
> move CSRCS to mozbuild (file batch #3)
> 
> >+CSRCS += extracsrcs
> >+CSRCS += simplecsrcs
> >+CSRCS.sort()
> 
> Why do we need to sort() here? I don't think it's necessary, so it should
> probably just go away.

CSRCS has the StrictOrdering attribute, mach configure will fail in this directory w/o it.

Comment 49

[Joey Armstrong [:joey]]

(In reply to Michael Shal [:mshal] from comment #47)
> Comment on attachment 774704 [details] [diff] [review]
> move CSRCS to mozbuild (file batch #3)
> 
> >diff --git a/mozglue/build/Makefile.in b/mozglue/build/Makefile.in
> >--- a/mozglue/build/Makefile.in
> >+++ b/mozglue/build/Makefile.in
> > 
> > ifeq (gonk, $(MOZ_WIDGET_TOOLKIT))
> >-CSRCS += cpuacct.c
> > endif
> 
> The whole ifeq..endif block can go away.

The conversion script will need an edit to purge these automatically.  They are remnants of removing DISABLED_* tokens.

Comment 50

[Michael Shal [:mshal]]

(In reply to Joey Armstrong [:joey] from comment #48)
> (In reply to Michael Shal [:mshal] from comment #47)
> > Comment on attachment 774704 [details] [diff] [review]
> > move CSRCS to mozbuild (file batch #3)
> > 
> > >+CSRCS += extracsrcs
> > >+CSRCS += simplecsrcs
> > >+CSRCS.sort()
> > 
> > Why do we need to sort() here? I don't think it's necessary, so it should
> > probably just go away.
> 
> CSRCS has the StrictOrdering attribute, mach configure will fail in this
> directory w/o it.

That only applies to an individual += statement - so for example extrasrcs has to be sorted, but the whole CSRCS list does not. I just tried removing the CSRCS.sort() line and config.status ran correctly. If for some reason we needed CSRCS to be sorted, we could sort it in the mozbuild python code, rather than in each moz.build file.

Comment 51

[Joey Armstrong [:joey]]

(In reply to Michael Shal [:mshal] from comment #50)
> (In reply to Joey Armstrong [:joey] from comment #48)
> > (In reply to Michael Shal [:mshal] from comment #47)
> > > Comment on attachment 774704 [details] [diff] [review]
> > > move CSRCS to mozbuild (file batch #3)
> > > 
> > > >+CSRCS += extracsrcs
> > > >+CSRCS += simplecsrcs
> > > >+CSRCS.sort()
> > > 
> > > Why do we need to sort() here? I don't think it's necessary, so it should
> > > probably just go away.
> > 
> > CSRCS has the StrictOrdering attribute, mach configure will fail in this
> > directory w/o it.
> 
> That only applies to an individual += statement - so for example extrasrcs
> has to be sorted, but the whole CSRCS list does not. I just tried removing
> the CSRCS.sort() line and config.status ran correctly.

If the file addition was wrapped by a platform/toolbox/etc conditional you may not see it.
Some will only show up if you temporarily hack 'True' into the conditionals so everything will be evaluated.

> If for some reason we needed CSRCS to be sorted, we could sort it in the mozbuild python code, rather than in each moz.build file.

Won't an internal sort negate the reason the sort attribute was added ?  That being maintain mozbuild content in sorted order.  Maybe the answer is to add a disable-sort override, by default sorting is required but can be selectively disabled for special cases.

Comment 52

[Michael Shal [:mshal]]

(In reply to Joey Armstrong [:joey] from comment #51)
> (In reply to Michael Shal [:mshal] from comment #50)
> > (In reply to Joey Armstrong [:joey] from comment #48)
> > > (In reply to Michael Shal [:mshal] from comment #47)
> > > > Comment on attachment 774704 [details] [diff] [review]
> > > > move CSRCS to mozbuild (file batch #3)
> > > > 
> > > > >+CSRCS += extracsrcs
> > > > >+CSRCS += simplecsrcs
> > > > >+CSRCS.sort()
> > > > 
> > > > Why do we need to sort() here? I don't think it's necessary, so it should
> > > > probably just go away.
> > > 
> > > CSRCS has the StrictOrdering attribute, mach configure will fail in this
> > > directory w/o it.
> > 
> > That only applies to an individual += statement - so for example extrasrcs
> > has to be sorted, but the whole CSRCS list does not. I just tried removing
> > the CSRCS.sort() line and config.status ran correctly.
> 
> If the file addition was wrapped by a platform/toolbox/etc conditional you
> may not see it.
> Some will only show up if you temporarily hack 'True' into the conditionals
> so everything will be evaluated.
> 

Try it out for yourself. This is fine:

CSRCS += ['z.c']
CSRCS += ['a.c']

This is not:
CSRCS += ['z.c', 'a.c']

> > If for some reason we needed CSRCS to be sorted, we could sort it in the mozbuild python code, rather than in each moz.build file.
> 
> Won't an internal sort negate the reason the sort attribute was added ? 
> That being maintain mozbuild content in sorted order.  Maybe the answer is
> to add a disable-sort override, by default sorting is required but can be
> selectively disabled for special cases.

The point is that so in a giant list of files, there is an ordering to it. Making sure that CSRCS is sorted at the end of the file doesn't add any value.

Comment 53

[Joey Armstrong [:joey]]

(In reply to Michael Shal [:mshal] from comment #52)
>
> Try it out for yourself. This is fine:
> 
> CSRCS += ['z.c']
> CSRCS += ['a.c']
> 
> This is not:
> CSRCS += ['z.c', 'a.c']

Yes I know how the syntax/sorting works.  I'll see if I can reproduce the failure that forced adding .sort().

Comment 54

[Joey Armstrong [:joey]]

(In reply to Michael Shal [:mshal] from comment #52)
> Try it out for yourself. This is fine:
> 
> CSRCS += ['z.c']
> CSRCS += ['a.c']
> 
> This is not:
> CSRCS += ['z.c', 'a.c']

Ok I see where this is coming from now.  When local variables are used to accumulate values -- which is in play here because this conversion is dependent on a set of yet-to-be-converted variables.

If values are accumulated within a local variable.
That list is unsorted due to platform/toolkit conditionals { which can happen, mozbuild just does not complain about isolated assignments within the full list }.
Attempt to append local vars to the CSRCS var and it will croak.

If variables 'CSRCS' and 'csrcs' are used:

CSRCS += ['aa.c']
if CONFIG ['MOZ_WIDGET_TOOLKIT'] != 'android':
  CSRCS += ['mm.c']
CSRCS += ['rr.c'
if CONFIG['OS_ARCH'] != 'Darwin':
  CSRCS += ['jj.c']

The above succeeds for CSRCS.

Replace CSRCS with csrcs with the following assignment and mach will complain:
# CSRCS += csrcs

The error as reported by Python is:
  ['UnsortedError: An attempt was made to add an unsorted sequence to a list.  The incoming list is unsorted starting at element 1.  We expected "jj.mm" but got "mm.mm"\n']

So if anyone needs to perform local collection or filtering on the file list prior to assignment as I did with the unconverted variables the assignment will trigger a sort failure.

Comment 55

[Joey Armstrong [:joey]]

Attachment: move CSRCS to mozbuild (file batch #3).

Comment 56

[Joey Armstrong [:joey]]

Comment on attachment 776365 [details] [diff] [review]
move CSRCS to mozbuild (file batch #3).

nits fixed.
removed NATIVE_THEME_SUPPORT conditional.
removed local vars containing file lists.  We lose the breakdown of file by type but the sort problem goes away.

cleaning up conditionals around variables being deleted morphs automated conversions into a manual task.  Cleaned these up but opened a bug to add a script that would remove them automatically.  The script searching for empty makefiles should also support this logic.

Comment 57

[Michael Shal [:mshal]]

Comment on attachment 776365 [details] [diff] [review]
move CSRCS to mozbuild (file batch #3).

This looks good to me! We may need to cleanup tools/trace-malloc/moz.build once the rest of the variables are moved over (especially for my lazy CPP_SOURCES change there - blech). But for now this works.

Comment 58

[Joey Armstrong [:joey]]

Attachment: move CSRCS to mozbuild (file batch #3).

Comment 59

[Joey Armstrong [:joey]]

Comment on attachment 776581 [details] [diff] [review]
move CSRCS to mozbuild (file batch #3).

rebase, r=mshal carried forward

Comment 60

[Joey Armstrong [:joey]]

Inbound push:
committed changeset 138734:8bbd27688a89
https://hg.mozilla.org/integration/mozilla-inbound/rev/8bbd27688a89

Comment 61

[Joey Armstrong [:joey]]

Attachment: cleanup - purge DISABLED_CSRCS variables.

Comment 62

[Joey Armstrong [:joey]]

Attachment: cleanup - purge DISABLED_CSRCS variables.

Comment 63

[Ryan VanderMeulen [:RyanVM]]

Backed out for linux bustage.
http://hg.mozilla.org/integration/mozilla-inbound/rev/e97286839337

https://tbpl.mozilla.org/php/getParsedLog.php?id=25342807&tree=Mozilla-Inbound

Comment 64

[Joey Armstrong [:joey]]

Attachment: move CSRCS to mozbuild (file batch #3).

Comment 65

[Joey Armstrong [:joey]]

Attachment: move CSRCS to mozbuild - patch #4.

Comment 66

[Joey Armstrong [:joey]]

Comment on attachment 799767 [details] [diff] [review]
move CSRCS to mozbuild - patch #4.

try build results: http://tbpl.mozilla.org/?tree=Try&rev=d7019662c912

Comment 67

[Joey Armstrong [:joey]]

Comment on attachment 799767 [details] [diff] [review]
move CSRCS to mozbuild - patch #4.

Refresh patch, modules/zlib/src/Makefile.in can be deleted.
Build try results for the patch:
http://tbpl.mozilla.org/?tree=Try&rev=d7019662c912
A test job is on the way.

Comment 68

[Joey Armstrong [:joey]]

(In reply to Joey Armstrong [:joey] from comment #67)
> Comment on attachment 799767 [details] [diff] [review]
> move CSRCS to mozbuild - patch #4.
> 
> Refresh patch, modules/zlib/src/Makefile.in can be deleted.
> Build try results for the patch:
> http://tbpl.mozilla.org/?tree=Try&rev=d7019662c912
> A test job is on the way.


try-test: http://tbpl.mozilla.org/?tree=Try&rev=d14e75ee5b29

Comment 69

[Michael Shal [:mshal]]

Comment on attachment 799767 [details] [diff] [review]
move CSRCS to mozbuild - patch #4.

>diff --git a/build/unix/elfhack/inject/moz.build b/build/unix/elfhack/inject/moz.build
>--- a/build/unix/elfhack/inject/moz.build
>+++ b/build/unix/elfhack/inject/moz.build
>@@ -1,7 +1,20 @@
> # -*- Mode: python; c-basic-offset: 4; indent-tabs-mode: nil; tab-width: 40 -*-
> # vim: set filetype=python:
> # 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/.
> 
> NO_DIST_INSTALL = True
>+
>+
>+if CONFIG['TARGET_CPU'][-2:] == '86':
>+    cpu = 'x86'
>+elif CONFIG['TARGET_CPU'][:3] == 'arm':
>+    cpu = 'arm'

I think it would be more clear if we did:

if CONFIG['TARGET_CPU'].endswith('86'):
    cpu = 'x86'
elif CONFIG['TARGET_CPU'].startswith('arm'):
    cpu = 'arm'

Though I am not a python expert :)

>diff --git a/modules/zlib/src/moz.build b/modules/zlib/src/moz.build
>--- a/modules/zlib/src/moz.build
>+++ b/modules/zlib/src/moz.build
>@@ -9,8 +9,25 @@ MODULE = 'zlib'
> EXPORTS += [
>     'mozzconf.h',
>     'zconf.h',
>     'zlib.h',
> ]
> 
> LIBRARY_NAME = 'mozz'
> 
>+CSRCS += [
>+    'adler32.c',
>+    'compress.c',
>+    'crc32.c',
>+    'deflate.c',
>+    'gzclose.c',
>+    'gzlib.c',
>+    'gzread.c',
>+    'gzwrite.c',
>+    'infback.c',
>+    'inffast.c',
>+    'inflate.c',
>+    'inftrees.c',
>+    'trees.c',
>+    'uncompr.c',
>+    'zutil.c',
>+]

Can you also remove objs.mk now that all this info is in moz.build? It doesn't look like anything else in the tree includes it, and I'd rather not have a defunct list of source files in the tree. I can r+ once it is removed, or if you confirm it is still needed.

Comment 70

[Joey Armstrong [:joey]]

(In reply to Michael Shal [:mshal] from comment #69)
> Comment on attachment 799767 [details] [diff] [review]
> move CSRCS to mozbuild - patch #4.
> 
> >diff --git a/build/unix/elfhack/inject/moz.build b/build/unix/elfhack/inject/moz.build
> >--- a/build/unix/elfhack/inject/moz.build
> >+++ b/build/unix/elfhack/inject/moz.build
> >@@ -1,7 +1,20 @@
> > # -*- Mode: python; c-basic-offset: 4; indent-tabs-mode: nil; tab-width: 40 -*-
> > # vim: set filetype=python:
> > # 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/.
> > 
> > NO_DIST_INSTALL = True
> >+
> >+
> >+if CONFIG['TARGET_CPU'][-2:] == '86':
> >+    cpu = 'x86'
> >+elif CONFIG['TARGET_CPU'][:3] == 'arm':
> >+    cpu = 'arm'
> 
> I think it would be more clear if we did:
> 
> if CONFIG['TARGET_CPU'].endswith('86'):
>     cpu = 'x86'
> elif CONFIG['TARGET_CPU'].startswith('arm'):
>     cpu = 'arm'
> 
> Though I am not a python expert :)

I did have these tests in earlier but python was complaining that utf values were not a string.

Comment 71

[Joey Armstrong [:joey]]

patch #3 will need to be recreated, some vars were converted with bug 906619.

Comment 72

[Joey Armstrong [:joey]]

Attachment: move CSRCS to mozbuild (batch #3)

refresh patch #3

Comment 73

[Joey Armstrong [:joey]]

Attachment: move CSRCS to mozbuild - patch #4.

Comment 74

[Joey Armstrong [:joey]]

Comment on attachment 805985 [details] [diff] [review]
move CSRCS to mozbuild - patch #4.

obj.mk removed added string funcs since no complaints about utf encoding this time.  try run: http://tbpl.mozilla.org/?tree=Try&rev=d14e75ee5b29

Comment 75

[Michael Shal [:mshal]]

Comment on attachment 805985 [details] [diff] [review]
move CSRCS to mozbuild - patch #4.

Looks good to me!

Comment 76

[Joey Armstrong [:joey]]

Comment on attachment 802590 [details] [diff] [review]
move CSRCS to mozbuild (batch #3)

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

Re-try widget/gtk2 w/o that pesky NATIVE_THEME_SUPPORT dependency from Makefile.in

http://tbpl.mozilla.org/?tree=Try&rev=f9be0e50f943

Comment 77

[Joey Armstrong [:joey]]

Attachment: move CSRCS to mozbuild (file batch #3).

Comment 78

[Joey Armstrong [:joey]]

Comment on attachment 807931 [details] [diff] [review]
move CSRCS to mozbuild (file batch #3).

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

Finally, widget/gtk2 has a clean run on linux*.
move csrcs to mozbuild batch 3

Comment 79

[Joey Armstrong [:joey]]

Comment on attachment 805985 [details] [diff] [review]
move CSRCS to mozbuild - patch #4.

Inbound push: https://hg.mozilla.org/integration/mozilla-inbound/rev/cee18252461e

Comment 80

[Michael Shal [:mshal]]

Comment on attachment 807931 [details] [diff] [review]
move CSRCS to mozbuild (file batch #3).

>diff --git a/tools/trace-malloc/moz.build b/tools/trace-malloc/moz.build
>--- a/tools/trace-malloc/moz.build
>+++ b/tools/trace-malloc/moz.build
>@@ -1,28 +1,36 @@
> # -*- Mode: python; c-basic-offset: 4; indent-tabs-mode: nil; tab-width: 40 -*-
> # vim: set filetype=python:
> # 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/.
> 
> if not CONFIG['MOZ_PROFILE_GENERATE']:
>     PROGRAM = 'spacetrace'
>+    CSRCS += [
>+        'formdata.c',
>+        'spacecategory.c',
>+        'spacetrace.c',
>+    ]
> 
> bin_suffix = CONFIG['BIN_SUFFIX']
> 
> 
> simple_c_sources = [
>     'leakstats',
>     'tmstats',
> ]
> 
> CSRCS += [
>     '%s.c' % s for s in simple_c_sources
> ]
>+CSRCS += [
>+    'tmreader.c', # EXTRACSRCS

Can we get rid of the # EXTRACSRCS comment? I don't think it tells us anything useful.

Comment 81

[Joey Armstrong [:joey]]

Attachment: move CSRCS to mozbuild (file batch #3).

Comment 82

[Joey Armstrong [:joey]]

Comment on attachment 807972 [details] [diff] [review]
move CSRCS to mozbuild (file batch #3).

prune the comment about source variable from Makefile.in.
r=mshal carried forward

Comment 83

[Joey Armstrong [:joey]]

Comment on attachment 807972 [details] [diff] [review]
move CSRCS to mozbuild (file batch #3).

Inbound push for patch 3: https://hg.mozilla.org/integration/mozilla-inbound/rev/bfc40b8abf4c

Comment 84

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/cee18252461e
https://hg.mozilla.org/mozilla-central/rev/bfc40b8abf4c

Comment 85

[Joey Armstrong [:joey]]

Attachment: move CSRCS to mozbuild (batch #5)

Comment 86

[Joey Armstrong [:joey]]

Attachment: move CSRCS to mozbuild (stlport)

Comment 87

[Joey Armstrong [:joey]]

Comment on attachment 809239 [details] [diff] [review]
move CSRCS to mozbuild (stlport)

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

1) move CSRCS to mozbuild, try job-http://tbpl.mozilla.org/?tree=Try&rev=44ea5b5e31c6
2) Expand CPP_SOURCES makefile macro while we are in here.

Comment 88

[Michael Shal [:mshal]]

Comment on attachment 809239 [details] [diff] [review]
move CSRCS to mozbuild (stlport)

Looks good - thanks for cleaning up CPP_SOURCES as well! Any idea what is up with the try results? Windows is having a bad day, though it doesn't look like it's related to your changes.

Comment 89

[Mike Hommey [:glandium]]

Attachment: part n - Move more CSRCS to moz.build

https://tbpl.mozilla.org/?tree=Try&rev=eb45f9b55273

This unbitrots the remaining bits from here, refactor some stuff for cairo, and add a few more moves. With this applied, the only remaining CSRCS in the tree require more stuff and would each need their own bug imho, so I'm all for closing this bug once this patch lands.

Comment 90

[Mike Hommey [:glandium]]

Attachment: part n - Move more CSRCS to moz.build

https://tbpl.mozilla.org/?tree=Try&rev=443008c77360

(Previous try had a problem on another patch, and a problem in the ordering of one of the SOURCES in this patch)

Comment 91

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

Comment on attachment 823884 [details] [diff] [review]
part n - Move more CSRCS to moz.build

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

::: gfx/cairo/cairo/src/moz.build
@@ +16,5 @@
>      'cairo-version.h',
>      'cairo.h',
>  ]
>  
> +if CONFIG['MOZ_WIDGET_TOOLKIT'] not in ('cocoa', 'uikit'):

Where does uikit come from?

::: gfx/ots/src/Makefile.in
@@ +25,5 @@
>  #
>  
>  
>  ifeq (WINNT,$(OS_TARGET))
>  endif

Feel free to remove this too

::: xpcom/base/moz.build
@@ +67,5 @@
>      EXPORTS += [
>          'nsWindowsHelpers.h',
>      ]
> +    if CONFIG['MOZ_DEBUG']:
> +        EXPORTS += ['pure.h']

Makes sense, I guess

Comment 92

[Mike Hommey [:glandium]]

(In reply to :Ms2ger from comment #91)
> > +if CONFIG['MOZ_WIDGET_TOOLKIT'] not in ('cocoa', 'uikit'):
> 
> Where does uikit come from?

From configure.in. It probably doesn't work anymore, but meh.

Comment 93

[Mike Hommey [:glandium]]

Attachment: part n - Move more CSRCS to moz.build

With various fixups. Crossing fingers.

https://tbpl.mozilla.org/?tree=Try&rev=520fd6a9e27b

Comment 94

[(not currently active) Ted Mielczarek]

(In reply to Mike Hommey [:glandium] from comment #92)
> > Where does uikit come from?
> 
> From configure.in. It probably doesn't work anymore, but meh.

It's never fully worked, but I haven't ruled out getting back to that port.

Comment 95

[Michael Shal [:mshal]]

Comment on attachment 823938 [details] [diff] [review]
part n - Move more CSRCS to moz.build

>diff --git a/build/stlport/moz.build b/build/stlport/moz.build
>--- a/build/stlport/moz.build
>+++ b/build/stlport/moz.build
>@@ -2,8 +2,46 @@
> # vim: set filetype=python:
> # 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/.
> 
> LIBRARY_NAME = 'stlport_static'
> 
> FORCE_STATIC_LIB = True
>+
>+SOURCES += [
>+    'src/allocators.cpp',
>+    'src/bitset.cpp',
>+    'src/codecvt.cpp',
>+    'src/collate.cpp',
>+    'src/complex.cpp',
>+    'src/complex_io.cpp',
>+    'src/complex_trig.cpp',
>+    'src/ctype.cpp',
>+    'src/dll_main.cpp',
>+    'src/facets_byname.cpp',
>+    'src/fstream.cpp',
>+    'src/ios.cpp',
>+    'src/iostream.cpp',
>+    'src/istream.cpp',
>+    'src/locale.cpp',
>+    'src/locale_catalog.cpp',
>+    'src/locale_impl.cpp',
>+    'src/messages.cpp',
>+    'src/monetary.cpp',
>+    'src/num_get.cpp',
>+    'src/num_get_float.cpp',
>+    'src/num_put.cpp',
>+    'src/num_put_float.cpp',
>+    'src/numpunct.cpp',
>+    'src/ostream.cpp',
>+    'src/sstream.cpp',
>+    'src/stdio_streambuf.cpp',
>+    'src/string.cpp',
>+    'src/strstream.cpp',
>+    'src/time_facets.cpp',
>+]
>+
>+SOURCES += [
>+    'src/c_locale.c',
>+    'src/cxa.c',
>+]

Any reason not to combine these SOURCES blocks?

Everyone else looks good to me.

Comment 96

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/564210bf70b3

Comment 97

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/564210bf70b3

Comment 98

[Octoploid]

"part n - Move more CSRCS to moz.build" breaks the build for me:

/usr/lib/gcc/x86_64-pc-linux-gnu/4.8.3/../../../../x86_64-pc-linux-gnu/bin/ld: error: /var/tmp/moz-build-dir/toolkit/library/../../ipc/chromium/event.o: requires dynamic R_X8
6_64_PC32 reloc against 'eventfd' which may overflow at runtime; recompile with -fPIC
/usr/lib/gcc/x86_64-pc-linux-gnu/4.8.3/../../../../x86_64-pc-linux-gnu/bin/ld: error: read-only segment has dynamic relocations
/var/tmp/moz-build-dir/toolkit/library/../../ipc/chromium/event.o:event.c:function evthread_make_base_notifiable.part.27: error: undefined reference to 'eventfd'
collect2: error: ld returned 1 exit status
make[5]: *** [libxul.so] Error 1

Comment 99

[Mike Hommey [:glandium]]

(In reply to Octoploid from comment #98)
> "part n - Move more CSRCS to moz.build" breaks the build for me:
> 
> /usr/lib/gcc/x86_64-pc-linux-gnu/4.8.3/../../../../x86_64-pc-linux-gnu/bin/
> ld: error:
> /var/tmp/moz-build-dir/toolkit/library/../../ipc/chromium/event.o: requires
> dynamic R_X8
> 6_64_PC32 reloc against 'eventfd' which may overflow at runtime; recompile
> with -fPIC
> /usr/lib/gcc/x86_64-pc-linux-gnu/4.8.3/../../../../x86_64-pc-linux-gnu/bin/
> ld: error: read-only segment has dynamic relocations
> /var/tmp/moz-build-dir/toolkit/library/../../ipc/chromium/event.o:event.c:
> function evthread_make_base_notifiable.part.27: error: undefined reference
> to 'eventfd'
> collect2: error: ld returned 1 exit status
> make[5]: *** [libxul.so] Error 1

bug 933672?

Comment 100

[Octoploid]

(In reply to Mike Hommey [:glandium] from comment #99)
> bug 933672?

Oops, yes.