bug 1255485 - build PROGRAMs directly in dist/bin instead of copying them. r=nalexander

Historically we built all our binaries in directories in the objdir, then
symlinked them into dist/bin. Some binaries needed to be copied instead
so that certain relative path lookups work properly, so we resorted to
sprinkling `NSDISTMODE=copy` around Makefiles.

This change makes it so we build PROGRAMs (not any other sort of targets)
directly in dist/bin instead. We could do the same for our other targets
with a little more work.

There were several places in the tree that were copying built binaries to
some other place and needed fixup to match the new location of binaries.

On Windows pdb files are left in the objdir where the program was
originally linked. symbolstore.py needs to locate the pdb file both to
determine whether it should dump symbols for a binary and also to copy
the pdb file into the symbol package. We fix this by simply looking for
the pdb file in the current working directory if it isn't present next
to the binary, which matches how we invoke symbolstore.py.

MozReview-Commit-ID: 8TOD1uTXD5e

browser/app/Makefile.in

@@ -46,7 +46,7 @@ ifneq (,$(filter-out WINNT,$(OS_ARCH)))
 
 ifdef COMPILE_ENVIRONMENT
 libs::
-	cp -p $(MOZ_APP_NAME)$(BIN_SUFFIX) $(DIST)/bin/$(MOZ_APP_NAME)-bin$(BIN_SUFFIX)
+	cp -p $(DIST)/bin/$(MOZ_APP_NAME)$(BIN_SUFFIX) $(DIST)/bin/$(MOZ_APP_NAME)-bin$(BIN_SUFFIX)
 endif
 
 GARBAGE += $(addprefix $(FINAL_TARGET)/defaults/pref/, firefox.js)

config/makefiles/target_binaries.mk

@@ -7,8 +7,8 @@
 
 ifndef NO_DIST_INSTALL
 
-ifneq (,$(strip $(PROGRAM)$(SIMPLE_PROGRAMS)$(RUST_PROGRAMS)))
-PROGRAMS_EXECUTABLES = $(SIMPLE_PROGRAMS) $(PROGRAM) $(RUST_PROGRAMS)
+ifneq (,$(strip $(SIMPLE_PROGRAMS)$(RUST_PROGRAMS)))
+PROGRAMS_EXECUTABLES = $(SIMPLE_PROGRAMS) $(RUST_PROGRAMS)
 PROGRAMS_DEST ?= $(FINAL_TARGET)
 PROGRAMS_TARGET := target
 INSTALL_TARGETS += PROGRAMS

config/rules.mk

@@ -552,7 +552,7 @@ endef
 # PROGRAM = Foo
 # creates OBJS, links with LIBS to create Foo
 #
-$(PROGRAM): $(PROGOBJS) $(STATIC_LIBS_DEPS) $(EXTRA_DEPS) $(RESFILE) $(GLOBAL_DEPS)
+$(PROGRAM): $(PROGOBJS) $(STATIC_LIBS_DEPS) $(EXTRA_DEPS) $(RESFILE) $(GLOBAL_DEPS) $(call mkdir_deps,$(FINAL_TARGET))
 	$(REPORT_BUILD)
 	@$(RM) $@.manifest
 ifeq (_WINNT,$(GNU_CC)_$(OS_ARCH))
@@ -817,7 +817,7 @@ endif
 endif
 
 ifdef MOZ_CRASHREPORTER
-$(foreach file,$(DUMP_SYMS_TARGETS),$(eval $(call syms_template,$(file),$(file)_syms.track)))
+$(foreach file,$(DUMP_SYMS_TARGETS),$(eval $(call syms_template,$(file),$(notdir $(file))_syms.track)))
 else ifneq (,$(and $(LLVM_SYMBOLIZER),$(filter WINNT,$(OS_ARCH)),$(MOZ_AUTOMATION)))
 $(foreach file,$(DUMP_SYMS_TARGETS),$(eval $(call syms_template,$(file),$(addsuffix .pdb,$(basename $(file))))))
 PDB_FILES = $(addsuffix .pdb,$(basename $(DUMP_SYMS_TARGETS)))

js/src/shell/moz.build

@@ -64,4 +64,4 @@ FINAL_TARGET_PP_FILES += ['js-gdb.py.in']
 OBJDIR_FILES.js.src.shell += ['!/dist/bin/js-gdb.py']
 
 # People expect the js shell to wind up in the top-level JS dir.
-OBJDIR_FILES.js.src += ['!js%s' % CONFIG['BIN_SUFFIX']]
+OBJDIR_FILES.js.src += ['!/dist/bin/js%s' % CONFIG['BIN_SUFFIX']]

modules/libmar/tests/moz.build

@@ -8,5 +8,5 @@ XPCSHELL_TESTS_MANIFESTS += ['unit/xpcshell.ini']
 
 if CONFIG['OS_TARGET'] != 'Android':
     TEST_HARNESS_FILES.xpcshell.modules.libmar.tests.unit += [
-        '!../tool/signmar%s' % CONFIG['BIN_SUFFIX'],
+        '!/dist/bin/signmar%s' % CONFIG['BIN_SUFFIX'],
     ]

python/mozbuild/mozbuild/backend/recursivemake.py

@@ -1129,7 +1129,7 @@ class RecursiveMakeBackend(CommonBackend):
         ))
 
     def _process_program(self, obj, backend_file):
-        backend_file.write('PROGRAM = %s\n' % obj.program)
+        backend_file.write('PROGRAM = %s\n' % self._pretty_path(obj.output_path, backend_file))
         if not obj.cxx_link and not self.environment.bin_suffix:
             backend_file.write('PROG_IS_C_ONLY_%s := 1\n' % obj.program)
 

python/mozbuild/mozbuild/frontend/data.py

@@ -17,6 +17,7 @@ structures.
 
 from __future__ import absolute_import, unicode_literals
 
+from mozbuild.frontend.context import ObjDirPath
 from mozbuild.util import StrictOrderingOnAppendList
 from mozpack.chrome.manifest import ManifestEntry
 
@@ -85,6 +86,10 @@ class ContextDerived(TreeMetadata):
     def install_target(self):
         return self._context['FINAL_TARGET']
 
+    @property
+    def installed(self):
+        return self._context['DIST_INSTALL'] is not False
+
     @property
     def defines(self):
         defines = self._context['DEFINES']
@@ -472,6 +477,13 @@ class BaseProgram(Linkable):
         self.program = program
         self.is_unit_test = is_unit_test
 
+    @property
+    def output_path(self):
+        if self.installed:
+            return ObjDirPath(self._context, '!/' + mozpath.join(self.install_target, self.program))
+        else:
+            return ObjDirPath(self._context, '!' + self.program)
+
     def __repr__(self):
         return '<%s: %s/%s>' % (type(self).__name__, self.relobjdir, self.program)
 

python/mozbuild/mozbuild/frontend/emitter.py

@@ -784,7 +784,8 @@ class TreeMetadataEmitter(LoggingMixin):
                 self._libs[libname].append(lib)
                 self._linkage.append((context, lib, 'USE_LIBS'))
                 linkables.append(lib)
-                generated_files.add(lib.lib_name)
+                if not lib.installed:
+                    generated_files.add(lib.lib_name)
                 if symbols_file and isinstance(symbols_file, SourcePath):
                     script = mozpath.join(
                         mozpath.dirname(mozpath.dirname(__file__)),

toolkit/crashreporter/tools/symbolstore.py

@@ -612,6 +612,26 @@ class Dumper:
 # Platform-specific subclasses.  For the most part, these just have
 # logic to determine what files to extract symbols from.
 
+def locate_pdb(path):
+    '''Given a path to a binary, attempt to locate the matching pdb file with simple heuristics:
+    * Look for a pdb file with the same base name next to the binary
+    * Look for a pdb file with the same base name in the cwd
+
+    Returns the path to the pdb file if it exists, or None if it could not be located.
+    '''
+    path, ext = os.path.splitext(path)
+    pdb = path + '.pdb'
+    if os.path.isfile(pdb):
+        return pdb
+    # If there's no pdb next to the file, see if there's a pdb with the same root name
+    # in the cwd. We build some binaries directly into dist/bin, but put the pdb files
+    # in the relative objdir, which is the cwd when running this script.
+    base = os.path.basename(pdb)
+    pdb = os.path.join(os.getcwd(), base)
+    if os.path.isfile(pdb):
+        return pdb
+    return None
+
 class Dumper_Win32(Dumper):
     fixedFilenameCaseCache = {}
 
@@ -619,14 +639,13 @@ class Dumper_Win32(Dumper):
         """This function will allow processing of exe or dll files that have pdb
         files with the same base name next to them."""
         if file.endswith(".exe") or file.endswith(".dll"):
-            path, ext = os.path.splitext(file)
-            if os.path.isfile(path + ".pdb"):
+            if locate_pdb(file) is not None:
                 return True
         return False
 
 
     def CopyDebug(self, file, debug_file, guid, code_file, code_id):
-        file = "%s.pdb" % os.path.splitext(file)[0]
+        file = locate_pdb(file)
         def compress(path):
             compressed_file = path[:-1] + '_'
             # ignore makecab's output

toolkit/crashreporter/tools/unit-symbolstore.py

@@ -484,8 +484,7 @@ class TestFunctional(HelperMixin, unittest.TestCase):
                                               'win32',
                                               'dump_syms_vc{_MSC_VER}.exe'.format(**buildconfig.substs))
             self.target_bin = os.path.join(buildconfig.topobjdir,
-                                           'browser',
-                                           'app',
+                                           'dist', 'bin',
                                            'firefox.exe')
         else:
             self.dump_syms = os.path.join(buildconfig.topobjdir,
@@ -504,6 +503,7 @@ class TestFunctional(HelperMixin, unittest.TestCase):
         dist_include_manifest = os.path.join(buildconfig.topobjdir,
                                              '_build_manifests/install/dist_include')
         dist_include = os.path.join(buildconfig.topobjdir, 'dist/include')
+        browser_app = os.path.join(buildconfig.topobjdir, 'browser/app')
         output = subprocess.check_output([sys.executable,
                                           self.script_path,
                                           '--vcs-info',
@@ -513,7 +513,8 @@ class TestFunctional(HelperMixin, unittest.TestCase):
                                           self.dump_syms,
                                           self.test_dir,
                                           self.target_bin],
-                                         stderr=open(os.devnull, 'w'))
+                                         stderr=open(os.devnull, 'w'),
+                                         cwd=browser_app)
         lines = filter(lambda x: x.strip(), output.splitlines())
         self.assertEqual(1, len(lines),
                          'should have one filename in the output')