Closed Bug 1185666 Opened 9 years ago Closed 7 years ago

Do l10n repacks of Firefox for Mac on Linux

Categories

(Infrastructure & Operations Graveyard :: CIDuty, task)

Product:
Infrastructure & Operations Graveyard
Component:
CIDuty
Platform:
x86_64
Linux
Type:
task
Priority:
Not set
Severity:
normal

Tracking

(firefox55 fixed)

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: Pike, Assigned: Callek)

References

Details

(Whiteboard: [kanban:engops:https://mozilla.kanbanize.com/ctrl_board/6/2212] [capacity])

Attachments

(3 files)

Bug 1185666 - Move DMG unpack logic to a python script, support linux.
59 bytes, text/x-review-board-request
mshal
: review+
Details
Bug 1185666 - Fix l10n repack scripts and config to support OSX on Linux.
59 bytes, text/x-review-board-request
mozilla
: review+
Details
Bug 1185666 - Make sure internal tooltool items are available for TC OSX repacks.
59 bytes, text/x-review-board-request
mozilla
: review+
Details 
Once we have cross-compilation figured out for OSX, it'll be really useful to investigate the changes required to do l10n repacks on linux.

Not only does that allow us to use the cloud, but it might actually allow us to share build processes between linux and osx.
What parts of the l10n repack process require platform specific tools?
unpacking and packing up the dmg are currently done by platform specific tools, unpacking being l10n specific (mounts the image and copies files over)
BTW, l10n repacks could be first to be switched. That would free some slaves for other kinds of builds.
If the packaging pieces are the only platform-specific requirements, we should also look at doing windows repacks on linux. I think all the packaging utilities for windows can work on linux as well.
Agreed,  we'll need to cross-compile NSIS for that. I've seen stuff for that back in the days. Not sure how things look today.

Bonus points in wall-clock time if we do all platforms for one locale on one machine, as we can save all of l10n repo checkout and l10n-merge.
Ooooh, that would be really cool.

Debian has nsis packaged already: https://packages.debian.org/jessie/nsis
This will block us from retiring Mac build hardware. We should tackle it soon. Looping in Callek.
This should be doable using the same tools we're using for packaging the cross-mac builds. Specifically, the `dmg` tool from https://github.com/zarvox/libdmg-hfsplus knows how to unpack and repack dmg files.
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #8)
> This should be doable using the same tools we're using for packaging the
> cross-mac builds. Specifically, the `dmg` tool from
> https://github.com/zarvox/libdmg-hfsplus knows how to unpack and repack dmg
> files.

Might try to sprint on this next week at the work week.
Assignee: nobody → bugspam.Callek
Blocks: 1267425
See Also: → 1318505, 1324052
Blocks: 1351387
Comment on attachment 8852289 [details]
Bug 1185666 - Fix l10n repack scripts and config to support OSX on Linux.

https://reviewboard.mozilla.org/r/124518/#review127060
Attachment #8852289 - Flags: review?(aki) → review+
Comment on attachment 8852290 [details]
Bug 1185666 - Make sure internal tooltool items are available for TC OSX repacks.

https://reviewboard.mozilla.org/r/124520/#review127062
Attachment #8852290 - Flags: review?(aki) → review+
Comment on attachment 8852288 [details]
Bug 1185666 - Move DMG unpack logic to a python script, support linux.

https://reviewboard.mozilla.org/r/124516/#review127318

This is looking great! I would like to re-review after the issues are fixed, so cleraing review for now.

::: python/mozbuild/mozbuild/action/unpack_dmg.py:13
(Diff revision 1)
> +
> +import argparse
> +import sys
> +
> +
> +def unpack_dmg(dmgfile, output_dir, dsstore=None, background=None, icon=None):

Why do you want a separate function here? The py_action will just call main() by default, so you can just have main call dmg.extract_dmg() rather than call a new unpack_dmg() function that has the same arguments.

::: python/mozbuild/mozpack/dmg.py:225
(Diff revision 1)
> +                               destdir])
> +
> +
> +def extract_dmg(dmgfile, output, dsstore=None, icon=None, background=None):
> +    if platform.system() not in ('Darwin', 'Linux'):
> +        raise Exception("Don't know how to build a DMG on '%s'" % platform.system())

"build" -> "extract"

::: python/mozbuild/mozpack/dmg.py:235
(Diff revision 1)
> +    with mozfile.TemporaryDirectory() as tmpdir:
> +        extract_dmg_contents(dmgfile, tmpdir)
> +        if os.path.islink(os.path.join(tmpdir, ' ')):
> +            # Rsync will fail on the presence of this symlink
> +            os.remove(os.path.join(tmpdir, ' '))
> +        rsync(tmpdir, output)

Why do we need to extract_dmg_contents into a tmpdir() only to then rsync tmpdir to output? Couldn't we extract_dmg_contents(dmgfile, output) instead?

::: python/mozbuild/mozpack/dmg.py:237
(Diff revision 1)
> +        if os.path.islink(os.path.join(tmpdir, ' ')):
> +            # Rsync will fail on the presence of this symlink
> +            os.remove(os.path.join(tmpdir, ' '))
> +        rsync(tmpdir, output)
> +
> +        if dsstore:

So if I understand these bits correctly, we are copying .DS_Store, .background, and .VolumeIcon.icns back into the places where the came from under objdir/dist. I tried to backtrack this, but can't find the reason. I presume it's because we blindly copy them back from objdir/dist into the package during make_dmg() - is that correct?

I believe this originates in bug 305131 from the patch "v4, stop the tears from the other eye".

I think we could just leave this in the unpacked dmg, and use the unpacked dmg versions when repacking instead of copying them again from dist. If that sounds like a reasonble thing to you, please file a followup.
Attachment #8852288 - Flags: review?(mshal)
Comment on attachment 8852288 [details]
Bug 1185666 - Move DMG unpack logic to a python script, support linux.

https://reviewboard.mozilla.org/r/124516/#review127318

> Why do we need to extract_dmg_contents into a tmpdir() only to then rsync tmpdir to output? Couldn't we extract_dmg_contents(dmgfile, output) instead?

I did this because its basically what happens during the mount case on OSX now, and I didn't want to pollute the objdir until we have a successful extract.

> So if I understand these bits correctly, we are copying .DS_Store, .background, and .VolumeIcon.icns back into the places where the came from under objdir/dist. I tried to backtrack this, but can't find the reason. I presume it's because we blindly copy them back from objdir/dist into the package during make_dmg() - is that correct?
> 
> I believe this originates in bug 305131 from the patch "v4, stop the tears from the other eye".
> 
> I think we could just leave this in the unpacked dmg, and use the unpacked dmg versions when repacking instead of copying them again from dist. If that sounds like a reasonble thing to you, please file a followup.

this does sound reasonable (I think) I'll file a followup, but it is low on my priority list.
Comment on attachment 8852288 [details]
Bug 1185666 - Move DMG unpack logic to a python script, support linux.

https://reviewboard.mozilla.org/r/124516/#review128190

Looks great! Just one nit.

::: toolkit/mozapps/installer/upload-files.mk:218
(Diff revision 2)
> -    set -ex; \
> -    rm -rf $(ABS_DIST)/unpack.tmp; \
> -    mkdir -p $(ABS_DIST)/unpack.tmp; \
> -    $(_ABS_MOZSRCDIR)/build/package/mac_osx/unpack-diskimage $(UNPACKAGE) /tmp/$(MOZ_PKG_APPNAME)-unpack $(ABS_DIST)/unpack.tmp; \
> -    rsync -a '$(ABS_DIST)/unpack.tmp/$(_APPNAME)' $(MOZ_PKG_DIR); \
> -    if test -n '$(MOZ_PKG_MAC_DSSTORE)' ; then \
> +    $(call py_action,unpack_dmg, \
> +        $(if $(MOZ_PKG_MAC_DSSTORE),--dsstore '$(MOZ_PKG_MAC_DSSTORE)') \
> +        $(if $(MOZ_PKG_MAC_BACKGROUND),--background '$(MOZ_PKG_MAC_BACKGROUND)') \
> +        $(if $(MOZ_PKG_MAC_ICON),--icon '$(MOZ_PKG_MAC_ICON)') \
> +        '$(UNPACKAGE)' '$(MOZ_PKG_DIR)' \
> +        );

Pretty sure this trailing semicolon is unnecessary.
Attachment #8852288 - Flags: review?(mshal) → review+
Pushed by Callek@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7c5c1555236c
Move DMG unpack logic to a python script, support linux. r=mshal
https://hg.mozilla.org/mozilla-central/rev/7c5c1555236c
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Component: Platform Support → Buildduty
Product: Release Engineering → Infrastructure & Operations
Product: Infrastructure & Operations → Infrastructure & Operations Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: