Closed Bug 1529228 Opened 1 year ago Closed 1 year ago

Export NSS CMS functions required by TB/SM in mailnews/nss-extra.symbols

Categories

(MailNews Core :: Security: S/MIME, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 67.0

People

(Reporter: KaiE, Assigned: KaiE)

References

Details

Attachments

(2 files)

Thunderbird could export the NSS_CMSMessage* it requires in comm-extra-nss-symbols.

This could allow Firefox some space savings.

See these commits parts, for how we can export locally in TB:
https://hg.mozilla.org/releases/comm-esr60/diff/a251d4246dcb/mail/confvars.sh
https://hg.mozilla.org/releases/comm-esr60/diff/a251d4246dcb/comm-extra-nss.symbols

See also bug 1526519, but I think the plan is to just use override the variable, as done in above commits.

See Also: → 1526519

Questions:

  • Will Seamonkey need these extra symbols as well? suite/confvars.sh will need updating accordingly.
  • Does this need to be behind a flag of some sort? My understanding is that this needs to affect all builds regardless, so a flag is not needed.
  • comm-extra-nss.symbols should be moved, probably to mailnews/. The root should be kept clear clear of clutter.
Flags: needinfo?(kaie)

(In reply to Rob Lemley [:rjl] from comment #2)

  • Will Seamonkey need these extra symbols as well? suite/confvars.sh will need updating accordingly.

yes

  • Does this need to be behind a flag of some sort?

no

My understanding is that this needs to affect all builds regardless, so a flag is not needed.

correct

  • comm-extra-nss.symbols should be moved, probably to mailnews/. The root should be kept clear clear of clutter.

That's fine with me. Or maybe it should be moved to toplevel/mail which is the same directory as confvars.sh lives in?

Also, we could just call it nss-extra.symbols

Flags: needinfo?(kaie)

Sounds good. I'm going to put the nss-extra.symbols under mailnews/ as Seamonkey uses code from there but not from mail/ from what I can tell.

Assignee: nobody → rob

Hi Rob, thanks for the offer to help. Note the exact details aren't worked out yet. The above is just an example of what was required for the ESR branch.

For this bug, it's just a suggestion right now. Only if the Firefox team likes my suggestion from bug 1529227, and removes those functions from mozilla/security/nss.symbols, then we'll have to add those symbols in TB/SM.

Rob, I'd like to steal this bug back from you, because there are dependencies between multiple bugs, but I'll ask you to review the change.

Assignee: rob → kaie
Summary: Export NSS CMS functions required by TB in comm-extra-nss-symbols → Export NSS CMS functions required by TB/SM in mailnews/nss-extra.symbols
Attached patch 1529228-v1.patchSplinter Review

This works for me locally on OSX.

(cannot use try, at least I don't know how to do a try-comm-central build with local changes to mozilla-central)

Attachment #9046715 - Flags: review?(rob)

You push your M-C changes to the M-C try without any try syntax. Then modify .taskcluster.yml to point to try and the try changeset here:
https://searchfox.org/comm-central/rev/fd068a83dde907b700883313682899dc27aad874/.taskcluster.yml#149-150

Comment on attachment 9046715 [details] [diff] [review]
1529228-v1.patch

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

This looks good to me.
Attachment #9046715 - Flags: review?(rob) → review+

Jörg, I think you asked about this bug on IRC, if this patch is ready to land.

This bug is part of a cross-repository change, involving NSS, Mozilla and COMM.
My original plan was to wait until after bug 1529227 arrives in m-c (which will break c-c).

If we land this first, (nss.symbols + nss-extra.symbols) will contain duplicates. I haven't tested if this works, but it likely will.

It's up to you if you prefer
(a) land now, and potentially have to backout until the other bug lands
or
(b) delay until the above back landed in m-c.

(Note this change only affects platforms "other than Linux".)

Bug 1529228, Export NSS CMS functions required by TB/SM in mailnews/nss-extra.symbols, r=rjl

Thanks, builds OK on Windows, so let's risk it ;-)

Proper patch for commit (includes the commit header)

Keywords: checkin-needed

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/99d2aa59c5c8
Export NSS CMS functions required by TB/SM in mailnews/nss-extra.symbols. r=rjl

Status: NEW → RESOLVED
Closed: 1 year ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 67.0
You need to log in before you can comment on or make changes to this bug.