882908 - Move OS_LDFLAGS to moz.build

Status: NEW

(Firefox Build System :: General, defect)

Comment 1

[Joey Armstrong [:joey]]

Attachment: move OS_LIBS to mozbuild (logic).

Comment 2

[Joey Armstrong [:joey]]

Comment on attachment 776572 [details] [diff] [review]
move OS_LIBS to mozbuild (logic).

Added a passthrough conversion for OS_LIBS.  May not be able to cover $(call expansions but we can start converting the simple assignments.

Comment 3

[Gregory Szorc [:gps]]

Comment on attachment 776572 [details] [diff] [review]
move OS_LIBS to mozbuild (logic).

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

::: python/mozbuild/mozbuild/test/backend/test_recursivemake.py
@@ +187,5 @@
>                  'HOST_LIBRARY_NAME := host_bar',
>              ],
>              'LIBRARY_NAME': [
>                  'LIBRARY_NAME := lib_name',
> +            ], 

Trailing whitespace.

Comment 4

[Joey Armstrong [:joey]]

Attachment: move OS_LIBS to mozbuild (logic).

Comment 5

[Joey Armstrong [:joey]]

Comment on attachment 777963 [details] [diff] [review]
move OS_LIBS to mozbuild (logic).

nit cleanup: fixed whitespace  r=gps carried forward

Comment 6

[Joey Armstrong [:joey]]

lingering references.


./gfx/angle/src/libGLESv2/Makefile.in:OS_LIBS += -ld3d9 -ldxguid
./gfx/angle/src/libEGL/Makefile.in:#OS_LIBS += $(call EXPAND_LIBNAME,dwmapi)
./gfx/angle/src/libEGL/Makefile.in:OS_LIBS += -ld3d9 -llibGLESv2
./js/src/Makefile.in:# - OS_LIBS includes libraries selected by the configure script.
./js/src/Makefile.in:JS_CONFIG_LIBS=$(EXTRA_DSO_LDOPTS) $(OS_LIBS) $(EXTRA_LIBS)
./layout/media/Makefile.in:OS_LIBS += $(call EXPAND_LIBNAME,usp10 ole32)
./layout/media/Makefile.in:OS_LIBS += $(call EXPAND_LIBNAME,secur32 crypt32 iphlpapi strmiids dmoguids wmcodecdspuuid amstrmid msdmo wininet)
./layout/media/Makefile.in:OS_LIBS += $(call EXPAND_LIBNAME,delayimp)
./layout/media/Makefile.in:OS_LIBS += $(call EXPAND_LIBNAME, msimg32)
./layout/media/Makefile.in:OS_LIBS += $(call EXPAND_LIBNAME, winmm)
./accessible/public/msaa/Makefile.in:OS_LIBS = $(call EXPAND_LIBNAME,kernel32 rpcns4 rpcrt4 oleaut32)
./accessible/public/ia2/Makefile.in:OS_LIBS = $(call EXPAND_LIBNAME,uuid kernel32 rpcns4 rpcrt4 ole32 oleaut32)
./xpcom/tests/windows/Makefile.in:OS_LIBS       += $(call EXPAND_LIBNAME,rpcrt4 uuid)
./testing/tools/screenshot/Makefile.in:OS_LIBS = $(TK_LIBS) $(XSS_LIBS)
./testing/tools/screenshot/Makefile.in:OS_LIBS += $(call EXPAND_LIBNAME,gdiplus)
./xulrunner/stub/Makefile.in:OS_LIBS += $(call EXPAND_LIBNAME,shell32)
./xulrunner/app/Makefile.in:OS_LIBS += $(call EXPAND_LIBNAME,comctl32 comdlg32 uuid shell32 ole32 oleaut32 version winspool)
./memory/replace/dmd/Makefile.in:OS_LIBS         += $(call EXPAND_LIBNAME,dbghelp)
./toolkit/mozapps/update/updater/Makefile.in:OS_LIBS += $(call EXPAND_LIBNAME,delayimp comctl32 ws2_32 shell32 shlwapi)
./toolkit/mozapps/update/updater/Makefile.in:OS_LIBS += $(TK_LIBS)
./toolkit/mozapps/update/updater/Makefile.in:OS_LIBS += -framework Cocoa
./toolkit/mozapps/update/updater/Makefile.in:OS_LIBS += -lcutils -lsysutils
./toolkit/library/Makefile.in:OS_LIBS += -ldbus
./toolkit/library/Makefile.in:OS_LIBS += -lstagefright -lstagefright_omx
./toolkit/library/Makefile.in:OS_LIBS += -lrt
./toolkit/library/Makefile.in:OS_LIBS += -framework OpenGL -lcups
./toolkit/library/Makefile.in:OS_LIBS += $(call EXPAND_LIBNAME,secur32 crypt32 iphlpapi strmiids dmoguids wmcodecdspuuid amstrmid msdmo wininet)
./toolkit/library/Makefile.in:OS_LIBS += -lGLESv2
./toolkit/library/Makefile.in:OS_LIBS += \
./toolkit/library/Makefile.in:OS_LIBS += $(call EXPAND_LIBNAME,mfuuid wmcodecdspuuid strmiids)
./toolkit/library/Makefile.in:OS_LIBS += $(call EXPAND_LIBNAME,dmoguids wmcodecdspuuid strmiids msdmo)
./toolkit/library/Makefile.in:OS_LIBS += \
./toolkit/library/Makefile.in:OS_LIBS += $(call EXPAND_LIBNAME,kvm)
./toolkit/library/Makefile.in:OS_LIBS += $(call EXPAND_LIBNAME,shell32 ole32 version winspool comdlg32 imm32 msimg32 shlwapi psapi ws2_32 dbghelp rasapi32 rasdlg iphlpapi uxtheme setupapi secur32 sensorsapi portabledeviceguids windowscodecs wininet)
./toolkit/library/Makefile.in:OS_LIBS += $(call EXPAND_LIBNAME,oleacc)
./toolkit/library/Makefile.in:OS_LIBS += $(call EXPAND_LIBNAME,uiautomationcore runtimeobject)
./toolkit/library/Makefile.in:OS_LIBS += $(call EXPAND_LIBNAME,delayimp)
./toolkit/library/Makefile.in:OS_LIBS += \
./toolkit/library/Makefile.in:OS_LIBS += $(call EXPAND_LIBNAME,dxguid dinput8)
./toolkit/library/Makefile.in:OS_LIBS += $(LIBICONV)
./toolkit/library/Makefile.in:OS_LIBS += $(call EXPAND_LIBNAME,usp10 oleaut32)
./toolkit/crashreporter/client/Makefile.in:OS_LIBS += $(call EXPAND_LIBNAME,comctl32 shell32 wininet shlwapi)
./toolkit/crashreporter/client/Makefile.in:OS_LIBS += -framework Cocoa
./toolkit/crashreporter/client/Makefile.in:OS_LIBS += $(TK_LIBS) $(MOZ_GTHREAD_LIBS)
./toolkit/crashreporter/client/Makefile.in:OS_LIBS += $(MOZ_GTK2_LIBS) $(MOZ_GTHREAD_LIBS)
./toolkit/components/maintenanceservice/Makefile.in:OS_LIBS += $(call EXPAND_LIBNAME,comctl32 ws2_32 shell32)
./toolkit/xre/test/win/Makefile.in:OS_LIBS += $(call EXPAND_LIBNAME,comctl32 ws2_32 shell32)
./webapprt/win/Makefile.in:OS_LIBS += $(call EXPAND_LIBNAME,shell32)
./b2g/app/Makefile.in:OS_LIBS += $(call EXPAND_LIBNAME,version)
./intl/uconv/tools/Makefile.in:OS_LIBS          += /usr/lib/libdl.so
./embedding/tests/winEmbed/Makefile.in:OS_LIBS          += $(call EXPAND_LIBNAME,ole32 comdlg32 shell32 version)
./tools/jprof/Makefile.in:OS_LIBS = \
./dom/plugins/ipc/hangui/Makefile.in:OS_LIBS = $(call EXPAND_LIBNAME,comctl32)
./browser/metro/shell/testing/Makefile.in:OS_LIBS = \
./browser/metro/shell/commandexecutehandler/Makefile.in:OS_LIBS = \
./browser/metro/shell/linktool/Makefile.in:OS_LIBS = \
./browser/components/build/Makefile.in:OS_LIBS  += $(call EXPAND_LIBNAME,ole32 shell32 shlwapi)
./browser/components/build/Makefile.in:OS_LIBS += $(call EXPAND_LIBNAME,version)

Comment 7

[Joey Armstrong [:joey]]

Attachment: move OS_LIBS to mozbuild (logic).

Comment 8

[Joey Armstrong [:joey]]

Comment on attachment 803228 [details] [diff] [review]
move OS_LIBS to mozbuild (logic).

refresh the patch for inbound checkin.  r=gps carried forward.

Comment 9

[Joey Armstrong [:joey]]

Inbound push: https://hg.mozilla.org/integration/mozilla-inbound/rev/e9ee43da0c16
committed changeset 146625:e9ee43da0c16

Comment 10

[Ed Morley [:emorley]]

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

Comment 11

[Joey Armstrong [:joey]]

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

if sys.platform == 'win32'
  from win32file import RemoveDirectory, DeleteFile,

ImportError: No module named win32file

slave: tst-w64-ec2-001

Comment 12

[Joey Armstrong [:joey]]

Attachment: move OS_LIBS to mozbuild (ffile batch #1)

Comment 13

[Joey Armstrong [:joey]]

try-win64 debug build: http://tbpl.mozilla.org/?tree=Try&rev=437581a320e9

Comment 14

[Joey Armstrong [:joey]]

Attachment: move OS_LIBS to mozbuild (ffile batch #1)

Comment 15

[Joey Armstrong [:joey]]

Comment on attachment 807925 [details] [diff] [review]
move OS_LIBS to mozbuild (ffile batch #1)

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

move os_libs to mozbuild, batch #1

Comment 16

[Joey Armstrong [:joey]]

Attachment: move OS_LIBS to mozbuild (batch #2).

Comment 17

[Joey Armstrong [:joey]]

Attachment: move OS_LIBS to mozbuild (batch #2).

Comment 18

[Michael Shal [:mshal]]

Comment on attachment 807925 [details] [diff] [review]
move OS_LIBS to mozbuild (ffile batch #1)

># HG changeset patch
># User Joey Armstrong <[e-mail]>
># Date 1379010577 14400
># Node ID 9a232471077716841bbcda2f983579b9e87d5722
># Parent  8ef53ff515b87d3293c8582f9ffdc35adc24fcde
>bug 882908: move OS_LIBS to mozbuild (ffile batch #1)
>
>diff --git a/browser/metro/shell/commandexecutehandler/moz.build b/browser/metro/shell/commandexecutehandler/moz.build
>--- a/browser/metro/shell/commandexecutehandler/moz.build
>+++ b/browser/metro/shell/commandexecutehandler/moz.build
>@@ -5,8 +5,18 @@
> # file, You can obtain one at http://mozilla.org/MPL/2.0/.
> 
> PROGRAM = 'CommandExecuteHandler'
> 
> CPP_SOURCES += [
>     'CEHHelper.cpp',
>     'CommandExecuteHandler.cpp',
> ]
>+
>+OS_LIBS += [
>+    'advapi32.lib',
>+    'kernel32.lib',
>+    'ole32.lib',
>+    'propsys.lib',
>+    'shlwapi.lib',
>+    'user32.lib',
>+    'wininet.lib',
>+]

Looks like you forgot to remove these from browser/metro/shell/commandexecutehandler/Makefile.in?

>diff --git a/gfx/angle/src/libEGL/Makefile.in b/gfx/angle/src/libEGL/Makefile.in
>--- a/gfx/angle/src/libEGL/Makefile.in
>+++ b/gfx/angle/src/libEGL/Makefile.in
>@@ -43,22 +43,19 @@ LOCAL_INCLUDES = \
> VPATH += $(srcdir)/../common
> # src/libEGL:
> VPATH += $(srcdir)/../libEGL
> DEFFILE = $(srcdir)/libEGL.def
> RCFILE  = $(srcdir)/libEGL.rc
> 
> include $(topsrcdir)/config/rules.mk
> 
>-#OS_LIBS += $(call EXPAND_LIBNAME,dwmapi)
>-
> ifdef GNU_CC
> 
> OS_CXXFLAGS := $(filter-out -fno-exceptions,$(OS_CXXFLAGS)) -fexceptions
>-OS_LIBS += -ld3d9 -llibGLESv2
> 
> else
> 
> EXTRA_DSO_LDOPTS = "$(MOZ_DIRECTX_SDK_PATH)/lib/$(MOZ_DIRECTX_SDK_CPU_SUFFIX)/d3d9.lib" \
>                    "$(DIST)/lib/libGLESv2.lib" \
>                    delayimp.lib
> 
> endif
>diff --git a/gfx/angle/src/libEGL/moz.build b/gfx/angle/src/libEGL/moz.build
>--- a/gfx/angle/src/libEGL/moz.build
>+++ b/gfx/angle/src/libEGL/moz.build
>@@ -19,8 +19,14 @@ CPP_SOURCES += [
>     'Config.cpp',
>     'Display.cpp',
>     'libEGL.cpp',
>     'main.cpp',
>     'Surface.cpp',
> ]
> 
> LIBRARY_NAME = 'libEGL'
>+
>+if CONFIG['OS_LIBS']:
>+    OS_LIBS += [
>+        '-ld3d9',
>+        '-llibGLESv2',
>+    ]

The Makefile.in has 'ifdef GNU_CC', but you have CONFIG['OS_LIBS'] here. Should be CONFIG['GNU_CC'], I think?

>diff --git a/intl/uconv/tools/moz.build b/intl/uconv/tools/moz.build
>--- a/intl/uconv/tools/moz.build
>+++ b/intl/uconv/tools/moz.build
>@@ -12,8 +12,12 @@ CSRCS += [
>     '%s.c' % s for s in sources
> ]
> 
> bin_suffix = CONFIG['BIN_SUFFIX']
> SIMPLE_PROGRAMS += [
>     '%s%s' % (s, bin_suffix) for s in sources
> ]
> 
>+if CONFIG['OS_ARCH'] is 'Linux' && CONFIG['OS_RELEASE'] == '1.2':

Why do you use 'is' for the first comparison and '==' for the second comparison? They should both be '==', based on my reading (or I'm misunderstanding what you're trying to do here :).

Comment 19

[Joey Armstrong [:joey]]

(In reply to Michael Shal [:mshal] from comment #18)
> Comment on attachment 807925 [details] [diff] [review]
> move OS_LIBS to mozbuild (ffile batch #1)

> Looks like you forgot to remove these from
> browser/metro/shell/commandexecutehandler/Makefile.in?

Heh time to sign off, this is an excellent flag to hint that you may have been starting at mozbuild patches for far too long over the course of one day.

Comment 20

[Michael Shal [:mshal]]

Note: We'll want to figure out a way to avoid using $(call EXPAND_LIBNAME) when setting OS_LIBS in moz.build. Since it is currently passthrough, some moz.build files currently set OS_LIBS using the make syntax (eg: chromium-config.mozbuild when bug 928709 lands)

Comment 21

[Trevor Saunders (:tbsaunde)]

(In reply to Michael Shal [:mshal] from comment #20)
> Note: We'll want to figure out a way to avoid using $(call EXPAND_LIBNAME)
> when setting OS_LIBS in moz.build. Since it is currently passthrough, some

I'd propose that OS_LIBS be made a list where we call expand_libname on each element.

> moz.build files currently set OS_LIBS using the make syntax (eg:
> chromium-config.mozbuild when bug 928709 lands)

O.O why is it doing that? I'm pretty sure almost all if not all the places that include chromium-config.mk don't actually need OS_LIBS for anything (it would be nice if we added more validation of "you set this in a moz.build file but that's useless".

Comment 22

[Mike Hommey [:glandium]]

See https://hg.mozilla.org/integration/mozilla-inbound/rev/a0988e587a90 for the semantics.

Comment 23

[(not currently active) Ted Mielczarek]

OS_LIBS and EXTRA_LIBS seem to be gone from Makefile.in:
https://dxr.mozilla.org/mozilla-central/search?tree=mozilla-central&q=OS_LIBS%20path%3AMakefile.in&redirect=true
https://dxr.mozilla.org/mozilla-central/search?q=EXTRA_LIBS+path%3AMakefile.in&redirect=false&case=false (just NSPR there)

OS_LDFLAGS has just a handful of uses:
https://dxr.mozilla.org/mozilla-central/search?q=OS_LDFLAGS+path%3AMakefile.in&redirect=true&case=false

Of the remaining three uses, two are to add `-Wl,-version-script`, so we could probably add something to moz.build to handle that. The clang-plugin thing probably wants a special case in moz.build to do what it's doing.