1320991 - --with-system-{nss,nspr} fail to build with --enable-tests

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Jan Beich]

$ echo ac_add_options --with-system-nspr >>.mozconfig
$ echo ac_add_options --with-system-nss >>.mozconfig
$ ./mach build
[...]
objdir/_virtualenv/bin/python -m mozbuild.action.preprocessor --depend .deps/gdb-tests-gdb.py.pp  -DNDEBUG=1 -DTRIMMED=1 -DMOZ_HAS_MOZGLUE -DEXPORT_JS_API -Dtopsrcdir=js/src -DBUILD_CTYPES=1 -DCPP_THROW_NEW='throw()' -DCROSS_COMPILE='' -DD_INO=d_ino -DE10S_TESTING_ONLY=1 -DEDITLINE=1 -DENABLE_INTL_API=1 -DENABLE_TESTS=1 -DEXPOSE_INTL_API=1 -DGTEST_HAS_RTTI=0 -DHAVE_64BIT_BUILD=1 -DHAVE_CLOCK_MONOTONIC=1 -DHAVE_CPP_AMBIGUITY_RESOLVING_USING=1 -DHAVE_CPP_DYNAMIC_CAST_TO_VOID_PTR=1 -DHAVE_CPUID_H=1 -DHAVE_DIRENT_H=1 -DHAVE_DLOPEN=1 -DHAVE_GETC_UNLOCKED=1 -DHAVE_GETOPT_H=1 -DHAVE_GMTIME_R=1 -DHAVE_I18N_LC_MESSAGES=1 -DHAVE_INTTYPES_H=1 -DHAVE_LANGINFO_CODESET=1 -DHAVE_LIBM=1 -DHAVE_LOCALECONV=1 -DHAVE_LOCALTIME_R=1 -DHAVE_MALLOC_USABLE_SIZE=1 -DHAVE_MBRTOWC=1 -DHAVE_NETINET_IN_H=1 -DHAVE_NL_TYPES_H=1 -DHAVE_POSIX_FADVISE=1 -DHAVE_POSIX_FALLOCATE=1 -DHAVE_POSIX_MEMALIGN=1 -DHAVE_RES_NINIT=1 -DHAVE_SETLOCALE=1 -DHAVE_SSIZE_T=1-DHAVE_STDINT_H=1 -DHAVE_STRNDUP=1 -DHAVE_SYS_MOUNT_H=1 -DHAVE_SYS_QUEUE_H=1 -DHAVE_SYS_STATVFS_H=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_THREAD_TLS_KEYWORD=1 -DHAVE_TM_ZONE_TM_GMTOFF=1 -DHAVE_UNISTD_H=1 -DHAVE_VALLOC=1 -DHAVE_VA_COPY=1 -DHAVE_VA_LIST_AS_ARRAY=1 -DHAVE_VISIBILITY_ATTRIBUTE=1 -DHAVE_VISIBILITY_HIDDEN_ATTRIBUTE=1 -DHAVE_WCRTOMB=1 -DHAVE___CXA_DEMANGLE=1 -DJS_CODEGEN_X64=1 -DJS_CPU_X64=1 -DJS_DEFAULT_JITREPORT_GRANULARITY=3 -DJS_HAS_CTYPES=1 -DJS_PUNBOX64=1 -DJS_TRACE_LOGGING=1 -DMALLOC_H='<malloc_np.h>' -DMALLOC_USABLE_SIZE_CONST_PTR=const -DMOZILLA_UAVERSION='"53.0"' -DMOZILLA_VERSION='"53.0a1"' -DMOZILLA_VERSION_U=53.0a1 -DMOZJS_MAJOR_VERSION=53 -DMOZJS_MINOR_VERSION=0 -DMOZ_BUILD_APP=js -DMOZ_DLL_SUFFIX='".so"' -DMOZ_GLUE_IN_PROGRAM=1 -DMOZ_STACKWALKING=1 -DMOZ_STATIC_JS=1 -DMOZ_UPDATE_CHANNEL=default -DNIGHTLY_BUILD=1 -DNO_NSPR_10_SUPPORT=1 -DSPIDERMONKEY_PROMISE=1 -DSTATIC_JS_API=1 -DSTDC_HEADERS=1 -DU_STATIC_IMPLEMENTATION=1 -DU_USING_ICU_NAMESPACE=0 -DVA_COPY=va_copy -DXP_UNIX=1 -D_REENTRANT=1 -D_THREAD_SAFE=1 -DAB_CD= 'js/src/gdb/gdb-tests-gdb.py.in' -o '../../../dist/bin/gdb-tests-gdb.py'
../../../config/nsinstall -R -m 644 '../../../dist/bin/gdb-tests-gdb.py' 'objdir/js/src/gdb'
gmake[5]: Leaving directory 'objdir/js/src/gdb'
gmake[5]: Entering directory 'objdir/modules/libmar/tests'
gmake[5]: *** No rule to make target '../../../dist/bin/libnspr4.so', needed by '../../../_tests/xpcshell/modules/libmar/tests/unit/libnspr4.so'.  Stop.

Comment 1

[Andrew Overholt [:overholt]]

Does this not break Linux distro builds because they don't --enable-tests? JC, do you know?

Comment 2

[J.C. Jones [:jcj] (he/him)]

No idea. Maybe Jed knows?

Comment 3

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

I don't, but could this be a regression from bug 1253775?

Comment 4

[Michael Shal [:mshal]]

I don't think this worked even before bug 1253775, but I believe the fix is to just use USE_LIBS instead of the mess of manually handling nss libs.

Comment 5

[Michael Shal [:mshal]]

Attachment: Bug 1320991 - Support --with-system-{nss,nspr} in modules/libmar;

Review commit: https://reviewboard.mozilla.org/r/100262/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/100262/

Comment 6

[Michael Shal [:mshal]]

Comment on attachment 8820880 [details]
Bug 1320991 - Support --with-system-{nss,nspr} in modules/libmar;

Jan, can you confirm that this works for you?

Comment 7

[Jan Beich]

Comment on attachment 8820880 [details]
Bug 1320991 - Support --with-system-{nss,nspr} in modules/libmar;

$ ./mach build
$ ./mach test modules/libmar/tests
 0:00.28 LOG: MainThread INFO Using at most 32 threads.
 0:00.28 SUITE_START: MainThread 3
 0:00.28 TEST_START: Thread-3 modules/libmar/tests/unit/test_sign_verify.js
 0:00.28 TEST_START: Thread-2 modules/libmar/tests/unit/test_extract.js
 0:00.29 TEST_START: Thread-1 modules/libmar/tests/unit/test_create.js
 0:00.42 TEST_END: Thread-2 PASS
 0:00.43 TEST_END: Thread-1 PASS
 0:00.76 TEST_END: Thread-3 PASS
 0:00.79 LOG: MainThread INFO INFO | Result summary:
 0:00.79 LOG: MainThread INFO INFO | Passed: 3
 0:00.79 LOG: MainThread INFO INFO | Failed: 0
 0:00.79 LOG: MainThread INFO INFO | Todo: 0
 0:00.79 LOG: MainThread INFO INFO | Retried: 0
 0:00.79 SUITE_END: MainThread
Summary
=======

Ran 3 tests
Expected results: 3
Unexpected results: 0

OK

However, you can completely remove[1] the section as modules/libmar/tests/moz.build doesn't build any binaries worth having USE_LIBS for. "signmar" is built by modules/libmar/tool/moz.build instead.

[1] https://svnweb.freebsd.org/ports/head/www/firefox/files/patch-bug1320991?revision=427388&view=markup

Comment 8

[Michael Shal [:mshal]]

Comment on attachment 8820880 [details]
Bug 1320991 - Support --with-system-{nss,nspr} in modules/libmar;

Hmm, good point. I'll update the patch.

Comment 9

[Michael Shal [:mshal]]

Comment on attachment 8820880 [details]
Bug 1320991 - Support --with-system-{nss,nspr} in modules/libmar;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/100262/diff/1-2/

Comment 10

[Mike Hommey [:glandium]]

Comment on attachment 8820880 [details]
Bug 1320991 - Support --with-system-{nss,nspr} in modules/libmar;

https://reviewboard.mozilla.org/r/100262/#review101138

This is probably fine, but it does something that, while I can see how it's related to the description, is completely different and may have implications that I don't know whether they have been assessed because there was no comment about it, either in the commit message or the bug.

That is, what this being removed here is copying nspr and nss to the test package for xpcshell libmar tests. I have no idea whether this breaks those tests, and whether this has been tested not to be breaking those tests.

Comment 11

[Michael Shal [:mshal]]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=844c6a093ab621514a74a7c07485dea1e3348a0a

I've also updated the commit message with some more information. Let me know if you think of other possible implications that should be tested beforehand.

Comment 12

[Michael Shal [:mshal]]

Comment on attachment 8820880 [details]
Bug 1320991 - Support --with-system-{nss,nspr} in modules/libmar;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/100262/diff/2-3/

Comment 13

[David Bolter [:davidb] (NeedInfo me for attention)]

Per today's triage meeting, updating flags to fix-optional meaning we want a fix but we don't need to track this weekly.

Comment 14

[Mike Hommey [:glandium]]

Comment on attachment 8820880 [details]
Bug 1320991 - Support --with-system-{nss,nspr} in modules/libmar;

https://reviewboard.mozilla.org/r/100262/#review104356

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/78756c08d6e8
Support --with-system-{nss,nspr} in modules/libmar; r=glandium

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/78756c08d6e8

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Probably not worth uplifting to 51 at this point since we're about to spin the release candidate build, but it sounds like something we should uplift to Aurora in support of the next ESR.

Comment 18

[Michael Shal [:mshal]]

Do people build ESR versions with custom mozconfigs? It should be harmless to uplift either way, but I don't know if it would be useful.

Comment 19

[Jan Beich]

(In reply to Michael Shal [:mshal] from comment #18)
> Do people build ESR versions with custom mozconfigs?

--with-system-nss --with-system-nspr are popular downstream but often together with --disable-tests. And tests can be used to debug downstream builds, especially if they have many patches applied.

Comment 20

[Michael Shal [:mshal]]

Comment on attachment 8820880 [details]
Bug 1320991 - Support --with-system-{nss,nspr} in modules/libmar;

Approval Request Comment
[Feature/Bug causing the regression]:
[User impact if declined]: Building Aurora (and eventually ESR, see #c17), will fail for developers who build with these custom mozconfig flags.
[Is this code covered by automated tests?]: Our build automation does not build with this particular set of flags.
[Has the fix been verified in Nightly?]:
[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: It should not be
[Why is the change risky/not risky?]: The build config change is related only to test binaries produced by the build.
[String changes made/needed]:

Comment 21

[Julien Cristau [:jcristau]]

Comment on attachment 8820880 [details]
Bug 1320991 - Support --with-system-{nss,nspr} in modules/libmar;

make tests work with system-{nss,nspr}, aurora52+

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/998f11970867