Remove sqlcipher linkage hack to appease Rust 1.50 (fixes SYNC-1999)

[dmose]

So, the root cause of this problem is a regression between rustc 1.49 and 1.50 (changelog). I think it's quite likely that rust-lang/cargo#8969 was the breaking PR. There was a subsequent fix for some piece of the problem in rust-lang/cargo#9065, but that didn't fix the issue that we're seeing, which is still in rustc 1.52 nightly. (BTW, I (heart) how trivial cargo makes it to test whether a given problem still exists in the nightly version of the compiler).

I haven't tried to nail down exactly what this piece is and filed a bug in rustlang because I don't have a sense of how long that's likely to take. Furthermore, I think the cheapest way to fix this is simply to unconditionally link NSS in all cases. It will slow down our build process a bit, but I'd be surprised if it were by an interesting amount. My assumption is that doing this won't make any difference to the executable produced, since I wouldn't expect anything to be pulled in from NSS unless there are symbols missing at link to.

This is particularly compelling, because, as I understand it, we're considering getting rid of sqlcipher entirely in #960, which would just make the root issue go away. Is this correct?

Note that I'm forcing everything to 1.50 in this PR to make sure that we're sure we solve the problem we're trying to.

My plan is to remove that commit before we land, just to keep things separate. I could land them all at once, though...

I have yet to test how this fix affects build in m-c, if at all...

Pull Request checklist

• Quality: This PR builds and tests run cleanly
  • automation/all_tests.sh runs to completion and produces no failures
  • Note: For changes that need extra cross-platform testing, consider adding [ci full] to the PR title.
• Tests: This PR includes thorough tests or an explanation of why it does not.
  • This is really just to fix stuff busted with the Rust 1.50 compiler.
• Changelog: This PR includes a changelog entry in CHANGES_UNRELEASED.md or an explanation of why it does not need one
  • There are no user-facing changes.
  • Any breaking changes to Swift or Kotlin binding APIs are noted explicitly
• Dependencies: This PR follows our dependency management guidelines
  • Any new dependencies are accompanied by a summary of the due dilligence applied in selecting them.
  • No dependency changes

[dmose]

I assume that the reason these cargo fmt changes are required is because cargo fmt must have changed something in 1.50, otherwise, I'd expect these changes to be required on main as well. Perhaps I should split them out into a separate commit...

[dmose]

I can't actually tell whether it's a problem on beta or nightly. The reason is that because we seeming to be doing the builds that we use for tests and benchmarking with hard-errors on clippy warnings, the builds on those channels abort before they finish. Any idea how hard this would be to fix?

[dmose]

@mhammond @rfk would love your thoughts on my proposed approach here...

[rfk]

The reason is that because we seeming to be doing the builds that we use for tests and benchmarking with
hard-errors on clippy warnings, the builds on those channels abort before they finish. Any idea how hard this would be to fix?

By "fix this" do you mean "stop the warnings from failing the build"? The magic sauce there the -D warnings in all_clippy_checks.sh.

[rfk]

I think the cheapest way to fix this is simply to unconditionally link NSS in all cases

This seems reasonable to me; if "slightly longer builds" is the only downside then it's not worth investing in any fancier, especially given longer-term plans to move away from sqlcipher.

[rfk]

r+ for the approach here; I would also support going ahead and changing 1.43 to 1.50 in the version pinning, on the understand that mozilla-central is also pinned to that version.

[mhammond]

rs=me!

[dmose]

The failing sync tests are just because a live server they depend upon is down.