1562952 - --disable-verify-mar broken

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Robin Steuber (they/them) [:bytesized]]

I attempted to build Firefox (on Windows 10), using this mozconfig:

ac_add_options --target=x86_64-pc-mingw32
ac_add_options --host=x86_64-pc-mingw32
mk_add_options AUTOCLOBBER=1
ac_add_options --disable-verify-mar
ac_add_options --enable-release
ac_add_options --enable-debug-symbols
ac_add_options --disable-optimize
mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/obj-64-release-debug

Multiple attempts to build were all met with this error message:

 0:23.36 lld-link: error: undefined symbol: mar_read_entire_file
 0:23.36 >>> referenced by c:\mozilla-central\modules\libmar\tool\mar.c:343
 0:23.36 >>>               mar.obj:(main)
 0:23.36 lld-link: error: undefined symbol: mar_verify_signatures
 0:23.36 >>> referenced by c:\mozilla-central\modules\libmar\tool\mar.c:375
 0:23.36 >>>               mar.obj:(main)

Removing this line from my mozconfig seems to fix the problem:

ac_add_options --disable-verify-mar

I sometimes need this feature to test update, so I would prefer that this flag continue to be maintained.

:mhowell thinks that this was regressed by Bug 1547217, but I have not verified this.

Comment 1

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

glandium, will you be able to take this?

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The product::component has been changed since the backlog priority was decided, so we're resetting it.
For more information, please visit auto_nag documentation.

Comment 3

[Mike Hommey [:glandium]]

Ok, I got something working, but all that got me is to ask myself more questions than I had coming in, so I'll ask this: does the combination of --enable-signmar --disable-verify-mar actually makes sense? Because it looks like from the code that it doesn't, in which case this is almost entirely a configure issue (although there are some things that seem badly #ifdef'ed in the code too).

Comment 4

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

I don't recall the reasons why this was done this way but I found bug 730862 and it appears --enable-signmar was added for l10n repackaging. I can't say for certain it is still needed since browser/confvars.sh always sets MOZ_ENABLE_SIGNMAR=1. I think it should be fine to combine the two if the reason it was added in bug 730862 is no longer necessary but I don't know if that is the case.

Comment 5

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

Actually, if bug 730862 is no longer needed then I think it would be better to always build signmar.

Comment 6

[Mike Hommey [:glandium]]

bug 903135 made --enable-signmar the default for browser/.
bug 1207890 made it auto-disable on artifact and l10n builds.
bug 1158870 removed the explicit --enable-signmar in mozconfigs.

Looking at the history, it looks like it was made optional because originally it wasn't supported on all platforms.

So I guess we could always build signmar, and make --disable-verify-mar mean only disable verification in the updater.

Does that sound like a plan?

Kirk, FWIW, --disable-signmar --disable-verify-mar should build without modification, and should work for what you want.

Comment 7

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

That sounds good to me

Comment 8

[Mike Hommey [:glandium]]

Attachment: Bug 1562952 - #ifdef-out NSS and certificate-related flags with NO_SIGN_VERIFY.

Comment 9

[Mike Hommey [:glandium]]

Attachment: Bug 1562952 - Always build signmar when mar is built.

When we build mar, there is no reason not to build signmar as well. It
used to be optional because not all platforms were supported, but they
are now.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fc4a6e8f6e34
#ifdef-out NSS and certificate-related flags with NO_SIGN_VERIFY. r=rstrong
https://hg.mozilla.org/integration/autoland/rev/12099ffad9ca
Always build signmar when mar is built. r=nalexander

Comment 11

[Andreea Pavel [:apavel]]

Backed out for failing xpcshell at test_create.js

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=255666892&resultStatus=testfailed%2Cbusted%2Cexception&revision=12099ffad9cae748d060872883a4b2c5e560c3ee

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=255666892&repo=autoland&lineNumber=1564

Backout: https://hg.mozilla.org/integration/autoland/rev/bf063ce64c58d2e22962f0b7f81c1fa307bb19f4

Comment 12

[Mike Hommey [:glandium]]

This failed because the second patch has the side effect of enabling signmar on android, but the signmar executable is not copied to the test archive (it's explicitely skipped on android) and the tests don't work without signmar. Enabling the copy still fails because signmar ends up missing the executable flag, and there might be other issues hidden after that. Considering the tests weren't running on android before this change, I'll reland with the tests disabled on android, and will file a followup to maybe make the test work on android.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/250696e18e7d
#ifdef-out NSS and certificate-related flags with NO_SIGN_VERIFY. r=rstrong
https://hg.mozilla.org/integration/autoland/rev/6b09d4c0868c
Always build signmar when mar is built. r=nalexander

Comment 14

[Mike Hommey [:glandium]]

The libmar tests have, in fact, never run on Android before. I don't think we actually ever use mar on Android, so maybe we don't even want to run them ever?

Comment 15

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

There isn't any value running these tests on Android.

Comment 16

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/250696e18e7d
https://hg.mozilla.org/mozilla-central/rev/6b09d4c0868c

Comment 17

[Tom Prince [:tomprince]]

Is signmar still useful? It is not used in automation.

Comment 18

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

It is used when signing mars but aiui a copy of signmar is put on one of the servers. So, we need it in automation so the tests can verify it is working properly

Comment 19

[Tom Prince [:tomprince]]

We use https://github.com/mozilla/build-mar to sign mar in automation here.

Comment 20

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

Assuming it is possible to create and sign test mar files locally and for distributions I'd be fine with removing all of this code as long as we can also remove the mar host binary too.

Comment 21

[Bogdan Tara[:bogdan_tara | bogdant]]

Backed out 2 changesets (Bug 1562952) for mar-tools failures

Failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&tochange=1e6dcacb3ab034b215d25a5cad501e8546e4c4e4&searchStr=toolchains%2Copt%2Ctoolchain%2Cbuilds%2Cfor%2Clinux%2C64-bits%2Ctoolchain-linux64-mar-tools%2Ctl%28mar-tools%29&fromchange=f9a6c7aa2f763cd6cee6d566d647ff2b7b1c15df&selectedJob=256014942

Backout link: https://hg.mozilla.org/mozilla-central/rev/4db5e839c57bd6823f02e51040f4dfda3b5c0c94

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=256014942&repo=autoland&lineNumber=580

[task 2019-07-11T20:07:44.010Z] 0:25.04 creating ./config.data
[task 2019-07-11T20:07:44.025Z] 0:25.05 Creating config.status
[task 2019-07-11T20:07:44.182Z] 0:25.21 Reticulating splines...
[task 2019-07-11T20:07:44.250Z] 0:25.28 Traceback (most recent call last):
[task 2019-07-11T20:07:44.250Z] 0:25.28 File "/builds/worker/workspace/build/src/configure.py", line 133, in <module>
[task 2019-07-11T20:07:44.250Z] 0:25.28 sys.exit(main(sys.argv))
[task 2019-07-11T20:07:44.251Z] 0:25.28 File "/builds/worker/workspace/build/src/configure.py", line 44, in main
[task 2019-07-11T20:07:44.251Z] 0:25.28 return config_status(config)
[task 2019-07-11T20:07:44.251Z] 0:25.28 File "/builds/worker/workspace/build/src/configure.py", line 128, in config_status
[task 2019-07-11T20:07:44.251Z] 0:25.28 return config_status(args=[], **encode(sanitized_config, encoding))
[task 2019-07-11T20:07:44.251Z] 0:25.28 File "/builds/worker/workspace/build/src/python/mozbuild/mozbuild/config_status.py", line 142, in config_status
[task 2019-07-11T20:07:44.251Z] 0:25.28 definitions = list(definitions)
[task 2019-07-11T20:07:44.251Z] 0:25.28 File "/builds/worker/workspace/build/src/python/mozbuild/mozbuild/frontend/emitter.py", line 194, in emit
[task 2019-07-11T20:07:44.251Z] 0:25.28 objs = list(self._emit_libs_derived(contexts))
[task 2019-07-11T20:07:44.251Z] 0:25.28 File "/builds/worker/workspace/build/src/python/mozbuild/mozbuild/frontend/emitter.py", line 269, in _emit_libs_derived
[task 2019-07-11T20:07:44.251Z] 0:25.28 self._link_libraries(context, obj, variable, idl_sources)
[task 2019-07-11T20:07:44.251Z] 0:25.28 File "/builds/worker/workspace/build/src/python/mozbuild/mozbuild/frontend/emitter.py", line 351, in _link_libraries
[task 2019-07-11T20:07:44.251Z] 0:25.28 self._link_library(context, obj, variable, path)
[task 2019-07-11T20:07:44.251Z] 0:25.28 File "/builds/worker/workspace/build/src/python/mozbuild/mozbuild/frontend/emitter.py", line 433, in _link_library
[task 2019-07-11T20:07:44.251Z] 0:25.28 context)
[task 2019-07-11T20:07:44.251Z] 0:25.28 mozbuild.frontend.reader.SandboxValidationError:
[task 2019-07-11T20:07:44.252Z] 0:25.28 ==============================
[task 2019-07-11T20:07:44.252Z] 0:25.28 FATAL ERROR PROCESSING MOZBUILD FILE
[task 2019-07-11T20:07:44.252Z] 0:25.28 ==============================
[task 2019-07-11T20:07:44.252Z] 0:25.28 The error occurred while processing the following file or one of the files it includes:
[task 2019-07-11T20:07:44.252Z] 0:25.28 /builds/worker/workspace/build/src/modules/libmar/tool/moz.build
[task 2019-07-11T20:07:44.252Z] 0:25.28 The error occurred when validating the result of the execution. The reported error is:
[task 2019-07-11T20:07:44.252Z] 0:25.28 USE_LIBS contains "nspr", which does not match any LIBRARY_NAME in the tree.
[task 2019-07-11T20:07:44.293Z] 0:25.32 *** Fix above errors and then restart with
[task 2019-07-11T20:07:44.293Z] 0:25.32 "./mach build"
[task 2019-07-11T20:07:44.293Z] 0:25.32 client.mk:111: recipe for target 'configure' failed
[task 2019-07-11T20:07:44.293Z] 0:25.32 make: *** [configure] Error 1
[fetches 2019-07-11T20:07:44.314Z] removing /builds/worker/workspace/build
[fetches 2019-07-11T20:07:51.834Z] finished
[taskcluster 2019-07-11 20:07:52.189Z] === Task Finished ===
[taskcluster 2019-07-11 20:07:53.123Z] Unsuccessful task run with exit code: 2 completed in 215.43 seconds

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/040c1590b378
#ifdef-out NSS and certificate-related flags with NO_SIGN_VERIFY. r=rstrong
https://hg.mozilla.org/integration/autoland/rev/5d98741cb9de
Always build signmar when mar is built. r=nalexander

Comment 23

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/040c1590b378
https://hg.mozilla.org/mozilla-central/rev/5d98741cb9de

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Not sure what the implications of this are outside of our CI environment. Is this something we should consider backporting to help downstream consumers?

Comment 25

[Mike Hommey [:glandium]]

It only really causes problem for people that at the same time do --enable-updater (the default) and --disable-verify-mar. I expect that's a very limited number of people that doesn't include downstreams.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

Thanks, sounds like we can just let this ride the trains then.