Closed Bug 1562952 Opened 5 months ago Closed 4 months ago

--disable-verify-mar broken

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox-esr60 unaffected, firefox-esr68 wontfix, firefox67 unaffected, firefox67.0.1 unaffected, firefox68 wontfix, firefox69 wontfix, firefox70 fixed)

RESOLVED FIXED
mozilla70
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- wontfix
firefox67 --- unaffected
firefox67.0.1 --- unaffected
firefox68 --- wontfix
firefox69 --- wontfix
firefox70 --- fixed

People

(Reporter: bytesized, Assigned: glandium)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

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.

glandium, will you be able to take this?

Flags: needinfo?(mh+mozilla)
Assignee: nobody → mh+mozilla
Component: Application Update → General
Flags: needinfo?(mh+mozilla)
Product: Toolkit → Firefox Build System

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.

Priority: P3 → --

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).

Flags: needinfo?(robert.strong.bugs)

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.

Flags: needinfo?(robert.strong.bugs)

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

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.

Flags: needinfo?(robert.strong.bugs)

That sounds good to me

Flags: needinfo?(robert.strong.bugs)

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.

Pushed by mh@glandium.org:
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

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.

Flags: needinfo?(mh+mozilla)
Pushed by mh@glandium.org:
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

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?

Flags: needinfo?(robert.strong.bugs)

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

Flags: needinfo?(robert.strong.bugs)
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70

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

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

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

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.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla70 → ---

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

Flags: needinfo?(mh+mozilla)
Pushed by mh@glandium.org:
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
Status: REOPENED → RESOLVED
Closed: 4 months ago4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
Flags: needinfo?(mh+mozilla)

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

Flags: needinfo?(mh+mozilla)

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.

Flags: needinfo?(mh+mozilla)

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

You need to log in before you can comment on or make changes to this bug.