1057783 - Add support for little-endian powerpc64

Status: RESOLVED FIXED

(NSS :: Build, defect, P2)

Description

[Mike Hommey [:glandium]]

Add support of little-endian powerpc64

Comment 1

[Mike Hommey [:glandium]]

Attachment: Add support for little-endian powerpc64

Comment 2

[Kai Engert (:KaiE:)]

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

[Kai Engert (:KaiE:)]

Comment on attachment 8477933 [details] [diff] [review]
Add support for little-endian powerpc64

r- for now, unless you can show the comma is correct

Comment 4

[Mike Hommey [:glandium]]

Attachment: Add support for little-endian powerpc64

You're right, and the test was removing ppc64 too.

Comment 5

[Kai Engert (:KaiE:)]

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).

Comment 6

[Mike Hommey [:glandium]]

Attachment: Add support for little-endian powerpc64

Let's go with this.

Comment 7

[Mike Hommey [:glandium]]

Would you mind landing this for me in the nss repo?

Comment 8

[Kai Engert (: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...

Comment 9

[Wan-Teh Chang]

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