Closed Bug 1165512 Opened 9 years ago Closed 9 years ago

OSX unified builds need to generate a unified SDK or 2 SDKs

Categories

(Release Engineering :: Applications: MozharnessCore, defect)

defect
Not set
normal

Tracking

(firefox42 fixed)

RESOLVED FIXED
Tracking Status
firefox42 --- fixed

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(1 file, 4 obsolete files)

Currently, the firefox SDK generated by OSX opt builds is essentially i386. Ideally, we'd want a SDK with unified binaries, but things like the following make me think we should go with two different SDKs, like xulrunner currently does:

diff -ruN xulrunner-sdk-64/include/js-config.h xulrunner-sdk/include/js-config.h
--- xulrunner-sdk-64/include/js-config.h        2015-04-16 19:39:27.000000000 +0900
+++ xulrunner-sdk/include/js-config.h   2015-04-16 19:18:27.000000000 +0900
@@ -47,10 +47,10 @@
 /* #undef JS_HAVE_SYS_ISA_DEFS_H */

 /* Define to 1 if SpiderMonkey is in NUNBOX32 mode. */
-/* #undef JS_NUNBOX32 */
+#define JS_NUNBOX32 1

 /* Define to 1 if SpiderMonkey is in PUNBOX64 mode. */
-#define JS_PUNBOX64 1
+/* #undef JS_PUNBOX64 */

 /* MOZILLA JSAPI version number components */
 #define MOZJS_MAJOR_VERSION 40
I think ideally the build system knows how to do this when SDK generation is enabled for universal builds.

What's the right way to get the SDK generated for both architectures?
presumably, running make sdk in both objdirs.
Blocks: 1147577
We could just replace those configure checks with inline preprocessor tests...
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #3)
> We could just replace those configure checks with inline preprocessor
> tests...

There are other differences between the i386 and x64 parts. Building two sdks for now is easier than finding and fixing them all (and matches what we already do).
(In reply to Mike Hommey [:glandium] from comment #2)
> presumably, running make sdk in both objdirs.

I tried this in a try push yesterday (https://treeherder.mozilla.org/#/jobs?repo=try&revision=8d7bc021fb3c) and it failed:
14:28:24     INFO -  /usr/bin/make -C /builds/slave/try-m64-0000000000000000000000/build/src/obj-firefox/x86_64 sdk
14:28:25     INFO -  /usr/bin/make stage-package UNIVERSAL_BINARY= STAGE_SDK=1 MOZ_PKG_DIR=sdk-stage
14:28:25     INFO -  /builds/slave/try-m64-0000000000000000000000/build/src/obj-firefox/x86_64/_virtualenv/bin/python -m mozbuild.action.preprocessor -DAB_CD=en-US -DNO_NSPR_10_SUPPORT -DMOZ_APP_NAME=firefox -DPREF_DIR=defaults/preferences -DJAREXT= -DMOZ_CHILD_PROCESS_NAME=plugin-container -DMOZ_SHARED_MOZGLUE=1 -DNECKO_WIFI -DDLL_PREFIX=lib -DDLL_SUFFIX=.dylib -DBIN_SUFFIX= -DDIR_MACOS=Contents/MacOS/ -DDIR_RESOURCES=Contents/Resources/ -DMOZ_FOLD_LIBS=1 -DAPPNAME=Nightly.app -DBINPATH=/Nightly.app/Contents/MacOS -DRESPATH=Nightly.app/Contents/Resources -DAB=en -DMOZ_ICU_VERSION=55 -DMOZ_ICU_DBG_SUFFIX= -DCLANG_CXX -DX_DISPLAY_MISSING='1' -DHAVE_64BIT_BUILD='1' -DMOZ_ENABLE_PROFILER_SPS='1' -DMOZ_INSTRUMENTS='1' -DMOZ_PROFILING='1' -DMOZILLA_VERSION='"42.0a1"' -DMOZILLA_VERSION_U='42.0a1' -DMOZILLA_UAVERSION='"42.0"' -DXP_MACOSX='1' -DXP_DARWIN='1' -DD_INO='d_ino' -DMOZ_DEBUG_SYMBOLS='1' -DSTDC_HEADERS='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_QUEUE_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' -DINCLUDE_MOZILLA_DTRACE='1' -DHAVE_SYS_CDEFS_H='1' -DHAVE_DLADDR='1' -DHAVE_MEMMEM='1' -DNO_X11='1' -DHAVE_PTHREAD_H='1' -DHAVE_STAT64='1' -DHAVE_LSTAT64='1' -DHAVE_GMTIME_R='1' -DHAVE_LOCALTIME_R='1' -DHAVE_ARC4RANDOM='1' -DHAVE_ARC4RANDOM_BUF='1' -DHAVE_LANGINFO_CODESET='1' -DVA_COPY='va_copy' -DHAVE_VA_COPY='1' -DHAVE_VA_LIST_AS_ARRAY='1' -DHAVE_I18N_LC_MESSAGES='1' -DHAVE_LOCALECONV='1' -DMALLOC_H='<malloc/malloc.h>' -DHAVE_ALLOCA_H='1' -DHAVE_STRNDUP='1' -DHAVE_POSIX_MEMALIGN='1' -DMALLOC_USABLE_SIZE_CONST_PTR='const' -DHAVE_VALLOC='1' -DTARGET_XPCOM_ABI='"x86_64-gcc3"' -DNIGHTLY_BUILD='1' -DE10S_TESTING_ONLY='1' -DMOZ_UPDATE_CHANNEL='default' -DEARLY_BETA_OR_EARLIER='1' -DMOZ_PHOENIX='1' -DMOZ_BUILD_APP='browser' -DMOZ_WIDGET_COCOA='1' -DMOZ_INSTRUMENT_EVENT_LOOP='1' -DNS_PRINTING='1' -DNS_PRINT_PREVIEW='1' -DMOZ_DISTRIBUTION_ID='"org.mozilla"' -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_WEBSPEECH_TEST_BACKEND='1' -DMOZ_RAW='1' -DATTRIBUTE_ALIGNED_MAX='64' -DMOZ_WEBM='1' -DMOZ_APPLEMEDIA='1' -DMOZ_FMP4='1' -DMOZ_EME='1' -DMOZ_MEDIA_NAVIGATOR='1' -DMOZ_VPX='1' -DMOZ_VPX_ERROR_CONCEALMENT='1' -DVPX_X86_ASM='1' -DMOZ_VPX_NO_MEM_REPORTING='1' -DMOZ_WAVE='1' -DMOZ_VORBIS='1' -DMOZ_WEBM_ENCODER='1' -DMOZ_PERMISSIONS='1' -DENABLE_SYSTEM_EXTENSION_DIRS='1' -DMOZ_WEBGL_CONFORMANT='1' -DMOZ_GAMEPAD='1' -DMOZ_CRASHREPORTER='1' -DMOZ_CRASHREPORTER_ENABLE_PERCENT='100' -DMOZ_WEBAPP_RUNTIME='1' -DMOZ_SIGNING='1' -DMOZ_VERIFY_MAR_SIGNATURE='1' -DMOZ_ENABLE_SIGNMAR='1' -DMOZ_UPDATER='1' -DENABLE_TESTS='1' -DGTEST_HAS_RTTI='0' -DMOZ_CONTENT_SANDBOX='1' -DMOZ_GMP_SANDBOX='1' -DMOZ_SANDBOX='1' -DMOZ_FEEDS='1' -DMOZ_SAFE_BROWSING='1' -DMOZ_URL_CLASSIFIER='1' -DGL_PROVIDER_='1' -DMOZ_STACKWALKING='1' -DMOZ_LOGGING='1' -DFORCE_PR_LOG='1' -DMOZ_REPLACE_MALLOC='1' -DMOZ_MEMORY='1' -DMOZ_MEMORY_DARWIN='1' -DMOZ_PAY='1' -DMOZ_ACTIVITIES='1' -DMOZ_SECUREELEMENT='1' -DHAVE___CXA_DEMANGLE='1' -DHAVE__UNWIND_BACKTRACE='1' -DJS_DEFAULT_JITREPORT_GRANULARITY='3' -DMOZ_OMNIJAR='1' -DMOZ_USER_DIR='"Mozilla"' -DMOZ_TREE_PIXMAN='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' -DENABLE_MARIONETTE='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_SERVICES_CLOUDSYNC='1' -DMOZ_JSDOWNLOADS='1' -DMOZ_MACBUNDLE_ID='org.mozilla.nightly' -DMOZ_B2G_VERSION='"1.0.0"' -DMOZ_B2G_OS_NAME='""' -DMOZ_APP_UA_NAME='""' -DMOZ_APP_UA_VERSION='"42.0a1"' -DFIREFOX_VERSION='42.0a1' -DMOZ_TELEMETRY_DISPLAY_REV='2' -DMOZ_TELEMETRY_REPORTING='1' -DMOZ_TELEMETRY_ON_BY_DEFAULT='1' -DMOZ_DATA_REPORTING='1' -DMOZ_DLL_SUFFIX='".dylib"' -DXP_UNIX='1' -DA11Y_LOG='1' -DEXPOSE_INTL_API='1' -DENABLE_INTL_API='1' -DU_STATIC_IMPLEMENTATION='1' -DU_USING_ICU_NAMESPACE='0' -DMOZ_STATIC_JS='1' /builds/slave/try-m64-0000000000000000000000/build/src/browser/installer/package-manifest.in -o package-manifest
14:28:25     INFO -  OMNIJAR_NAME=omni.ja \
14:28:25     INFO -  	NO_PKG_FILES="core bsdecho js js-config jscpucfg nsinstall viewer TestGtkEmbed elf-dynstr-gc mangle* maptsv* mfc* msdump* msmap* nm2tsv* nsinstall* res/samples res/throbber shlibsign* certutil* pk12util* BadCertServer* ClientAuthServer* OCSPStaplingServer* GenerateOCSPResponse* chrome/chrome.rdf chrome/app-chrome.manifest chrome/overlayinfo components/compreg.dat components/xpti.dat content_unit_tests necko_unit_tests *.dSYM " \
14:28:25     INFO -  	/builds/slave/try-m64-0000000000000000000000/build/src/obj-firefox/x86_64/_virtualenv/bin/python /builds/slave/try-m64-0000000000000000000000/build/src/toolkit/mozapps/installer/packager.py -DAB_CD=en-US -DNO_NSPR_10_SUPPORT -DMOZ_APP_NAME=firefox -DPREF_DIR=defaults/preferences -DJAREXT= -DMOZ_CHILD_PROCESS_NAME=plugin-container -DMOZ_SHARED_MOZGLUE=1 -DNECKO_WIFI -DDLL_PREFIX=lib -DDLL_SUFFIX=.dylib -DBIN_SUFFIX= -DDIR_MACOS=Contents/MacOS/ -DDIR_RESOURCES=Contents/Resources/ -DMOZ_FOLD_LIBS=1 -DAPPNAME=Nightly.app -DBINPATH=/Nightly.app/Contents/MacOS -DRESPATH=Nightly.app/Contents/Resources -DAB=en -DMOZ_ICU_VERSION=55 -DMOZ_ICU_DBG_SUFFIX= -DCLANG_CXX \
14:28:25     INFO -  		--format omni \
14:28:25     INFO -  		--removals /builds/slave/try-m64-0000000000000000000000/build/src/browser/installer/removed-files.in \
14:28:25     INFO -  		 \
14:28:25     INFO -  		 \
14:28:25     INFO -  		 \
14:28:25     INFO -  		 \
14:28:25     INFO -  		--optimizejars \
14:28:25     INFO -  		--unify /builds/slave/try-m64-0000000000000000000000/build/src/obj-firefox/x86_64/../i386/dist \
14:28:25     INFO -  		package-manifest ../../dist ../../dist/sdk-stage \
14:28:25     INFO -  Error: /builds/slave/try-m64-0000000000000000000000/build/src/obj-firefox/x86_64/browser/installer/package-manifest:62: File missing in /builds/slave/try-m64-0000000000000000000000/build/src/obj-firefox/x86_64/../i386/dist: Nightly.app/Contents/MacOS/libreplace_jemalloc.dylib
14:28:25     INFO -  Error: /builds/slave/try-m64-0000000000000000000000/build/src/obj-firefox/x86_64/browser/installer/package-manifest:62: Missing file(s): /Nightly.app/Contents/MacOS/libreplace_jemalloc.dylib
14:28:28     INFO -  Traceback (most recent call last):
14:28:28     INFO -    File "/builds/slave/try-m64-0000000000000000000000/build/src/toolkit/mozapps/installer/packager.py", line 404, in <module>
14:28:28     INFO -      main()
14:28:28     INFO -    File "/builds/slave/try-m64-0000000000000000000000/build/src/toolkit/mozapps/installer/packager.py", line 353, in main
14:28:28     INFO -      copier.add(mozpath.join(respath, 'removed-files'), removals)
14:28:28     INFO -    File "/tools/python/lib/python2.7/contextlib.py", line 24, in __exit__
14:28:28     INFO -      self.gen.next()
14:28:28     INFO -    File "/builds/slave/try-m64-0000000000000000000000/build/src/python/mozbuild/mozpack/errors.py", line 131, in accumulate
14:28:28     INFO -      raise AccumulatedErrors()
14:28:28     INFO -  mozpack.errors.AccumulatedErrors
Looks like the file this is barfing on is missing (but not errored out for) if you generate the SDK from XULRunner:
Warning: File missing in ../../dist: bin/liblogalloc.dylib
Warning: File missing in ../../dist: bin/libreplace_jemalloc.dylib
Warning: File missing in ../../dist: bin/libreplace_malloc.dylib
Warning: File missing in ../../dist: bin/logalloc-replay

I'm guessing the XULRunner ones don't use a package-manifest. I suspect these files are OK to be missing from the SDK if the XULRunner generated one is already missing them. So maybe a tweak to the package-manifest will fix this...

It's curious that a build with only the 32-bit sdk building (http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bhearsum@mozilla.com-97636d48fdf7/try-macosx64/try-macosx64-bm83-try1-build6985.txt.gz) works fine though.
This is a horrible patch that shouldn't ever be checked in, but it seems to work. I ran into a few issues:
* The one noted above where packaging one of the SDKs fails because of a jemalloc file being missing. This patch "fixes" that by removing it from the package manifest, which probably breaks Firefox.
* To run "make sdk" in the x86_64 I added an extra step to postflight_all, similar to what we do for test packages (but without unification). I think this part might be OK, but maybe there's a better place for it.
* Getting the new SDK into UPLOAD_FILES. This part is a hardcoded hack that probably breaks in multiple ways. I wanted to put this near the postflight_all definition, since that where we seem to handle most universal build stuff, but it didn't look like there'd be enough way to append to UPLOAD_FILES from there. Even within upload-files.mk I couldn't find a way to do it without hardcoding by using $(MOZ_OBJDIR) and parsing $(MOZ_BUILD_PROJECTS) -- they don't seem to be accessible from there, but maybe I was doing something wrong.
Attachment #8632193 - Flags: feedback?(mh+mozilla)
Comment on attachment 8632193 [details] [diff] [review]
horrible, horrible hack to generate + upload both sdks

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

(In reply to Ben Hearsum [:bhearsum] from comment #7)
> Created attachment 8632193 [details] [diff] [review]
> horrible, horrible hack to generate + upload both sdks
> 
> This is a horrible patch that shouldn't ever be checked in, but it seems to
> work. I ran into a few issues:
> * The one noted above where packaging one of the SDKs fails because of a
> jemalloc file being missing. This patch "fixes" that by removing it from the
> package manifest, which probably breaks Firefox.

It doesn't break Firefox. It's also something that is not shipped on aurora and up the train. That is, that lib is only built on nightlies. There are several ways this can be fixed, but the simplest is probably to create a dummy lib on the mac x86 part. I'll look into this.

> * To run "make sdk" in the x86_64 I added an extra step to postflight_all,
> similar to what we do for test packages (but without unification). I think
> this part might be OK, but maybe there's a better place for it.

It would probably be better to do that in the sdk rules in packager.mk, ifdefed on UNIFY_DIST.

> * Getting the new SDK into UPLOAD_FILES. This part is a hardcoded hack that
> probably breaks in multiple ways. I wanted to put this near the
> postflight_all definition, since that where we seem to handle most universal
> build stuff, but it didn't look like there'd be enough way to append to
> UPLOAD_FILES from there. Even within upload-files.mk I couldn't find a way
> to do it without hardcoding by using $(MOZ_OBJDIR) and parsing
> $(MOZ_BUILD_PROJECTS) -- they don't seem to be accessible from there, but
> maybe I was doing something wrong.

Just use UNIFY_DIST.
Attachment #8632193 - Flags: feedback?(mh+mozilla)
(In reply to Mike Hommey [:glandium] from comment #8)
> Comment on attachment 8632193 [details] [diff] [review]
> horrible, horrible hack to generate + upload both sdks
> 
> Review of attachment 8632193 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> (In reply to Ben Hearsum [:bhearsum] from comment #7)
> > Created attachment 8632193 [details] [diff] [review]
> > horrible, horrible hack to generate + upload both sdks
> > 
> > This is a horrible patch that shouldn't ever be checked in, but it seems to
> > work. I ran into a few issues:
> > * The one noted above where packaging one of the SDKs fails because of a
> > jemalloc file being missing. This patch "fixes" that by removing it from the
> > package manifest, which probably breaks Firefox.
> 
> It doesn't break Firefox. It's also something that is not shipped on aurora
> and up the train. That is, that lib is only built on nightlies. There are
> several ways this can be fixed, but the simplest is probably to create a
> dummy lib on the mac x86 part. I'll look into this.

That said, I'll first look why it doesn't complain for the same reason when building firefox itself.
Oh I get it, it's getting away with it because stage-package is done in the i386 directory. I think a reasonable thing to do would be to skip stage-package when doing make sdk in the x86_64 directory, and copy dist/firefox from the i386 directory instead.
(In reply to Mike Hommey [:glandium] from comment #8)
> Comment on attachment 8632193 [details] [diff] [review]
> horrible, horrible hack to generate + upload both sdks
> 
> Review of attachment 8632193 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> (In reply to Ben Hearsum [:bhearsum] from comment #7)
> > * To run "make sdk" in the x86_64 I added an extra step to postflight_all,
> > similar to what we do for test packages (but without unification). I think
> > this part might be OK, but maybe there's a better place for it.
> 
> It would probably be better to do that in the sdk rules in packager.mk,
> ifdefed on UNIFY_DIST.

Done.

> > * Getting the new SDK into UPLOAD_FILES. This part is a hardcoded hack that
> > probably breaks in multiple ways. I wanted to put this near the
> > postflight_all definition, since that where we seem to handle most universal
> > build stuff, but it didn't look like there'd be enough way to append to
> > UPLOAD_FILES from there. Even within upload-files.mk I couldn't find a way
> > to do it without hardcoding by using $(MOZ_OBJDIR) and parsing
> > $(MOZ_BUILD_PROJECTS) -- they don't seem to be accessible from there, but
> > maybe I was doing something wrong.
> 
> Just use UNIFY_DIST.

I did this, but I had to add UNIFY_ARCH in order to get "x86-64" in the filename. This is also used in finding the x86-64 objdir to run make in.


This patch doesn't quite work either - we end up with infinite recursion inside of the make-sdk target. I tried to use a few different things as a base case to break out, but couldn't get any to work.
Attachment #8634345 - Flags: feedback?(mh+mozilla)
Comment on attachment 8634345 [details] [diff] [review]
Move x86-64 sdk call to packager.mk; use UNIFY_DIST where possible

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

::: toolkit/mozapps/installer/packager.mk
@@ +181,5 @@
>  endif
>  	cd $(DIST) && $(MAKE_SDK)
> +ifdef UNIFY_DIST
> +ifndef (,$(findstring $(UNIFY_ARCH),$(DIST)))
> +	cd $(DIST)/../../$(UNIFY_ARCH) && $(MAKE) sdk

FOO=$(UNIFY_DIST)

ifdef FOO
    make -C $(UNIFY_DIST)/.. sdk FOO=
endif

should work (as long as FOO is set outside the recipe) and avoid the infinite loop. That doesn't solve the stage-package step, though.
Attachment #8634345 - Flags: feedback?(mh+mozilla)
I'm still waiting on a try push for this, but it worked well locally. I ended up with SDKs in both dist dirs, and each appeared to have the correct things in it. I compared against the latest nightly XULRunner builds and found the following differences:
* bin/Nightly.app instead of bin/XUL.framework - this seems pretty clearly expected.
* Test files such as include/testing/TestHarness.h (and many others) exist in the new SDK - this seems suboptimal.
* Some media files (include/libavcodec, libavutil) didn't exist in the new SDK. This also seems suboptimal.

It's possible that the test files and media file differences are because I was comparing against a build from a different rev...I'll compare things again after my try push completes (https://treeherder.mozilla.org/#/jobs?repo=try&revision=95002a0abbc8).
Attachment #8636142 - Flags: feedback?(mh+mozilla)
(In reply to Ben Hearsum [:bhearsum] from comment #13)
> Created attachment 8636142 [details] [diff] [review]
> generate and upload both sdk archs
> 
> I'm still waiting on a try push for this, but it worked well locally. I
> ended up with SDKs in both dist dirs, and each appeared to have the correct
> things in it. I compared against the latest nightly XULRunner builds and
> found the following differences:
> * bin/Nightly.app instead of bin/XUL.framework - this seems pretty clearly
> expected.
> * Test files such as include/testing/TestHarness.h (and many others) exist
> in the new SDK - this seems suboptimal.
> * Some media files (include/libavcodec, libavutil) didn't exist in the new
> SDK. This also seems suboptimal.
> 
> It's possible that the test files and media file differences are because I
> was comparing against a build from a different rev...I'll compare things
> again after my try push completes
> (https://treeherder.mozilla.org/#/jobs?repo=try&revision=95002a0abbc8).

This try build has a couple of issues. There's no detached sig for the 64-bit sdk, and the build also failed when trying to upload it to Taskcluster. Shouldn't be too hard to fix either of these...
(In reply to Ben Hearsum [:bhearsum] from comment #13)
> Created attachment 8636142 [details] [diff] [review]
> generate and upload both sdk archs
> 
> I'm still waiting on a try push for this, but it worked well locally. I
> ended up with SDKs in both dist dirs, and each appeared to have the correct
> things in it. I compared against the latest nightly XULRunner builds and
> found the following differences:
> * bin/Nightly.app instead of bin/XUL.framework - this seems pretty clearly
> expected.
> * Test files such as include/testing/TestHarness.h (and many others) exist
> in the new SDK - this seems suboptimal.

That is due to the fact that xulrunner builds are built with --disable-tests. I'd say file a followup, but that has been a build system issue for a very long number of years. As a matter of fact, the debian packages for the corresponding SDK do have those files because of --enable-tests.

> * Some media files (include/libavcodec, libavutil) didn't exist in the new
> SDK. This also seems suboptimal.

It's actually likely that it's better that they are *not* in the SDK.
Comment on attachment 8636142 [details] [diff] [review]
generate and upload both sdk archs

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

::: toolkit/mozapps/installer/packager.mk
@@ +156,2 @@
>  make-sdk:
>  	$(MAKE) stage-package UNIVERSAL_BINARY= STAGE_SDK=1 MOZ_PKG_DIR=sdk-stage

It would be better if this copied the sdk-stage from the other arch when doing the second make sdk. That would solve the replace-jemalloc problem, too.
Attachment #8636142 - Flags: feedback?(mh+mozilla)
(In reply to Mike Hommey [:glandium] from comment #16)
> Comment on attachment 8636142 [details] [diff] [review]
> generate and upload both sdk archs
> 
> Review of attachment 8636142 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/mozapps/installer/packager.mk
> @@ +156,2 @@
> >  make-sdk:
> >  	$(MAKE) stage-package UNIVERSAL_BINARY= STAGE_SDK=1 MOZ_PKG_DIR=sdk-stage
> 
> It would be better if this copied the sdk-stage from the other arch when
> doing the second make sdk. That would solve the replace-jemalloc problem,
> too.

Maybe I got it backwards, or this patch is otherwise not doing what you intended, but I couldn't get rid of the replace-jemalloc problem. I did some poking on my local machine, and AFAICT it's not included in either sdk-stage directory - only in dist/Nightly.app. Sorry if I'm being dense here =\

This version of the patch passed on try, and uploaded all of the expected sdks: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bhearsum@mozilla.com-340c1c2fd9cf/
Attachment #8638556 - Flags: review?(mh+mozilla)
Assignee: nobody → bhearsum
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a6a9cb04cea4

Ben, can you check the generated SDKs look kocher to you? They do to me.
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mh@glandium.org-a6a9cb04cea4/try-macosx64/
Assignee: bhearsum → mh+mozilla
Attachment #8632193 - Attachment is obsolete: true
Attachment #8634345 - Attachment is obsolete: true
Attachment #8636142 - Attachment is obsolete: true
Attachment #8638556 - Attachment is obsolete: true
Attachment #8638556 - Flags: review?(mh+mozilla)
Attachment #8639722 - Flags: review?(mshal)
Attachment #8639722 - Flags: feedback?(bhearsum)
Comment on attachment 8639722 [details] [diff] [review]
Generate one SDK per universal-build architecture

It looks like some files are being uploaded multiple times now, like:

00:22:38     INFO -  http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mh@glandium.org-a6a9cb04cea4/try-macosx64/sccache.log.gz
00:22:38     INFO -  http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mh@glandium.org-a6a9cb04cea4/try-macosx64/firefox-42.0a1.en-US.mac-i386.sdk.tar.bz2.asc
00:22:38     INFO -  http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mh@glandium.org-a6a9cb04cea4/try-macosx64/mar
00:22:38     INFO -  http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mh@glandium.org-a6a9cb04cea4/try-macosx64/mbsdiff
00:22:38     INFO -  http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mh@glandium.org-a6a9cb04cea4/try-macosx64/sccache.log.gz
00:22:38     INFO -  http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mh@glandium.org-a6a9cb04cea4/try-macosx64/firefox-42.0a1.en-US.mac-i386.sdk.tar.bz2.asc
00:22:38     INFO -  http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mh@glandium.org-a6a9cb04cea4/try-macosx64/mar
00:22:38     INFO -  http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mh@glandium.org-a6a9cb04cea4/try-macosx64/mbsdiff
00:22:38     INFO -  http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mh@glandium.org-a6a9cb04cea4/try-macosx64/sccache.log.gz
00:22:38     INFO -  http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mh@glandium.org-a6a9cb04cea4/try-macosx64/firefox-42.0a1.en-US.mac-i386.sdk.tar.bz2.asc
00:22:38     INFO -  http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mh@glandium.org-a6a9cb04cea4/try-macosx64/mar
00:22:38     INFO -  http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mh@glandium.org-a6a9cb04cea4/try-macosx64/mbsdiff

I think this is because upload-files.mk just appends to UPLOAD_EXTRA_FILES, and having make-sdk call itself ends up including upload-files.mk again. Is there a way we can separate out the sdk building into a separate target and have make-sdk depend on that or something?
Attachment #8639722 - Flags: review?(mshal) → feedback+
> I think this is because upload-files.mk just appends to UPLOAD_EXTRA_FILES, and having make-sdk call itself ends up including upload-files.mk again. Is there a way we can separate out the sdk building into a separate target and have make-sdk depend on that or something?

Can't be that. Invoking $(MAKE) -C other/dir sdk shouldn't affect the variables in the current make.
Comment on attachment 8639722 [details] [diff] [review]
Generate one SDK per universal-build architecture

cf. comment 21.
Attachment #8639722 - Flags: feedback+ → review?(mshal)
Here is a try without mk_add_options "export UPLOAD_EXTRA_FILES+=sccache.log.gz" and it's uploading each file only once.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e7000fa8cf75

I'm not sure what's going on with that UPLOAD_EXTRA_FILES line, but it's a separate issue.
Filed bug 1188766
Comment on attachment 8639722 [details] [diff] [review]
Generate one SDK per universal-build architecture

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

Excluding Nightly.app vs. XULRunner.app differences, there's still some changes in the header and idl files compared to the most recent nightly xulrunner sdk, but I _think_ that's probably expected and/or okay. This is the list (+ is stuff only in yours, - is only in XULRunner.
+./idl/fuelIApplication.idl
+./idl/NotXPCOMTest.idl
+./idl/nsIBrowserGlue.idl
+./idl/nsIBrowserHandler.idl
+./idl/nsIBrowserProfileMigrator.idl
+./idl/nsIFeedResultService.idl
+./idl/nsIHttpServer.idl
+./idl/nsIMacShellService.idl
+./idl/nsINavigatorPayment.idl
+./idl/nsIPaymentFlowInfo.idl
+./idl/nsIPaymentProviderStrategy.idl
+./idl/nsIPaymentUIGlue.idl
+./idl/nsISessionStartup.idl
+./idl/nsISessionStore.idl
+./idl/nsIShellService.idl
-./idl/nsISimpleTest.idl
+./idl/nsIWebContentConverterRegistrar.idl
+./idl/nsIWorkerTest.idl
+./idl/xpctest_attributes.idl
+./idl/xpctest_bug809674.idl
+./idl/xpctest_interfaces.idl
+./idl/xpctest_params.idl
+./idl/xpctest_returncode.idl
+./include/fuelIApplication.h
+./include/gmock
+./include/gmock/gmock-actions.h
+./include/gmock/gmock-cardinalities.h
+./include/gmock/gmock-generated-actions.h
+./include/gmock/gmock-generated-function-mockers.h
+./include/gmock/gmock-generated-matchers.h
+./include/gmock/gmock-generated-nice-strict.h
+./include/gmock/gmock.h
+./include/gmock/gmock-matchers.h
+./include/gmock/gmock-more-actions.h
+./include/gmock/gmock-spec-builders.h
+./include/gmock/internal
+./include/gmock/internal/gmock-generated-internal-utils.h
+./include/gmock/internal/gmock-internal-utils.h
+./include/gmock/internal/gmock-port.h
+./include/gtest/gtest-death-test.h
+./include/gtest/gtest.h
+./include/gtest/gtest-message.h
+./include/gtest/gtest-param-test.h
+./include/gtest/gtest_pred_impl.h
+./include/gtest/gtest-printers.h
+./include/gtest/gtest_prod.h
+./include/gtest/gtest-spi.h
+./include/gtest/gtest-test-part.h
+./include/gtest/gtest-typed-test.h
+./include/gtest/internal
+./include/gtest/internal/gtest-death-test-internal.h
+./include/gtest/internal/gtest-filepath.h
+./include/gtest/internal/gtest-internal.h
+./include/gtest/internal/gtest-linked_ptr.h
+./include/gtest/internal/gtest-param-util-generated.h
+./include/gtest/internal/gtest-param-util.h
+./include/gtest/internal/gtest-port.h
+./include/gtest/internal/gtest-string.h
+./include/gtest/internal/gtest-tuple.h
+./include/gtest/internal/gtest-type-util.h
+./include/javascript-trace.h
+./include/libmkv
+./include/libmkv/EbmlBufferWriter.h
+./include/libmkv/EbmlIDs.h
+./include/libmkv/EbmlWriter.h
+./include/libmkv/WebMElement.h
+./include/mozilla/browser
+./include/mozilla/browser/AboutRedirector.h
+./include/mozilla/browser/DirectoryProvider.h
+./include/mozilla/dom/MozPaymentProviderBinding.h
+./include/mozilla/dom/PaymentProviderUtils.h
-./include/mozilla/dom/PermissionsBinding.h
-./include/mozilla/dom/Permissions.h
-./include/mozilla/dom/PermissionStatusBinding.h
-./include/mozilla/dom/PermissionStatus.h
+./include/mozilla-trace.h
+./include/NotXPCOMTest.h
+./include/nsBrowserCompsCID.h
+./include/nsIBrowserGlue.h
+./include/nsIBrowserHandler.h
+./include/nsIBrowserProfileMigrator.h
+./include/nsIFeedResultService.h
+./include/nsIHttpServer.h
+./include/nsIMacShellService.h
+./include/nsINavigatorPayment.h
+./include/nsIPaymentFlowInfo.h
+./include/nsIPaymentProviderStrategy.h
+./include/nsIPaymentUIGlue.h
+./include/nsISessionStartup.h
+./include/nsISessionStore.h
+./include/nsIShellService.h
-./include/nsISimpleTest.h
+./include/nsIWebContentConverterRegistrar.h
+./include/nsIWorkerTest.h
+./include/testing
+./include/testing/TestHarness.h
+./include/VorbisTrackEncoder.h
+./include/VP8TrackEncoder.h
+./include/WebMWriter.h
+./include/xpctest_attributes.h
+./include/xpctest_bug809674.h
+./include/xpctest_interfaces.h
+./include/xpctest_params.h
+./include/xpctest_returncode.h


Thank you for putting this together, much appreciated.
Attachment #8639722 - Flags: feedback?(bhearsum) → feedback+
(In reply to Mike Hommey [:glandium] from comment #23)
> Here is a try without mk_add_options "export
> UPLOAD_EXTRA_FILES+=sccache.log.gz" and it's uploading each file only once.
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=e7000fa8cf75
> 
> I'm not sure what's going on with that UPLOAD_EXTRA_FILES line, but it's a
> separate issue.

Ahh, I forgot about that bit. I was just comparing to an m-c build, which of course doesn't trigger the sccache logic.
Attachment #8639722 - Flags: review?(mshal) → review+
https://hg.mozilla.org/mozilla-central/rev/79d76aa791fb
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: