695993 - Update NSPR's config.guess and config.sub, get rid of our non-upstreamed hacks

Status: RESOLVED FIXED

(NSPR :: NSPR, defect, P1)

Description

[Phil Ringnalda (:philor)]

NSPR has a config.guess which may or may not be from when its timestamp says, 2005-10-13 (other than the MozillaHack we no longer want, for some ancient Netscape Windows uname hack), but is certainly from before 64-bit OS X existed, which seems to be part or all of why the SpiderMonkey shell builds choose to build a 64-bit JS shell and a 32-bit NSPR, and then fail to run (bug 678435).

It also has a config.sub which is certainly not from when its timestamp says (or rather, pretends that the timestamp is something other than how upstream does versions, or perhaps doesn't care that it's impossible to tell what it is based on), with two separate hacks, one for wince/winmo that we no longer care wasn't upstreamed like it was supposed to be because we no longer want to carry it around, and one for android that we still have to carry around, so we should label as the MozillaHack it is so we can tell what we haven't upstreamed while updating the next time.

I wouldn't much care about config.sub, since it isn't currently beating my wife, but some ancient djgpp stuff in the current config.guess "Left here for compatibility" talks about how it must be in sync with config.sub, and I don't want to be the one who leaves them out of sync, despite the improbability of anyone compiling NSPR with djgpp.

Comment 1

[Phil Ringnalda (:philor)]

Attachment: fix v.1

Maybe this works. This is just the straight unmodified upstream head of config.guess and config.sub, plus a change to configure to match what upstream does with our NDK r5 target, turning "arm-linux-androideabi" into "arm-unknown-linux-androideabi" rather than what we're currently doing to it, "arm-linuxandroid".

I checked by diffing against the upstream versions of our base, and the current config.guess is just a straight 2005-09-19 plus the hack for whatever "# Netscape's hacked uname" was in the dim dark past, while the current config.sub is just 2009-08-19 plus the hack for winmo we no longer care about and the hack for telling arm-android-eabi from arm-linux-androideabi which I think I'm getting right in the configure.in change.

Comment 2

[Phil Ringnalda (:philor)]

Apparently that lacks that feature that all too many people seem to want, "actually compiles," though I don't know what https://tbpl.mozilla.org/php/getParsedLog.php?id=6999351&full=1&branch=try is trying to tell me.

Comment 3

[Phil Ringnalda (:philor)]

Comment on attachment 568969 [details] [diff] [review]
fix v.1

Oh, change *all* the instances in configure.in, not just the first one. That'll probably make a difference.

Comment 4

[Phil Ringnalda (:philor)]

Except that it won't, since all the others are *-android*|*-linuxandroid* which should match arm-unknown-linux-androideabi just fine, so I have no idea what I'm doing.

Comment 5

[Phil Ringnalda (:philor)]

Fortunately, I'm quite familiar with having no idea what I'm doing.

Comment 6

[Wan-Teh Chang]

Attachment: Upstream config.guess, timestamp 2005-09-19

I studied the CVS history of the config.guess and config.sub
in NSPR and the current git repository of the GNU config project.
I identified the upstream versions of config.guess and config.sub
that NSPR uses. This allows me to generate NSPR's patches for
these files.

Here is the config.guess file, downloaded from
http://git.savannah.gnu.org/cgit/config.git/tree/config.guess?id=920b2fbb36bdeff2c6cdc10b096808da51bb8ae2

Comment 7

[Wan-Teh Chang]

Attachment: Upstream config.sub, timestamp 2009-08-19

This is the upstream config.sub file, downloaded from
http://git.savannah.gnu.org/cgit/config.git/tree/config.sub?id=882ce79b49fa06166256ee05a0c2174b1cc0e081

Comment 8

[Wan-Teh Chang]

Attachment: NSPR's config.guess patch

The timestamp difference can be ignored.

Comment 9

[Wan-Teh Chang]

Attachment: NSPR's config.sub patch

The timestamp difference should be ignored.

Note: The timestamp difference is very big because the timestamp
value in NSPR's config.sub was updated incorrectly. Fortunately
it wasn't too hard to identify the correct upstream config.sub
revision.

In the next few days I will update NSPR's config.guess and config.sub
to the latest upstream revisions, and try to remove as many of
our custom changes as possible.

Comment 10

[Wan-Teh Chang]

Attachment: Update NSPR's config.guess and config.sub scripts to 6947a35648e577c2e3a12d5c88d488c6ea94e1c0

Comment 11

[Wan-Teh Chang]

Attachment: Update NSPR's config.guess and config.sub scripts to 6947a35648e577c2e3a12d5c88d488c6ea94e1c0, v2

According to bug 976648 comment 36 and bug 976648 comment 37,
our Android target triples (arm-linux-android and i386-linux-android)
are different from the Android target triples used in GNU config.sub
upstream. As a temporary workaround I removed the new upstream Android
related code, as was done in Mozilla's config.sub.

Comment 12

[Wan-Teh Chang]

Comment on attachment 8388294 [details] [diff] [review]
Update NSPR's config.guess and config.sub scripts to 6947a35648e577c2e3a12d5c88d488c6ea94e1c0, v2

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

Ulrich:

You're now perhaps the best person to review this patch.
The main difference from your patch in bug 977685 are:

1. a README file that documents the upstream GNU config
revision and where our local patches are. Speaking of
which, the README file probably should also describe
what the local patches are.

2. A patches/ directory that contains our local patches.

I will remove the "MozillaHack" and "winmo" changes
separately, and focus this patch on the update to current
upstream revision. Thanks.

Comment 13

[(not currently active) Ted Mielczarek]

I didn't realize we were carrying local patches. That's unfortunate. We should fix our build config to not require patches. Getting ourselves into a state where we're using latest upstream + patches is an improvement, at least.

Comment 14

[Wan-Teh Chang]

Comment on attachment 8388294 [details] [diff] [review]
Update NSPR's config.guess and config.sub scripts to 6947a35648e577c2e3a12d5c88d488c6ea94e1c0, v2

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

Ted: I believe it is possible to eliminate all local changes.
The only local change I need help with is the Android related
changes. I can take care of the rest.

Comment 15

[Ulrich Weigand]

Comment on attachment 8388294 [details] [diff] [review]
Update NSPR's config.guess and config.sub scripts to 6947a35648e577c2e3a12d5c88d488c6ea94e1c0, v2

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

Looks good to me.  I agree that it is a good idea to at least have the local changes called out as patches.

Comment 16

[Wan-Teh Chang]

Comment on attachment 8388294 [details] [diff] [review]
Update NSPR's config.guess and config.sub scripts to 6947a35648e577c2e3a12d5c88d488c6ea94e1c0, v2

Patch checked in: https://hg.mozilla.org/projects/nspr/rev/c8b3d4100698

Comment 17

[Wan-Teh Chang]

Attachment: Remove NSPR's local patch for config.guess

Comment 18

[Wan-Teh Chang]

Comment on attachment 8389180 [details] [diff] [review]
Remove NSPR's local patch for config.guess

This patch removes NSPR's local patch for config.guess.
The local patch handles Netscape's uname.exe for Windows.
The patched config.guess outputs "i586-pc-msvc" for that
uname.exe. So, I also searched for "msvc" in nspr/configure.in
and removed the relevant code.

In addition, I removed handling of other outputs of that
uname in nspr/configure.in. Specifically:

1. "uname -s" returns "WINNT" or "WIN95".
2. "uname -p" returns "I386".

Note that nspr/configure.in initializes OS_ARCH to the output
of "uname -s" except when we're cross-compiling:

if test -n "$CROSS_COMPILE"; then
    OS_ARCH=`echo $target_os | sed -e 's|/|_|g'`
    OS_RELEASE=
    OS_TEST="${target_cpu}"
    case "${target_os}" in
        linux*)       OS_ARCH=Linux ;;
        solaris*)     OS_ARCH=SunOS OS_RELEASE=5 ;;
        mingw*)       OS_ARCH=WINNT ;;
        wince*)       OS_ARCH=WINCE ;;
        winmo*)       OS_ARCH=WINCE ;;
        darwin*)      OS_ARCH=Darwin ;;
        riscos*)      OS_ARCH=RISCOS ;;
    esac
else
    OS_ARCH=`uname -s | sed -e 's|/|_|g'`
    OS_RELEASE=`uname -r`
    OS_TEST=`uname -m`
fi

So, OS_ARCH is initialized to "WINNT" when we're
cross-compiling for target_os mingw*. This is why I
didn't remove a case statement for the OS_ARCH "WINNT".
That case statement seems wrong for cross-compilation.
I didn't fix it; I just added a comment to note the
problem.

Comment 19

[Wan-Teh Chang]

Attachment: Netscape uname.exe source code: uname.c

For reference, I attached the source code of Netscape's uname.exe.
Here is its output with various command-line options on Windows 7
Service Pack 1:

D:\nspr-tip.3\tmp\buildtools\windows\source\uname\Release>uname -s
WINNT
D:\nspr-tip.3\tmp\buildtools\windows\source\uname\Release>uname -p
I386
D:\nspr-tip.3\tmp\buildtools\windows\source\uname\Release>uname -m
xx
D:\nspr-tip.3\tmp\buildtools\windows\source\uname\Release>uname -r
6.1
D:\nspr-tip.3\tmp\buildtools\windows\source\uname\Release>uname -v
7601
D:\nspr-tip.3\tmp\buildtools\windows\source\uname\Release>uname -n
PHENOM2-PC
D:\nspr-tip.3\tmp\buildtools\windows\source\uname\Release>uname -a
WINNT PHENOM2-PC 6.1 7601 xx I386

Comment 20

[Wan-Teh Chang]

Attachment: Remove NSPR's local patch for config.guess, v2

I studied how CPU_ARCH is used in the NSPR build system.

1. In nspr/configure.in, for Windows builds, CPU_ARCH only
affects the CPU_ARCH_TAG variable, which is a component of
the RELEASE_OBJDIR_NAME variable. Since RELEASE_OBJDIR_DIR
is not being used right now (it's only used by an obsolete
makefile rule named "release", for producing binary releases
at Netscape), the exact value of CPU_ARCH for Windows builds
doesn't really matter.

In this patch I hardcode CPU_ARCH to "x86" when cross-compiling
to i386-mingw32, which has the effect of making CPU_ARCH_TAG
an empty string (since x86 is the most common Windows CPU,
it is omitted in RELEASE_OBJDIR_NAME).

2. In makefiles, CPU_ARCH only matters to Solaris, as this
MXR query shows: http://mxr.mozilla.org/nspr/search?string=CPU_ARCH

Comment 21

[Wan-Teh Chang]

Comment on attachment 8389603 [details] [diff] [review]
Remove NSPR's local patch for config.guess, v2

cls: you and I probably know the history of this code better than
everyone else. If you are still reading Bugzilla mail, could you
review this patch? Thanks.

Comment 22

[Wan-Teh Chang]

Comment on attachment 8389603 [details] [diff] [review]
Remove NSPR's local patch for config.guess, v2

I checked in this patch first because I need to create the
second patch that removes "winmo":
https://hg.mozilla.org/projects/nspr/rev/79643fcac165

I will make any changes you suggest.

Comment 23

[Wan-Teh Chang]

Attachment: Remove NSPR's local changes to config.sub for winmo. Remove wince and winmo support from nspr/configure.in.

Comment 24

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 8391651 [details] [diff] [review]
Remove NSPR's local changes to config.sub for winmo. Remove wince and winmo support from nspr/configure.in.

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

::: configure.in
@@ +557,3 @@
>          DEFINES="$DEFINES -DDEBUG_`echo ${USERNAME} | sed -e 's| |_|g'`"
>          ;;
>      *) 

might as well remove this ws while you're in the neighborhood

Comment 25

[Wan-Teh Chang]

Comment on attachment 8391651 [details] [diff] [review]
Remove NSPR's local changes to config.sub for winmo. Remove wince and winmo support from nspr/configure.in.

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

::: configure.in
@@ +557,3 @@
>          DEFINES="$DEFINES -DDEBUG_`echo ${USERNAME} | sed -e 's| |_|g'`"
>          ;;
>      *) 

I assume you meant the space character at the end of the line? Done.

Comment 26

[Wan-Teh Chang]

Attachment: Remove NSPR's local changes to config.sub for winmo. Remove wince and winmo support from nspr/configure.in. v2

Patch checked in: https://hg.mozilla.org/projects/nspr/rev/fa056198ee14

Comment 27

[Wan-Teh Chang]

The only remaining non-upstream changes are all related to Android.
I don't know how to fix those. Hopefully the Mozilla team will
send me a patch.

In the future our policy should be to always change config.guess
and config.sub through the upstream GNU config project.

Comment 28

[Kai Engert [:KaiE:]]

Comment on attachment 8388294 [details] [diff] [review]
Update NSPR's config.guess and config.sub scripts to 6947a35648e577c2e3a12d5c88d488c6ea94e1c0, v2

rubberstamp=kaie