1237140 - nss.symbols cannot include NSS_EXTRA_SYMBOLS_FILE: not in context

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Patrick Cloke [:clokep]]

>    Creating library purplexpcom.lib and object purplexpcom.exp
> 
> sipe-xml.obj : warning LNK4049: locally defined symbol _xmlFree imported
> 
> sipe-crypt-nss.obj : error LNK2019: unresolved external symbol _PK11_PubDecryptRaw referenced in function _sipe_crypt_rsa_decrypt
> 
> sipe-crypt-nss.obj : error LNK2019: unresolved external symbol _PK11_PubEncryptRaw referenced in function _sipe_crypt_rsa_encrypt
> 
> sipe-crypt-nss.obj : error LNK2019: unresolved external symbol _PK11_Verify referenced in function _sipe_crypt_verify_rsa
> 
> sipe-digest-nss.obj : error LNK2019: unresolved external symbol _PK11_SaveContextAlloc referenced in function _sipe_digest_md5_end
> 
> sipe-digest-nss.obj : error LNK2019: unresolved external symbol _PK11_RestoreContext referenced in function _sipe_digest_md5_end
> 
> purplexpcom.dll : fatal error LNK1120: 5 unresolved externals

I believe this is from bug 1235132, specifically https://hg.mozilla.org/mozilla-central/rev/186450f22aab

This code was added/used in https://hg.mozilla.org/mozilla-central/rev/0fdd92c0a958 and http://hg.mozilla.org/comm-central/rev/05bb70d5b231

Not really sure what's going on here, but I've been able to reproduce locally.

Comment 1

[Patrick Cloke [:clokep]]

Looks like this is a bug in the build configuration. Patch coming up.

Comment 2

[Patrick Cloke [:clokep]]

Attachment: extra-symbols.patch

Briefly, what's happening here is that EXTRA_NSS_SYMBOLS_FILE is appropriately ifdef'd / included [0], but EXTRA_NSS_SYMBOLS_FILE can NEVER be added to the context of the preprocessor, since it is inside of buildconfig.substs. buildconfig.defines is the only part of the build config added to the preprocessor context [1]. This updates the context to also include buildconfig.substs.

Not sure if I should include tests / how I would run them etc.

[0] https://hg.mozilla.org/mozilla-central/rev/186450f22aab#l4.274
[1] https://hg.mozilla.org/mozilla-central/rev/3b61f9f09b04#l2.22

Comment 3

[Patrick Cloke [:clokep]]

It seems like glandium is on vacation, so I set gps as the reviewer (since he reviewed bug 1235132), please let me know if there's a more appropriate reviewer.

Comment 4

[Mike Hommey [:glandium]]

Comment on attachment 8704453 [details] [diff] [review]
extra-symbols.patch

It'd be better to make NSS_EXTRA_SYMBOLS_FILE part of buildconfig.defines instead, which you can do by replacing the AC_SUBST with a proper AC_DEFINE.

Comment 5

[Patrick Cloke [:clokep]]

I did a try push with a different fix for this:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=234ccfe878a1

Assuming that looks good, I'll put a patch up for review.

Comment 6

[Patrick Cloke [:clokep]]

Attachment: extra-symbols.patch v2

This uses AC_DEFINE instead of AC_SUBST. I'm not a master at modifying configure.in, so please let me know if I did something crazy. Most of the try builds have passed: https://treeherder.mozilla.org/#/jobs?repo=try&revision=234ccfe878a1

I tested that this lets me build Instantbird locally.

Comment 7

[Mike Hommey [:glandium]]

Comment on attachment 8707995 [details] [diff] [review]
extra-symbols.patch v2

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

I have a better, generic idea, which will help for other things as well.

Comment 8

[Mike Hommey [:glandium]]

Attachment: Move DefinesAction from mozbuild.action.process_install_manifest to mozbuild.util

Comment 9

[Mike Hommey [:glandium]]

Attachment: Allow file_generate scripts to take additional flags

Currently, file_generate scripts can only accept input and output file
name, but sometimes, one would like for them to take additional flags.

Comment 10

[Mike Hommey [:glandium]]

Attachment: Add support for local defines in generate_symbols_file

generate_symbols_file only supports the global defines, and completely
ignores DEFINES from the same moz.build the SYMBOLS_FILE is defined.
This fixes this misfeature.

Comment 11

[Mike Hommey [:glandium]]

Attachment: Pass NSS_EXTRA_SYMBOLS_FILE down to nss.symbols processing

Comment 12

[Jean-Yves Avenard [:jya]]

Attachment: don't export unecessary symbol using VS2015.

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dc75f7ff583e
https://hg.mozilla.org/integration/mozilla-inbound/rev/2ef9b742008c
https://hg.mozilla.org/integration/mozilla-inbound/rev/0de5c8c0d5d2
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac8978a872e8

Comment 14

[Mike Hommey [:glandium]]

Comment on attachment 8709373 [details] [diff] [review]
don't export unecessary symbol using VS2015.

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

::: media/ffvpx/ffvpxcommon.mozbuild
@@ +77,5 @@
>          '-wd4057', '-wd4204', '-wd4706', '-wd4305', '-wd4152', '-wd4324',
>          '-we4013', '-wd4100', '-wd4214', '-wd4307', '-wd4273', '-wd4554',
>      ]
>      if CONFIG['_MSC_VER'] < '1900':
> +        DEFINES['_MSVC_2013_OR_LOWER'] = True

doesn't need to start with an underscore.

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c2538cf8b8dd

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/dc75f7ff583e
https://hg.mozilla.org/mozilla-central/rev/2ef9b742008c
https://hg.mozilla.org/mozilla-central/rev/0de5c8c0d5d2
https://hg.mozilla.org/mozilla-central/rev/ac8978a872e8
https://hg.mozilla.org/mozilla-central/rev/c2538cf8b8dd