1243750 - mac sdk generated in the wrong place

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[[e-mail] (:bhearsum)]

It ends up in places like http://ftp.mozilla.org/pub/firefox/releases/44.0/Firefox-44.0.en-US.mac-x86_64.sdk.tar.bz2 instead of http://ftp.mozilla.org/pub/firefox/releases/44.0/mac/en-US.

The capital letters make me suspicious that something may be wrong or inconistent in package-name/packager/upload.mk.

Comment 1

[Michael Shal [:mshal]]

I think the location is different because of this OSX-specific code in upload-files.mk:

SDK = $(MOZ_PKG_APPNAME)-$(MOZ_PKG_VERSION).$(AB_CD).mac-$(TARGET_CPU).sdk$(SDK_SUFFIX)
ifeq ($(MOZ_APP_NAME),xulrunner)
SDK = $(SDK_PATH)$(MOZ_APP_NAME)-$(MOZ_PKG_VERSION).$(AB_CD).mac-$(TARGET_CPU).sdk$(SDK_SUFFIX)
endif

Specifically, since MOZ_APP_NAME is no longer 'xulrunner', we use the first SDK definition, which doesn't have $(SDK_PATH). SDK_PATH is defined as:

SDK_PATH      = $(PKG_PATH)
ifeq ($(MOZ_APP_NAME),xulrunner)
SDK_PATH = sdk/
endif

So the SDK ends up in the same place as the package for Linux & Windows, but since the OSX specific code ignores SDK_PATH when we're not building xulrunner, it gets dumped into the top-level.

Do we want to change this structure so that the SDK is always built in "xulrunner-mode", meaning SDK_PATH = sdk, and we use the second 'SDK' definition for OSX? This would put the sdk in a separate 'sdk' directory, not alongside the package. If we don't want to move the sdks there, we at least need to add $(SDK_PATH) to this line I think:

SDK = $(MOZ_PKG_APPNAME)-$(MOZ_PKG_VERSION).$(AB_CD).mac-$(TARGET_CPU).sdk$(SDK_SUFFIX)

Does any code inside an 'if MOZ_APP_NAME == xulrunner' block do anything anymore? There's a bunch, not just in Makefiles, but also some test .js files.

Comment 2

[[e-mail] (:bhearsum)]

(In reply to Michael Shal [:mshal] from comment #1)
> I think the location is different because of this OSX-specific code in
> upload-files.mk:
> 
> SDK =
> $(MOZ_PKG_APPNAME)-$(MOZ_PKG_VERSION).$(AB_CD).mac-$(TARGET_CPU).
> sdk$(SDK_SUFFIX)
> ifeq ($(MOZ_APP_NAME),xulrunner)
> SDK =
> $(SDK_PATH)$(MOZ_APP_NAME)-$(MOZ_PKG_VERSION).$(AB_CD).mac-$(TARGET_CPU).
> sdk$(SDK_SUFFIX)
> endif
> 
> Specifically, since MOZ_APP_NAME is no longer 'xulrunner', we use the first
> SDK definition, which doesn't have $(SDK_PATH). SDK_PATH is defined as:
> 
> SDK_PATH      = $(PKG_PATH)
> ifeq ($(MOZ_APP_NAME),xulrunner)
> SDK_PATH = sdk/
> endif
> 
> So the SDK ends up in the same place as the package for Linux & Windows, but
> since the OSX specific code ignores SDK_PATH when we're not building
> xulrunner, it gets dumped into the top-level.
> 
> Do we want to change this structure so that the SDK is always built in
> "xulrunner-mode", meaning SDK_PATH = sdk, and we use the second 'SDK'
> definition for OSX? This would put the sdk in a separate 'sdk' directory,
> not alongside the package. If we don't want to move the sdks there, we at
> least need to add $(SDK_PATH) to this line I think:

I think it makes sense to put them in "sdks" if it's easy to. I'm worried there's still more twists and turns in this rabbit hole though :(.

> SDK =
> $(MOZ_PKG_APPNAME)-$(MOZ_PKG_VERSION).$(AB_CD).mac-$(TARGET_CPU).
> sdk$(SDK_SUFFIX)
> 
> Does any code inside an 'if MOZ_APP_NAME == xulrunner' block do anything
> anymore? There's a bunch, not just in Makefiles, but also some test .js
> files.

I don't think so. XULRunner is a dead product...

Comment 3

[Michael Shal [:mshal]]

Attachment: 0001-Bug-1243750-Install-all-SDKs-into-sdk.patch

This does a few things:
 1) Add $(SDK_PATH) to the SDK definition for OSX, so it doesn't end up in the top-level directory

 2) Force SDK_PATH to always be 'sdk', which is essentially the xulrunner behavior. This puts the sdks into dist/sdk locally, and should put them into an sdk subdirectory with post_upload.py, I think. :nthomas, is this correct? The try behavior seems different so it's a little hard to verify.

 3) Removes some dead xulrunner code in package-name.mk

Comment 4

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 8713875 [details] [diff] [review]
0001-Bug-1243750-Install-all-SDKs-into-sdk.patch

(In reply to Michael Shal [:mshal] from comment #3)
>  2) Force SDK_PATH to always be 'sdk', which is essentially the xulrunner
> behavior. This puts the sdks into dist/sdk locally, and should put them into
> an sdk subdirectory with post_upload.py, I think. :nthomas, is this correct?
> The try behavior seems different so it's a little hard to verify.

I'm not sure tbh. My guess is that the --release-to-candidates case will work, but what is the desired behaviour for dep builds ? Dirs like http://archive.mozilla.org/pub/firefox/tinderbox-builds/mozilla-central-macosx64/1454263346/ don't appear to have a sdk. I bring this up because release-promotion world will have dep and then use beetmover to rename+move files.

The code is at https://github.com/mozilla-services/product-delivery-tools/tree/master/post_upload if you want to read some Go. Alternatively you can test in the staging setup by modifying some environment variables
  UPLOAD_HOST="upload.ffxbld.productdelivery.stage.mozaws.net"
and POST_UPLOAD_CMD ending with
  --bucket-prefix net-mozaws-stage-delivery
Or just ssh to [e-mail] from any slave, set up some test files in /tmp/<tmpdir> and call post_upload directly.
Check the output and/or http://bucketlister-delivery.stage.mozaws.net/ for results.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

So if we are going to put those builds into a separate directory (thanks a lot for doing that!!) do we have a chance to get this also fixed for the 44.0 release? If yes, I wouldn't necessarily have to update mozdownload but if not I would expect breakage for our update tests once the next release is out and we have to test updates from a 44.0 release build.

Comment 6

[Mike Hommey [:glandium]]

Comment on attachment 8713875 [details] [diff] [review]
0001-Bug-1243750-Install-all-SDKs-into-sdk.patch

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

::: toolkit/mozapps/installer/upload-files.mk
@@ +535,4 @@
>  # The plst and blkx resources are skipped because they belong to each
>  # individual dmg and are created by hdiutil.
>  SDK_SUFFIX = .tar.bz2
> +SDK = $(SDK_PATH)$(MOZ_PKG_APPNAME)-$(MOZ_PKG_VERSION).$(AB_CD).mac-$(TARGET_CPU).sdk$(SDK_SUFFIX)

SDK is already set to $(SDK_PATH)$(PKG_BASENAME).sdk$(SDK_SUFFIX) and $(SDK_PATH)$(PKG_BASENAME)-$(TARGET_CPU).sdk$(SDK_SUFFIX) on universal builds. PKG_BASENAME being $(MOZ_PKG_APPNAME)-$(MOZ_PKG_VERSION).$(AB_CD).$(MOZ_PKG_PLATFORM), and MOZ_PKG_PLATFORM being mac on universal builds, this means you shouldn't even need this assignment.

Comment 7

[Michael Shal [:mshal]]

Attachment: 0001-Bug-1243750-Install-all-SDKs-into-sdk.patch

This removes the separate OSX assignment. However, on OSX the PKG_BASENAME is actually '$(MOZ_PKG_APPNAME) $(MOZ_PKG_LONGVERSION)', which contains a space. So we have to change a few things where SDK and $(SDK).asc are used to account for spaces in the filename.

I also changed the SDK_PATH to be $(PKG_PATH)/sdk/ instead of just sdk/, otherwise the SDKs for all platforms go into the same directory, rather than a per-platform directory.

Since this causes OSX builds to create the SDK in $(SDK_PATH), it also fixes uploading the i386 version since that's where the wildcards in the UNIFY_DIST block are looking. However, the i386 version still goes into the top-level build directory due to how full paths are handled in post_upload, I believe (see also https://bugzilla.mozilla.org/show_bug.cgi?id=1245272#c2 ). If post_upload.py can't be changed to account for this, we may need to create the i386 sdk in objdir/x86_64/dist/mac/en-US/sdk so that it ends up in the right place.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5bf07e98c4ee

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5bf07e98c4ee

Comment 10

[Carsten Book [:Tomcat]]

sorry had to back this out seems this caused problems on OSX and Linux Nightly Builds like https://treeherder.mozilla.org/logviewer.html#?job_id=3268145&repo=mozilla-central

Comment 11

[Pulsebot]

Backout:
https://hg.mozilla.org/mozilla-central/rev/ac39fba33c6d

Comment 12

[Eric Hu]

Attachment: Backed out changeset 5bf07e98c4ee () for breaking Linux and OS X Nightly Builds

Comment 13

[Eric Hu]

I didn't mean to create this attachment or modify this bug--accidentally used `hg bzexport` incorrectly.  Could someone help me undo the changes?

Comment 14

[rafaelgaspar]

Attachment: Backed out changeset 5bf07e98c4ee () for breaking Linux and OS X Nightly Builds

Comment 15

[rafaelgaspar]

Attachment: Backed out changeset 5bf07e98c4ee () for breaking Linux and OS X Nightly Builds

Comment 16

[Michael Shal [:mshal]]

Attachment: 0001-Bug-1243750-Install-all-SDKs-into-sdk-r-glandium.patch

Here's my latest attempt. I moved SDK_PATH into package-name.mk, so it is empty when PKG_PATH is empty for nightlies (which is what caused the bustage).

I also changed build/upload.py to cause the i386 sdk to be uploaded in the 'mac/en-US' directory for release builds. I'm still testing this out in staging, but it seemed to work when I ran things manually. Basically we just need to strip off the '.../obj-firefox/i386/dist' path similar to how we strip off '.../obj-firefox/x86_64/dist'. When the paths don't match, the files end up in the top-level directory of the release. post_upload.py just copies the files as they are in the temporary directory, so trying to fix it there won't help.

Comment 17

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9817e865b1b6

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9817e865b1b6

Comment 19

[Michael Shal [:mshal]]

Comment on attachment 8718565 [details] [diff] [review]
0001-Bug-1243750-Install-all-SDKs-into-sdk-r-glandium.patch

I believe we just need to uplift this patch to aurora/beta/release in order for the SDK to be built and uploaded to the correct release directory. I've tried to test it as best I can in staging, but that's always a bit of a wildcard. It's currently running in nightly, though nightly and release builds differ in how this logic is invoked, so that isn't the best indicator.

Approval Request Comment
[Feature/regressing bug #]: 1233005, 1243750, 1245272
[User impact if declined]: SDKs for OSX will not be available for the release
[Describe test coverage new/current, TreeHerder]: Patch is running on nightly, and has been tested in staging.
[Risks and why]: Staging test environment is not really the same as production.
[String/UUID change made/needed]:

Comment 20

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8718565 [details] [diff] [review]
0001-Bug-1243750-Install-all-SDKs-into-sdk-r-glandium.patch

Important regression, taking it.
Should be in 45 beta 9

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/29c821a91c05

Comment 22

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/dc89d7351227

Comment 23

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8718565 [details] [diff] [review]
0001-Bug-1243750-Install-all-SDKs-into-sdk-r-glandium.patch

No longer relevant

Comment 24

[Mayur Patil]

[bugday-20160323]

Status: RESOLVED,FIXED -> UNVERIFIED

Comments:
STR: Not clear.
Developer specific testing

Component: 
Name			Firefox
Version			46.0b9
Build ID		20160322075646
Update Channel          beta
User Agent		Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
OS			Windows 7 SP1 x86_64

Expected Results: 
Developer specific testing

Actual Results: 
As expected