730862 - Disable signmar by default and provide an option to enable it

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect, P1)

Description

[Brian R. Bondy [:bbondy]]

Attachment: Patch v1.

When repackaging builds we do a selective build of specific modules.  There is no need for signmar when we do this but we do need to build mar.  This task is to provide a define that can be used to selectively not build signmar when building modules/libmar.

Comment 1

[[e-mail] (:bhearsum)]

Comment on attachment 600947 [details] [diff] [review]
Patch v1.

I'm testing out this patch by hand on a slave.

Comment 2

[Brian R. Bondy [:bbondy]]

You probably already see this, but you also have to have NO_SIGN_VERIFY defined.

Comment 3

[Brian R. Bondy [:bbondy]]

Comment on attachment 600947 [details] [diff] [review]
Patch v1.

Going to disable by default and add the option to enable it in Nightly and Debug config/mozconfigs

That way no buildconfig for repackaging MARs will be needed

Comment 4

[Brian R. Bondy [:bbondy]]

Attachment: Patch v2.

Comment 5

[Brian R. Bondy [:bbondy]]

Attachment: Patch v3.

Added in the default mozconfigs for all platforms not just Windows to build signmar so that the libmar sign/verify tests can run by default on all platforms.

Comment 6

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

Any particular reason to build to set --enable-signmar on debug builds ? We don't produce mar files or set --enable-update-packaging there.

Comment 7

[[e-mail] (:bhearsum)]

We talked a bit on IRC about that and figured it would be good to test signmar compilation with any code changes, and there might be some tests too?

Comment 8

[Brian R. Bondy [:bbondy]]

We don't use signmar for producing MAR files at all, we use the dist/host/bin/mar program instead.  signmar is just for the signing server.

The reason to build signmar for both Nightly and Debug is the same, so that modules/libmar's unit tests can always be run.

Comment 9

[Brian R. Bondy [:bbondy]]

It sounds a bit confusing but --enable-signmar doesn't enable signing of MAR files, it enables building of the signmar program.

Comment 10

[Robert Strong (they/them - no direct email)]

Comment on attachment 601035 [details] [diff] [review]
Patch v3.

Might be a good thing to run the new --enable-signmar by khuey

Comment 11

[[e-mail] (:bhearsum)]

Comment on attachment 601035 [details] [diff] [review]
Patch v3.

This patch worked for me in a minimal test. I didn't do a full l10n build, but I did go through the autoconf, configure, and module building it does. modules/libmar built without issue.

Comment 12

[Brian R. Bondy [:bbondy]]

I tried a full build locally without the mozconfig option and it didn't build signmar so it looks good to me as well.  Try and Elm tests also passed.  I'm doing a second nightly now and will make sure nothing is broken.  I'll push soon as long as khuey gives an sr+.

Comment 13

[Brian R. Bondy [:bbondy]]

Comment on attachment 601035 [details] [diff] [review]
Patch v3.

This is very important because repackaged builds are currently broken, a fast super-review would be greatly appreciated :D

Comment 14

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 601035 [details] [diff] [review]
Patch v3.

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

I'm fine with adding --enable-signmar.  Deferring to rs on everything else.

Comment 15

[Brian R. Bondy [:bbondy]]

http://hg.mozilla.org/mozilla-central/rev/5439f4751116

Comment 16

[Brian R. Bondy [:bbondy]]

http://hg.mozilla.org/releases/mozilla-aurora/rev/18840d884c97

Comment 17

[[e-mail] (:bhearsum)]

I don't think this worked...I was just looking for a signmar binary in an OS X objdir on mozilla-inbound, and couldn't find it:
moz2-darwin10-slave07:obj-firefox cltbld$ ls -l i386/modules/libmar/
total 8
-rw-rw-r--   1 cltbld  admin  2312 Mar 19 01:27 Makefile
drwxrwxr-x  12 cltbld  admin   408 Mar 19 20:44 src
drwxrwxr-x   6 cltbld  admin   204 Mar 19 20:44 tool
moz2-darwin10-slave07:obj-firefox cltbld$ ls -l x86_64/modules/libmar/
total 8
-rw-rw-r--   1 cltbld  admin  2312 Mar 19 02:47 Makefile
drwxrwxr-x  12 cltbld  admin   408 Mar 19 22:03 src
drwxrwxr-x   6 cltbld  admin   204 Mar 19 22:03 tool
moz2-darwin10-slave07:obj-firefox cltbld$ pwd
/builds/slave/m-in-osx64/build/obj-firefox
moz2-darwin10-slave07:obj-firefox cltbld$ cat ../.mozconfig
. $topsrcdir/build/macosx/universal/mozconfig

# Universal builds override the default of browser (bug 575283 comment 29)
ac_add_options --enable-application=browser

ac_add_options --enable-update-channel=${MOZ_UPDATE_CHANNEL}
ac_add_options --enable-update-packaging
ac_add_options --enable-codesighs
ac_add_options --disable-install-strip
ac_add_options --enable-signmar

# Nightlies only since this has a cost in performance
ac_add_options --enable-js-diagnostics

# Needed to enable breakpad in application.ini
export MOZILLA_OFFICIAL=1

export MOZ_TELEMETRY_REPORTING=1
mk_add_options MOZ_MAKE_FLAGS="-j4"

ac_add_options --with-macbundlename-prefix=Firefox

# Treat warnings as errors in directories with FAIL_ON_WARNINGS.
ac_add_options --enable-warnings-as-errors

# Package js shell.
export MOZ_PACKAGE_JSSHELL=1

Comment 18

[Brian R. Bondy [:bbondy]]

I spotted the problem here in configure.in:

> MOZ_ARG_ENABLE_BOOL(sign-mar,

It should instead be:

> MOZ_ARG_ENABLE_BOOL(signmar,

So basically we are never building it right now.

Please change this locally for now if you need to build it.  I don't want to land this until I am around and can test it. It will enable some tests as well and I suspect some of the tests with fail with the major version increase, but I'm not sure about that yet.

Comment 19

[Brian R. Bondy [:bbondy]]

Attachment: Fix for signmar option not working

Will ask for review after I try the associated tests with 14.0a1 (instead of 13.0a1, I think there are some binary MAR comparisons that might have broken)

Comment 20

[Brian R. Bondy [:bbondy]]

Attachment: Signmar test

The create mar test in particular had to have the channel and version set to what the reference MARs have.

Also I found a bug in the binary comparison check for the create mar test. 
The reference MARs actually have the following data encoded: 
Version: 13.0a1
Channel: @MAR_CHANNEL_ID@
So I match that via the optional signmar command line parameters that can be passed when creating a MAR.

Comment 21

[Brian R. Bondy [:bbondy]]

Attachment: fix for other platforms

I had to create different MARs for windows vs other platforms because of the permissions inside the MAR file.  This patch fixes that since the tests compare the binary MAR file data.  The try tests were failing on non windows platforms before this patch.

Comment 22

[[e-mail] (:bhearsum)]

I need an OS X signmar...let's throw this patch at try.

Comment 23

[[e-mail] (:bhearsum)]

Apparently autosign is broken right now. I pushed this by hand: https://tbpl.mozilla.org/?tree=Try&rev=0ed2f9764b3e

Comment 24

[[e-mail] (:bhearsum)]

(In reply to Ben Hearsum [:bhearsum] from comment #23)
> Apparently autosign is broken right now. I pushed this by hand:
> https://tbpl.mozilla.org/?tree=Try&rev=0ed2f9764b3e

This didn't build signmar, either, unless it's only built for the x86_64 target.

Comment 25

[Brian R. Bondy [:bbondy]]

Looked like you pushed the fix to the tests but not the fix to the configure.in which is the important one:
https://bugzilla.mozilla.org/attachment.cgi?id=608944

Comment 26

[[e-mail] (:bhearsum)]

I applied that patch locally, re-ran autoconf & configure, and built modules/libmar by hand (for purposes of speed). I ended up with a 'signmar' binary that was actually a 'mar' binary:
try-mac64-slave14:bin cltbld$ ./signmar
usage:
  mar [-H MARChannelID] [-V ProductVersion] [-C workingDir] {-c|-x|-t|-T} archive.mar [files...]
  mar [-H MARChannelID] [-V ProductVersion] [-C workingDir] -i unsigned_archive_to_refresh.mar

Comment 27

[[e-mail] (:bhearsum)]

the mozconfig looked like this:
try-mac64-slave14:build cltbld$ cat .mozconfig
. $topsrcdir/build/macosx/universal/mozconfig

# Universal builds override the default of browser (bug 575283 comment 29)
ac_add_options --enable-application=browser

ac_add_options --enable-update-channel=${MOZ_UPDATE_CHANNEL}
ac_add_options --enable-update-packaging
ac_add_options --enable-codesighs
ac_add_options --disable-install-strip
ac_add_options --enable-signmar

# Nightlies only since this has a cost in performance
ac_add_options --enable-js-diagnostics

# Needed to enable breakpad in application.ini
export MOZILLA_OFFICIAL=1

export MOZ_TELEMETRY_REPORTING=1
mk_add_options MOZ_MAKE_FLAGS="-j4"

ac_add_options --with-macbundlename-prefix=Firefox

# Treat warnings as errors in directories with FAIL_ON_WARNINGS.
ac_add_options --enable-warnings-as-errors

# Package js shell.

Comment 28

[Brian R. Bondy [:bbondy]]

Attachment: signmar

Here's a bin I just build using the configure.in patch (change sign-mar to signmar).  I also had to add: ac_add_options --enable-signmar in my .mozconfig.

It was built with:
Mac OS X Lion 10.7.3

I think you probably have it setup locally correctly but you have to do a clean build.  I think somehow your signmar option is on but some object files are compiled wrongly.

Comment 29

[[e-mail] (:bhearsum)]

Weird. I just started from scratch on a new machine, and it worked fine. Thanks bbondy, I have no clue what I was doing wrong =\.

Comment 30

[Brian R. Bondy [:bbondy]]

https://hg.mozilla.org/mozilla-central/rev/f75b7814f698
https://hg.mozilla.org/mozilla-central/rev/ea7a6ca0a55c
https://hg.mozilla.org/mozilla-central/rev/576a14e57ea6