Mention NSPR libs in NSS version check error message.

Mention NSPR libs in NSS version check error message.

Some Linux distributions don't have tightly controlled dependencies
and allow older nspr libs in conjunction with newer nss libs that
can lead to confusion.

BUG=33163
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=37401

[Evan Martin, 2010-01-27 18:06:26]

http://codereview.chromium.org/558002/diff/1/2
File base/nss_util.cc (right):

http://codereview.chromium.org/558002/diff/1/2#newcode84
base/nss_util.cc:84: "and corresponding NSPR libs.";
Is there any way for us to know what NSPR version NSS wants?  Then we could
inform the user about what they're missing...

[Craig, 2010-01-27 18:48:59]

On 2010/01/27 18:06:26, Evan Martin wrote:
> http://codereview.chromium.org/558002/diff/1/2
> File base/nss_util.cc (right):
> 
> http://codereview.chromium.org/558002/diff/1/2#newcode84
> base/nss_util.cc:84: "and corresponding NSPR libs.";
> Is there any way for us to know what NSPR version NSS wants?  Then we could
> inform the user about what they're missing...

Grepping for PR_VERSION didn't look too exciting but I'll dig some more later
and see if I can find a way. We could probably distinguish between a NSS and a
NSPR failure by calling NSS check version with a suitably low "that's gotta
work" version and if it fails then it must be NSPR that's wrong...

[Evan Martin, 2010-01-27 18:50:39]

Wan-teh: any thoughts on how we can make this error message more useful?

[Craig, 2010-01-27 19:02:06]

One way that might work is if we played games with PR_VersionCheck similar to
the existing localtime tricks using RTLD_NEXT but I need to sift through what is
already available first - I'm totally unfamiliar with nss - I haven't even
compiled it from source yet or studied the code to any degree so there may be a
more reasonable way to do it.

[Evan Martin, 2010-01-27 19:14:49]

On 2010/01/27 19:02:06, Craig wrote:
> One way that might work is if we played games with PR_VersionCheck similar to
> the existing localtime tricks using RTLD_NEXT but I need to sift through what
is
> already available first - I'm totally unfamiliar with nss - I haven't even
> compiled it from source yet or studied the code to any degree so there may be
a
> more reasonable way to do it.

Yikes, way too much effort!  I think what you have is ok, or maybe we can call
PR_VersionCheck() with the appropriate includes
  https://developer.mozilla.org/en/PR_VersionCheck

[wtc, 2010-01-27 19:19:07]

LGTM, with a more informative error message.
I suggested one below.

http://codereview.chromium.org/558002/diff/1/2
File base/nss_util.cc (right):

http://codereview.chromium.org/558002/diff/1/2#newcode84
base/nss_util.cc:84: "and corresponding NSPR libs.";
There is no easy way to know what NSPR version NSS wants.
It's the version of NSPR that the NSS package was compiled
against.

The message can say:

    We depend on NSS >= 3.12.3.
    If NSS is up to date, please
    update NSPR to the latest version.

NSPR is extremely stable, so it is safe
to tell users to just update to the latest
version.

[Craig, 2010-01-27 20:13:20]

On 2010/01/27 19:19:07, wtc wrote:
> LGTM, with a more informative error message.
> I suggested one below.

OK.

> http://codereview.chromium.org/558002/diff/1/2
> File base/nss_util.cc (right):
> 
> http://codereview.chromium.org/558002/diff/1/2#newcode84
> base/nss_util.cc:84: "and corresponding NSPR libs.";
> There is no easy way to know what NSPR version NSS wants.
> It's the version of NSPR that the NSS package was compiled
> against.
> 
> The message can say:
> 
>     We depend on NSS >= 3.12.3.
>     If NSS is up to date, please
>     update NSPR to the latest version.
> 
> NSPR is extremely stable, so it is safe
> to tell users to just update to the latest
> version.

Done.

Thank you.

[Evan Martin, 2010-01-27 20:16:51]

LGTM++

http://codereview.chromium.org/558002/diff/5001/4002
File base/nss_util.cc (right):

http://codereview.chromium.org/558002/diff/5001/4002#newcode85
base/nss_util.cc:85: "update NSPR to the latest version.\n";
I guess putting in newlines might be confusing because the first line will have
the whole filename+line number prefix on it while the others won't.  But don't
let me bikeshed.  :)

[wtc, 2010-01-27 20:27:47]

Craig,

LGTM.  Please remove the \n characters as Evan suggested.

[Craig, 2010-01-28 14:36:12]

On 2010/01/27 20:27:47, wtc wrote:
> Craig,
> 
> LGTM.  Please remove the \n characters as Evan suggested.

Fixed. Will land shortly ...