Closed Bug 1243750 Opened 9 years ago Closed 9 years ago

mac sdk generated in the wrong place

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(firefox45 fixed, firefox46 fixed, firefox47 fixed)

RESOLVED FIXED
mozilla47
Tracking Status
firefox45 --- fixed
firefox46 --- fixed
firefox47 --- fixed

People

(Reporter: bhearsum, Assigned: mshal)

References

Details

(Keywords: qablocker)

Attachments

(1 file, 5 obsolete files)

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.
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.
(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...
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
Assignee: nobody → mshal
Attachment #8713875 - Flags: review?(mh+mozilla)
Attachment #8713875 - Flags: feedback?(nthomas)
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 ffxbld@upload.ffxbld.productdelivery.stage.mozaws.net 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.
Attachment #8713875 - Flags: feedback?(nthomas)
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 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.
Attachment #8713875 - Flags: review?(mh+mozilla) → feedback+
Blocks: 1245272
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.
Attachment #8713875 - Attachment is obsolete: true
Attachment #8716493 - Flags: review?(mh+mozilla)
Attachment #8716493 - Flags: review?(mh+mozilla) → review+
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
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
Status: RESOLVED → REOPENED
Flags: needinfo?(mshal)
Resolution: FIXED → ---
Assignee: mshal → pineapple.rice
Status: REOPENED → ASSIGNED
QA Whiteboard: [QAnalyst-Triage+]
Keywords: qablocker
I didn't mean to create this attachment or modify this bug--accidentally used `hg bzexport` incorrectly. Could someone help me undo the changes?
Assignee: pineapple.rice → mshal
Flags: needinfo?(mshal)
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.
Attachment #8716493 - Attachment is obsolete: true
Attachment #8717970 - Attachment is obsolete: true
Attachment #8718105 - Attachment is obsolete: true
Attachment #8718114 - Attachment is obsolete: true
Attachment #8718565 - Flags: review?(mh+mozilla)
Attachment #8718565 - Flags: review?(mh+mozilla) → review+
Status: ASSIGNED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
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]:
Attachment #8718565 - Flags: approval-mozilla-release?
Attachment #8718565 - Flags: approval-mozilla-beta?
Attachment #8718565 - Flags: approval-mozilla-aurora?
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
Attachment #8718565 - Flags: approval-mozilla-beta?
Attachment #8718565 - Flags: approval-mozilla-beta+
Attachment #8718565 - Flags: approval-mozilla-aurora?
Attachment #8718565 - Flags: approval-mozilla-aurora+
Comment on attachment 8718565 [details] [diff] [review] 0001-Bug-1243750-Install-all-SDKs-into-sdk-r-glandium.patch No longer relevant
Attachment #8718565 - Flags: approval-mozilla-release? → approval-mozilla-release-
[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
Component: Build Config → General
Product: Firefox → Firefox Build System
Keywords: qablocker
Target Milestone: Firefox 47 → mozilla47
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: