Closed
Bug 1057783
Opened 10 years ago
Closed 10 years ago
Add support for little-endian powerpc64
Categories
(NSS :: Build, defect, P2)
Tracking
(Not tracked)
RESOLVED
FIXED
3.17.1
People
(Reporter: glandium, Assigned: glandium)
Details
Attachments
(1 file, 2 obsolete files)
927 bytes,
patch
|
KaiE
:
review+
|
Details | Diff | Splinter Review |
Add support of little-endian powerpc64
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8477933 -
Flags: review?(kaie)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → mh+mozilla
Status: NEW → ASSIGNED
Comment 2•10 years ago
|
||
Comment on attachment 8477933 [details] [diff] [review]
Add support for little-endian powerpc64
ifeq (,$(filter-out ppc,ppc64le,$(OS_TEST)))
Looking at other uses of filter-out in coreconf, and looking at documentation, I think you want a space between ppc and ppc64le?
Like this:
ifeq (,$(filter-out ppc ppc64le,$(OS_TEST)))
Do you agree?
https://www.gnu.org/software/make/manual/html_node/Text-Functions.html
(See the $filter command which lists multiple items to filter, I assume the $filter-out is used in the same way.)
Comment 3•10 years ago
|
||
Comment on attachment 8477933 [details] [diff] [review]
Add support for little-endian powerpc64
r- for now, unless you can show the comma is correct
Attachment #8477933 -
Flags: review?(kaie) → review-
Assignee | ||
Comment 4•10 years ago
|
||
You're right, and the test was removing ppc64 too.
Attachment #8484507 -
Flags: review?(kaie)
Assignee | ||
Updated•10 years ago
|
Attachment #8477933 -
Attachment is obsolete: true
Comment 5•10 years ago
|
||
It seems it was intentional that ppc isn't included here. Further down, the final "else", includes a list the "else" is intended to handle, and ppc is included in the list.
The existing code ignores the setting of USE_64 if OS_TEST=ppc. Your new patch has the potential to introduce a new behaviour, if anyone has set USE_64=1 on a ppc system.
I don't know how likely that is. I have a slight preference to keep the existing behaviour, meaning we'd remove ppc from your patch. What do you think?
I'll give you r=kaie for the patch you prefer (with or without "ppc" in the list).
Assignee | ||
Updated•10 years ago
|
Attachment #8484507 -
Attachment is obsolete: true
Attachment #8484507 -
Flags: review?(kaie)
Updated•10 years ago
|
Attachment #8484580 -
Flags: review?(kaie) → review+
Assignee | ||
Comment 7•10 years ago
|
||
Would you mind landing this for me in the nss repo?
Flags: needinfo?(kaie)
Comment 8•10 years ago
|
||
Checked in:
https://hg.mozilla.org/projects/nss/rev/eb792774acc3
FYI, I accidentally included other changes in the first attempt
https://hg.mozilla.org/projects/nss/rev/cad641b1475d
which I immediately backed out
https://hg.mozilla.org/projects/nss/rev/ea4cee936403
Sorry...
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: needinfo?(kaie)
Resolution: --- → FIXED
Target Milestone: --- → 3.17.1
Updated•10 years ago
|
OS: All → Linux
Priority: -- → P2
Hardware: All → PowerPC
Comment 9•10 years ago
|
||
Comment on attachment 8484580 [details] [diff] [review]
Add support for little-endian powerpc64
Patch (as part of NSS_3_17_1_BETA2) pushed to mozilla-central:
https://hg.mozilla.org/mozilla-central/rev/e1c5c185b707
You need to log in
before you can comment on or make changes to this bug.
Description
•