Closed Bug 1057783 Opened 6 years ago Closed 6 years ago

Add support for little-endian powerpc64

Categories

(NSS :: Build, defect, P2)

PowerPC
Linux
defect

Tracking

(Not tracked)

RESOLVED FIXED
3.17.1

People

(Reporter: glandium, Assigned: glandium)

Details

Attachments

(1 file, 2 obsolete files)

Add support of little-endian powerpc64
Attachment #8477933 - Flags: review?(kaie)
Assignee: nobody → mh+mozilla
Status: NEW → ASSIGNED
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 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-
You're right, and the test was removing ppc64 too.
Attachment #8484507 - Flags: review?(kaie)
Attachment #8477933 - Attachment is obsolete: true
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).
Let's go with this.
Attachment #8484580 - Flags: review?(kaie)
Attachment #8484507 - Attachment is obsolete: true
Attachment #8484507 - Flags: review?(kaie)
Attachment #8484580 - Flags: review?(kaie) → review+
Would you mind landing this for me in the nss repo?
Flags: needinfo?(kaie)
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: 6 years ago
Flags: needinfo?(kaie)
Resolution: --- → FIXED
Target Milestone: --- → 3.17.1
OS: All → Linux
Priority: -- → P2
Hardware: All → PowerPC
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.