Make AM_PATH_NSS compatible with input version in the form of major.minor

RESOLVED FIXED in Firefox 38

Status

()

Core
Build Config
RESOLVED FIXED
3 years ago
6 months ago

People

(Reporter: hectorz, Assigned: hectorz)

Tracking

unspecified
mozilla39
Points:
---

Firefox Tracking Flags

(firefox38 fixed, firefox39 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

3 years ago
Currently, with system nss version being 3.17.4, and AM_PATH_NSS checking for 3.18, the test will pass with:

nss_config_major_version = "3"
nss_config_minor_version = "17"
nss_config_micro_version = "4"

min_nss_major_version = "3"
min_nss_minor_version = "1"
min_nss_micro_version = ""

The "." between "\)" and "\(" should be "\."
(Assignee)

Comment 1

3 years ago
Created attachment 8582380 [details] [diff] [review]
Patch
Assignee: nobody → bzhao
Attachment #8582380 - Flags: review?(mh+mozilla)
Comment on attachment 8582380 [details] [diff] [review]
Patch

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

AM_PATH_NSPR has the same problem, can you fix it too?
Attachment #8582380 - Flags: review?(mh+mozilla) → feedback+
(Assignee)

Comment 3

3 years ago
(In reply to Mike Hommey [:glandium] from comment #2)
> 
> AM_PATH_NSPR has the same problem, can you fix it too?

Sure, I'll upload a patch later.
Note, ISTR a bug was filed in the past for some previous 3.* release that didn't have a .0 (which, iirc, older releases used to have).
Duplicate of this bug: 989616
(Assignee)

Comment 6

3 years ago
Created attachment 8582847 [details] [diff] [review]
Patch, with AM_PATH_NSPR

I assumed I should not touch /nsprpub/config/nspr.m4 with this patch, and prepare a separate patch for NSPR instead? It's already not identical with /build/autoconf/nspr.m4.

Also, would you recommend a try syntax for this patch? Sheriffs will ask for a try run when I request checkin. Thanks.
Attachment #8582380 - Attachment is obsolete: true
Attachment #8582847 - Flags: review?(mh+mozilla)
Attachment #8582847 - Flags: review?(mh+mozilla) → review+
(In reply to Hector Zhao [:hectorz] from comment #6)
> Created attachment 8582847 [details] [diff] [review]
> Patch, with AM_PATH_NSPR
> 
> I assumed I should not touch /nsprpub/config/nspr.m4 with this patch, and
> prepare a separate patch for NSPR instead? It's already not identical with
> /build/autoconf/nspr.m4.

Yes, please. Send it Ted Mielczarek's way.

> Also, would you recommend a try syntax for this patch? Sheriffs will ask for
> a try run when I request checkin. Thanks.

There is nothing you can do on try that will exercise this code path (well, there is, but it would require a lot of work to build nspr and nss independently and then build firefox against that).
Keywords: checkin-needed
Whiteboard: [npotb]
(Assignee)

Comment 8

3 years ago
Created attachment 8582902 [details] [diff] [review]
Patch, for NSPR

(In reply to Mike Hommey [:glandium] from comment #7)
> (In reply to Hector Zhao [:hectorz] from comment #6)
> > I assumed I should not touch /nsprpub/config/nspr.m4 with this patch, and
> > prepare a separate patch for NSPR instead? It's already not identical with
> > /build/autoconf/nspr.m4.
> 
> Yes, please. Send it Ted Mielczarek's way.

Patch against http://hg.mozilla.org/projects/nspr/
Attachment #8582902 - Flags: review?(ted)
https://hg.mozilla.org/integration/mozilla-inbound/rev/88d41ff68289
Keywords: checkin-needed
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/8aaa0b07dda6 (next to bug 1127801) for being the cause of mass bustage:
One example:
https://treeherder.mozilla.org/logviewer.html#?job_id=8052676&repo=mozilla-inbound
Flags: needinfo?(bzhao)
*very* unlikely to be the cause of the bustage, because it's not code that's actually hit for firefox builds.
Comment hidden (obsolete)
Comment hidden (obsolete)
(Assignee)

Comment 14

3 years ago
(In reply to Hector Zhao [:hectorz] from comment #12)
> (In reply to Mike Hommey [:glandium] from comment #11)
> > *very* unlikely to be the cause of the bustage, because it's not code that's
> > actually hit for firefox builds.
> 
> I agree, AM_PATH_{NSPR,NSS} will not be used unless you set
> "--with-system-{nspr,nss}" in mozconfig.

Sth. is wrong with my previous try push.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f1d84a5f4627 on top of:
https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=37d3dcbf23a9
(Assignee)

Comment 15

3 years ago
(In reply to Hector Zhao [:hectorz] from comment #14)
> 
> Sth. is wrong with my previous try push.
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=f1d84a5f4627 on top
> of:
> https://treeherder.mozilla.org/#/jobs?repo=mozilla-
> central&revision=37d3dcbf23a9

The subset of tests in this try run is now green.
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/9b2d059dffc9
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/9b2d059dffc9
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
status-firefox38: --- → affected
Whiteboard: [npotb] → [npotb][checkin-needed-beta]
https://hg.mozilla.org/releases/mozilla-beta/rev/0fce0415d8b4
status-firefox38: affected → fixed
Whiteboard: [npotb][checkin-needed-beta]
Attachment #8582902 - Flags: review?(ted)
You need to log in before you can comment on or make changes to this bug.