1013882 - make install fails with --enable-debug --disable-tests: Missing file(s): bin/components/TestInterfaceJS.js

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Jan Beich]

$ gmake install
gmake[1]: Entering directory `/a/mozilla-beta/obj-x86_64-unknown-freebsd11.0'
gmake[2]: Entering directory `/a/mozilla-beta/obj-x86_64-unknown-freebsd11.0/browser/installer'
/a/mozilla-beta/obj-x86_64-unknown-freebsd11.0/_virtualenv/bin/python -m mozbuild.action.preprocessor -DMOZ_GLUE_IN_PROGRAM -DNO_NSPR_10_SUPPORT -DAB_CD=en-US -DMOZ_APP_NAME=firefox -DPREF_DIR=defaults/preferences -DMOZ_DEBUG=1 -DMOZ_ENABLE_GNOME_COMPONENT=1 -DMOZ_GTK=1 -DMOZ_NATIVE_NSPR=1 -DMOZ_NATIVE_NSS=1 -DJAREXT= -DMOZ_CHILD_PROCESS_NAME=plugin-container -DMOZ_JSDEBUGGER -DNECKO_WIFI -DDLL_PREFIX=lib -DDLL_SUFFIX=.so -DBIN_SUFFIX= -DBINPATH=bin -DMOZ_ICU_VERSION=52 -DMOZ_NATIVE_ICU -DMOZ_SHARED_ICU -DHAVE_X86_AVX2='1' -DHAVE_64BIT_OS='1' -DMOZILLA_VERSION='"30.0"' -DMOZILLA_VERSION_U='30.0' -DMOZILLA_UAVERSION='"30.0"' -DD_INO='d_ino' -DMOZ_DEBUG_SYMBOLS='1' -DSTDC_HEADERS='1' -DHAVE_ST_BLKSIZE='1' -DHAVE_SIGINFO_T='1' -DHAVE_VISIBILITY_HIDDEN_ATTRIBUTE='1' -DHAVE_VISIBILITY_ATTRIBUTE='1' -DHAVE_DIRENT_H='1' -DHAVE_GETOPT_H='1' -DHAVE_MEMORY_H='1' -DHAVE_UNISTD_H='1' -DHAVE_NL_TYPES_H='1' -DHAVE_X11_XKBLIB_H='1' -DHAVE_CPUID_H='1' -DHAVE_SYS_TYPES_H='1' -DHAVE_NETINET_IN_H='1' -DHAVE_SIN_LEN='1' -DHAVE_SCONN_LEN='1' -DHAVE_SIN6_LEN='1' -DHAVE_SA_LEN='1' -DHAVE_SYS_CDEFS_H='1' -DHAVE_DLOPEN='1' -DHAVE_DLADDR='1' -DHAVE_MEMMEM='1' -DFUNCPROTO='15' -DHAVE_LIBXSS='1' -D_REENTRANT='1' -D_THREAD_SAFE='1' -DHAVE_LOCALTIME_R='1' -DHAVE_CLOCK_MONOTONIC='1' -DHAVE_RES_NINIT='1' -DHAVE_LANGINFO_CODESET='1' -DVA_COPY='va_copy' -DHAVE_VA_COPY='1' -DHAVE_VA_LIST_AS_ARRAY='1' -DHAVE_THREAD_TLS_KEYWORD='1' -DMALLOC_H='<malloc_np.h>' -DHAVE_STRNDUP='1' -DHAVE_POSIX_MEMALIGN='1' -DHAVE_VALLOC='1' -DHAVE_MALLOC_USABLE_SIZE='1' -DHAVE_I18N_LC_MESSAGES='1' -DHAVE_LOCALECONV='1' -DNS_ATTR_MALLOC='__attribute__((malloc))' -DNS_WARN_UNUSED_RESULT='__attribute__((warn_unused_result))' -DRELEASE_BUILD='1' -DHAVE_LIBPNG='1' -DMOZ_UPDATE_CHANNEL='default' -DEARLY_BETA_OR_EARLIER='1' -DMOZ_PHOENIX='1' -DMOZ_BUILD_APP='browser' -DMOZ_X11='1' -DMOZ_WIDGET_GTK2='1' -DMOZ_WIDGET_GTK='2' -DMOZ_PDF_PRINTING='1' -DMOZ_ENABLE_XREMOTE='1' -DMOZ_INSTRUMENT_EVENT_LOOP='1' -DNS_PRINTING='1' -DNS_PRINT_PREVIEW='1' -DMOZ_OFFICIAL_BRANDING='1' -DMOZ_DISTRIBUTION_ID='"org.mozilla"' -DMOZ_ENABLE_GIO='1' -DIBMBIDI='1' -DACCESSIBILITY='1' -DMOZ_WEBRTC='1' -DMOZ_WEBRTC_ASSERT_ALWAYS='1' -DMOZ_WEBRTC_SIGNALING='1' -DMOZ_PEERCONNECTION='1' -DMOZ_SCTP='1' -DMOZ_SRTP='1' -DMOZ_SAMPLE_TYPE_FLOAT32='1' -DMOZ_WEBSPEECH='1' -DMOZ_RAW='1' -DATTRIBUTE_ALIGNED_MAX='64' -DMOZ_OGG_NO_MEM_REPORTING='1' -DMOZ_WEBM='1' -DMOZ_MEDIA_NAVIGATOR='1' -DMOZ_VPX='1' -DMOZ_VPX_ERROR_CONCEALMENT='1' -DHAVE_LIBVPX='1' -DMOZ_VPX_NO_MEM_REPORTING='1' -DMOZ_WAVE='1' -DMOZ_VORBIS='1' -DMOZ_OPUS='1' -DMOZ_WEBM_ENCODER='1' -DMOZ_GSTREAMER='1' -DGST_API_VERSION='"1.0"' -DENABLE_SYSTEM_EXTENSION_DIRS='1' -DMOZ_WEBGL='1' -DMOZ_WEBGL_CONFORMANT='1' -DMOZ_CRASHREPORTER_ENABLE_PERCENT='100' -DMOZ_WEBAPP_RUNTIME='1' -DMOZ_CONTENT_SANDBOX_REPORTER='1' -DMOZ_FEEDS='1' -DMOZ_NATIVE_SQLITE='1' -DMOZ_SAFE_BROWSING='1' -DMOZ_URL_CLASSIFIER='1' -DGL_PROVIDER_GLX='1' -DMOZ_LOGGING='1' -DMOZ_DUMP_PAINTING='1' -DHAVE_MALLCTL='1' -DHAVE_NALLOCM='1' -DMOZ_MEMORY='1' -DMOZ_JEMALLOC3='1' -DMOZ_NATIVE_JEMALLOC='1' -DJSGC_INCREMENTAL='1' -DJSGC_USE_EXACT_ROOTING='1' -DJS_GC_ZEAL='1' -DMOZ_PAY='1' -DMOZ_ACTIVITIES='1' -DMOZ_SERVICES_FXACCOUNTS='1' -DHAVE___CXA_DEMANGLE='1' -DMOZ_DEMANGLE_SYMBOLS='1' -DHAVE__UNWIND_BACKTRACE='1' -DJS_DEFAULT_JITREPORT_GRANULARITY='3' -DMOZ_OMNIJAR='1' -DMOZ_USER_DIR='".mozilla"' -DHAVE_FT_BITMAP_SIZE_Y_PPEM='1' -DHAVE_FT_GLYPHSLOT_EMBOLDEN='1' -DHAVE_FT_LOAD_SFNT_TABLE='1' -DHAVE_FONTCONFIG_FCFREETYPE_H='1' -DHAVE_STDINT_H='1' -DHAVE_INTTYPES_H='1' -DMOZ_TREE_CAIRO='1' -DMOZ_ENABLE_SKIA='1' -DUSE_SKIA='1' -DUSE_SKIA_GPU='1' -DMOZ_XUL='1' -DMOZ_PROFILELOCKING='1' -DBUILD_CTYPES='1' -DMOZ_PLACES='1' -DMOZ_SOCIAL='1' -DMOZ_SERVICES_COMMON='1' -DMOZ_SERVICES_CRYPTO='1' -DMOZ_SERVICES_HEALTHREPORT='1' -DMOZ_SERVICES_METRICS='1' -DMOZ_SERVICES_SYNC='1' -DMOZ_JSDOWNLOADS='1' -DMOZ_MACBUNDLE_ID='org.mozilla.firefoxdebug' -DMOZ_B2G_VERSION='"1.0.0"' -DMOZ_B2G_OS_NAME='""' -DMOZ_APP_UA_NAME='""' -DMOZ_APP_UA_VERSION='"30.0"' -DFIREFOX_VERSION='30.0' -DMOZ_TELEMETRY_DISPLAY_REV='2' -DMOZ_DATA_REPORTING='1' -DMOZ_DLL_SUFFIX='".so"' -DHAVE_POSIX_FADVISE='1' -DHAVE_POSIX_FALLOCATE='1' -DXP_UNIX='1' -DMOZ_REFLOW_PERF='1' -DMOZ_REFLOW_PERF_DSP='1' -DMOZ_ACCESSIBILITY_ATK='1' -DATK_MAJOR_VERSION='2' -DATK_MINOR_VERSION='8' -DATK_REV_VERSION='0' -DA11Y_LOG='1' -DEXPOSE_INTL_API='1' -DENABLE_INTL_API='1' -DMOZ_STATIC_JS='1' /a/mozilla-beta/browser/installer/package-manifest.in -o package-manifest
OMNIJAR_NAME=omni.ja \
/a/mozilla-beta/obj-x86_64-unknown-freebsd11.0/_virtualenv/bin/python /a/mozilla-beta/toolkit/mozapps/installer/packager.py -DMOZ_GLUE_IN_PROGRAM -DNO_NSPR_10_SUPPORT -DAB_CD=en-US -DMOZ_APP_NAME=firefox -DPREF_DIR=defaults/preferences -DMOZ_DEBUG=1 -DMOZ_ENABLE_GNOME_COMPONENT=1 -DMOZ_GTK=1 -DMOZ_NATIVE_NSPR=1 -DMOZ_NATIVE_NSS=1 -DJAREXT= -DMOZ_CHILD_PROCESS_NAME=plugin-container -DMOZ_JSDEBUGGER -DNECKO_WIFI -DDLL_PREFIX=lib -DDLL_SUFFIX=.so -DBIN_SUFFIX= -DBINPATH=bin -DMOZ_ICU_VERSION=52 -DMOZ_NATIVE_ICU -DMOZ_SHARED_ICU \
        --format omni \
        --removals /a/mozilla-beta/browser/installer/removed-files.in \
         \
         \
         \
        --optimizejars \
         \
        package-manifest ../../dist ../../dist/firefox \

Error: /a/mozilla-beta/obj-x86_64-unknown-freebsd11.0/browser/installer/package-manifest:393: Missing file(s): bin/components/TestInterfaceJS.js
Error: /a/mozilla-beta/obj-x86_64-unknown-freebsd11.0/browser/installer/package-manifest:394: Missing file(s): bin/components/TestInterfaceJS.manifest
Traceback (most recent call last):
  File "/a/mozilla-beta/toolkit/mozapps/installer/packager.py", line 385, in <module>
    main()
  File "/a/mozilla-beta/toolkit/mozapps/installer/packager.py", line 338, in main
    copier.add(mozpack.path.join(binpath, 'removed-files'), removals)
  File "/usr/local/lib/python2.7/contextlib.py", line 24, in __exit__
    self.gen.next()
  File "/a/mozilla-beta/python/mozbuild/mozpack/errors.py", line 129, in accumulate
    raise AccumulatedErrors()
mozpack.errors.AccumulatedErrors
gmake[2]: *** [stage-package] Error 1
gmake[2]: Leaving directory `/a/mozilla-beta/obj-x86_64-unknown-freebsd11.0/browser/installer'
gmake[1]: *** [install] Error 2
gmake[1]: Leaving directory `/a/mozilla-beta/obj-x86_64-unknown-freebsd11.0'
*** Error code 2

Comment 1

[Jan Beich]

Obviously, tests are only built with --enable-tests.

Comment 2

[Jan Beich]

Attachment: Check for -DENABLE_TESTS=1 from ACDEFINES

Comment 3

[Jan Beich]

Attachment: Check for -DENABLE_TESTS=1 from ACDEFINES

s/installer/make install/ in commit message

Comment 4

[Jan Beich]

Attachment: Pass -DMOZ_DEBUG=1 on Android/B2G, too

I don't see TestInterfaceJS.* being installed in TBPL logs on Android and B2G. Looking into configure.in MOZ_DEBUG has only AC_SUBST but not AC_DEFINE.

Comment 5

[Jan Beich]

Attachment: Pass -DMOZ_DEBUG=1 on Android/B2G, too

Rebased. Not sure what caused the conflict.

Comment 6

[Jan Beich]

Comment on attachment 8438528 [details] [diff] [review]
Pass -DMOZ_DEBUG=1 on Android/B2G, too

Sorry, the patch conflict was specific to |git bz apply|.

Comment 7

[Jan Beich]

Comment on attachment 8438528 [details] [diff] [review]
Pass -DMOZ_DEBUG=1 on Android/B2G, too

Green on Try. However, TestInterfaceJS* aren't installed on Android. I don't see any ACDEFINES after -m mozbuild.action.preprocessor at all.

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

Comment 8

[Mark Smith [:mcs]]

Attachment #8438516 [details] [diff] fixes this problem on Mac OS.  It would be great if this patch landed.

Comment 9

[u43589]

(In reply to Mark Smith from comment #8)
> Attachment #8438516 [details] [diff] [diff] fixes this problem on Mac OS.  It would
> be great if this patch landed.

Indeed. This is silly

Comment 10

[Landry Breuil (:gaston)]

What's the holdup with this ? I've hit this bug when building 44beta. Provided the patches still apply, is it only a matter of setting c-n ?

Comment 11

[Landry Breuil (:gaston)]

patches don't apply as-is - im testing a variation and if that works here i'll post an unbitrotten version to try.

Comment 12

[Jan Beich]

Comment on attachment 8438528 [details] [diff] [review]
Pass -DMOZ_DEBUG=1 on Android/B2G, too

This patch was blocked by insufficient testing. However, it's now in the tree as part of bug 1025183.
https://hg.mozilla.org/mozilla-central/rev/1be6390b8931

Comment 13

[Jan Beich]

(In reply to Landry Breuil (:gaston) from comment #11)
Please, include any new places TestInterface* are installed by.

$ ls **/package-manifest.in
b2g/installer/package-manifest.in
browser/installer/package-manifest.in
mobile/android/b2gdroid/installer/package-manifest.in  <---- not in attachment 8438516 [details] [diff] [review]
mobile/android/installer/package-manifest.in

Comment 14

[Landry Breuil (:gaston)]

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

Comment 15

[Landry Breuil (:gaston)]

Yeah i saw the b2gdroid installer, Makefile.in and package-manifest.in amended accordingly... but i missed your comment about bug 1025183 before pushing to try :)

Comment 16

[Landry Breuil (:gaston)]

Attachment: bug1013882-package-debug

This should be the correct version, all package-manifest.in properly amended. Allowed me to package 44.0rc2 with debug syms, would be cool to backport to 45 :)

Comment 17

[Landry Breuil (:gaston)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=034649292b0b is the correct tbpl link, damn old bz extensions.

Comment 18

[Landry Breuil (:gaston)]

two 3 test failures are about a pip version and seem unrelated to the patch.

Comment 19

[Mike Hommey [:glandium]]

Comment on attachment 8711442 [details] [diff] [review]
bug1013882-package-debug

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

From a build system perspective, this is fine, but I want to question why those files should be part of a debug build at all. (and why would anyone want to make install a --enable-debug build is also a mystery to me)

Comment 20

[Mike Hommey [:glandium]]

Boris, is there a particular reason those files (TestInterfaceJS.manifest, TestInterfaceJSMaplike.js, TestInterfaceJS.js) are not in one of the test addons (or its own)?

Comment 21

[Boris Zbarsky [:bzbarsky]]

I don't know.  I didn't add the initial addition of this stuff, but even if I had I would likely have stamped it "if it works, great".

The only real requirement here is that these files be around when the relevant tests are run.  I don't have a strong opinion on how we accomplish that.

Comment 22

[Landry Breuil (:gaston)]

(In reply to Mike Hommey [:glandium] from comment #19)
> Comment on attachment 8711442 [details] [diff] [review]
> bug1013882-package-debug
> 
> Review of attachment 8711442 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> From a build system perspective, this is fine, but I want to question why
> those files should be part of a debug build at all. (and why would anyone
> want to make install a --enable-debug build is also a mystery to me)

Well, as a packager, you want to provide your users the possibility to run a debug build so that they provide logs and proper crash backtraces with more info, hence have it packaged.. which runs make install...

Comment 23

[Mike Hommey [:glandium]]

If you want to provide proper crash backtraces, you want debug *symbols*, not debug builds.

Comment 24

[Jan Beich]

assert checks, disabled SIMD, extra verbosity are hidden under --enabled-debug, even for bundled libraries which are *harder* to debug from downstream POV. Issues like bug 952048 and bug 1153683 is hard to debug relying on just symbols.

Not sure if |make package| from debug builds ever end up on Mozilla CDN unlike tests.

Comment 25

[Jan Beich]

Landry, are you going to fix this? If not Assignee is misleading.

Comment 26

[Landry Breuil (:gaston)]

So many bugs, so little time and interest fading....

Comment 27

[Mark Smith [:mcs]]

It is sad that the patch in attachment 8711442 [details] [diff] [review] has not landed yet. While it is strange that the Test... files are included in debug packages, that seems like a separate bug to me. This one is about fixing a build breakage, and the (simple) fix has been "in hand" for most of 2016.

Comment 28

[Michelangelo De Simone [Offline] [:mds]]

Attachment: Bug 1013882 - TestInterfaceJS should be packaged only if it's available.

Review commit: https://reviewboard.mozilla.org/r/92830/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/92830/

Comment 29

[Michelangelo De Simone [Offline] [:mds]]

The original patch has been rebased. As per Bug 1306391 (nukeb2g), b2g manifests have been skipped.

:glandium, as the original patch was r=you, do you mind if we land this?
It's a pretty trivial change and I'm sure it'll be very welcome the *BSD folks (myself included :)).

Comment 30

[Jan Beich]

Originally, there was also attachment 8438528 [details] [diff] [review] but it was fixed by bug 1025183.

Comment 31

[Mike Hommey [:glandium]]

Comment on attachment 8810563 [details]
Bug 1013882 - TestInterfaceJS should be packaged only if it's available.

https://reviewboard.mozilla.org/r/92830/#review92966

You should make the same change to b2g/installer/package-manifest.in.

Comment 32

[Michelangelo De Simone [Offline] [:mds]]

Clearing tracking flags before landing.

Comment 33

[Michelangelo De Simone [Offline] [:mds]]

Comment on attachment 8810563 [details]
Bug 1013882 - TestInterfaceJS should be packaged only if it's available.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/92830/diff/1-2/

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ae6bb745b299
TestInterfaceJS should be packaged only if it's available. r=glandium

Comment 35

[Carsten Book [:Tomcat]]

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

Comment 36

[Jan Beich]

Comment on attachment 8810563 [details]
Bug 1013882 - TestInterfaceJS should be packaged only if it's available.

Approval Request Comment
[ESR consideration]: Easier packaging debug builds downstream
[Feature/Bug causing the regression]: bug 923904 regression
[User impact if declined]: Broken install after build with --enable-debug --disable-tests
[Is this code covered by automated tests?]: No, --disable-tests are NPOTB.
[Has the fix been verified in Nightly?]: Yes, also downstream.
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Can only break package build e.g., firefox-52.1esr.tar.bz2 maybe empty or N/A.
[String changes made/needed]: None

Comment 37

[Julien Cristau [:jcristau]]

Comment on attachment 8810563 [details]
Bug 1013882 - TestInterfaceJS should be packaged only if it's available.

packaging fix for esr52

Comment 38

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/fde6e9ccfc72