Closed Bug 1237140 Opened 5 years ago Closed 5 years ago

nss.symbols cannot include NSS_EXTRA_SYMBOLS_FILE: not in context

Categories

(Firefox Build System :: General, defect)

Unspecified
Windows
defect
Not set
normal

Tracking

(firefox46 fixed)

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: clokep, Assigned: glandium)

References

Details

Attachments

(5 files, 2 obsolete files)

>    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.
Looks like this is a bug in the build configuration. Patch coming up.
Blocks: 1235132
Severity: blocker → normal
Component: Other → Build Config
Product: Instantbird → Core
Whiteboard: [1.6-blocking]
Version: trunk → Trunk
Summary: Build-bustage: "purplexpcom.dll : fatal error LNK1120: 5 unresolved externals" on Windows → nss.symbols cannot include NSS_EXTRA_SYMBOLS_FILE: not in context
Attached patch extra-symbols.patch (obsolete) — Splinter Review
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
Attachment #8704453 - Flags: review?(gps)
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 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.
Attachment #8704453 - Flags: review?(gps) → review-
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.
Attached patch extra-symbols.patch v2 (obsolete) — Splinter Review
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.
Attachment #8704453 - Attachment is obsolete: true
Attachment #8707995 - Flags: review?(mh+mozilla)
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.
Attachment #8707995 - Flags: review?(mh+mozilla)
Assignee: clokep → mh+mozilla
Attachment #8709232 - Flags: review?(gps)
Currently, file_generate scripts can only accept input and output file
name, but sometimes, one would like for them to take additional flags.
Attachment #8709233 - Flags: review?(gps)
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.
Attachment #8709234 - Flags: review?(gps)
Attachment #8707995 - Attachment is obsolete: true
Attachment #8709237 - Flags: review?(gps)
Attachment #8709232 - Flags: review?(gps) → review+
Attachment #8709233 - Flags: review?(gps) → review+
Attachment #8709234 - Flags: review?(gps) → review+
Attachment #8709237 - Flags: review?(gps) → review+
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.
Attachment #8709373 - Flags: review?(mh+mozilla) → review+
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.