166105 - Perl-based platform detection needs to support cygwin perl

Status: RESOLVED FIXED

(SeaMonkey :: Build Config, defect)

Description

[[e-mail] (away - not reading bugmail)]

This is a spin-off from bug 144551, where Chris Seawood correctly pointed out 
that this code doesn't support platform detection when using cygwin perl; it 
only works with ActivePerl from ActiveState.

Let me know, if you want me to own this.

http://lxr.mozilla.org/seamonkey/source/config/make-chromelist.pl#71
http://lxr.mozilla.org/seamonkey/source/embedding/config/gen_mn.pl#15

Comment 1

[[e-mail] (away - not reading bugmail)]

The code in question was written by Gerv and Adam, I put them on cc to keep 
them in the loop.

Comment 2

[Adam Lock]

perl -e "print $^O;" returns "cygwin"

Comment 3

[[e-mail] (away - not reading bugmail)]

Adam, over in bug 144551 we are looking at using this:

my $win32 = ($^O =~ /((MS)?win32)|cygwin|os2/i) ? 1 : 0;
my $macos = ($^O =~ /MacOS|darwin/i)     ? 1 : 0;
my $unix  = !($win32 || $macos) ? 1 : 0;

The *nix platform default was suggested by Chris Seeawood to avoid dealing with
the detection of a multitude of *nix flavors.

Comment 4

[Adam Lock]

Attachment: Patch for embedding

This is the patch for embedding. I haven't changed anything except used a perl
switch-like block and added cygwin detection. The requirements for
make-chromelist.pl are probably more stringent.

Even with the patch applied, I suspect one of the packages in cygwin perl is
broken, at least it is for me. Our gen_mn.pl uses the FindBin package (I'll CC
alecf in a moment). In cygwin, I see this error:

Cannot chdir to
/cygdrive/c/m/source/mozilla/embedding/config/c:/m/source/mozilla/embedding/config/:No
such file or directory at /usr/lib/perl5/5.6.1/cygwin-multi/FindBin.pm line 162

BEGIN failed--compilation aborted at
/usr/lib/perl5/5.6.1/cygwin-multi/FindBin.pm line 166.
Compilation failed in require at c:/m/source/mozilla/embedding/config/gen_mn.pl
line 7.
BEGIN failed--compilation aborted at
c:/m/source/mozilla/embedding/config/gen_mn.pl line 7.

Comment 5

[Adam Lock]

CC'ing Alec to determine what FindBin was added for.

Comment 6

[Alec Flett]

findbin is supposed to be part of the current perl. have you updated to the
latest cygwin perl package?

Anyhow, findbin is used to include files that are in the same directory as the
perl executable. This allows embeddors to run gen_mn.pl from a directory other
than mozilla/embedding/config

Basically its like adding the directory that a binary lives on to the PATH, such
that the binary can load DLLs at runtime that exist in that directory, no matter
what directory the executable is run from.

Comment 7

[Alec Flett]

I at least have FindBin.pm in both c:\cygwin\lib\perl5\5.6.1 and
c:\cygwin\lib\perl5\5.6.1\cygwin-multi and I'm 99.44% certain that I didn't
install it there myself.

Comment 8

[Adam Lock]

I think the problem is in how the perl script is invoked. On my system, the
Makefile expands it out to this (ignoring some stuff):

perl -Ic:/m/source/mozilla/embedding/config
c:/m/source/mozilla/embedding/config/gen_mn.pl

The include path isn't the problem, but the fully qualified path to the script
is. I think FindBin is getting confused by the
c:/m/source/mozilla/embedding/config/gen_mn.pl path and doesn't know how to
merge it properly with the value returned by getcwd() which is
/cygdrive/c/m/source/mozilla/embedding/config. 

Therefore it makes a total mess of it and appends the two together generating
the chdir() error that I saw. FindBin.pm does have some Win32 specific code, but
like us it's testing $^O for "Win32" and fails when it contains "cygwin".

$^O appears to be a read-write variable, so I'll see if can temporarily change
it to Win32 before it enters this package and restore it when it comes out.

Comment 9

[hacker formerly known as [e-mail]]

The problem is that cygwin perl is balking on our use of dos paths.  Fir
example, it requires that we use a /cygdrive path for the include path. 
Apparently, it doesn't canonicalize the paths when it is comparing them in its
modules.  The use of FindBin in make-jars.pl was disabled if using cygwin perl
for that reason.  

Comment 10

[Adam Lock]

Attachment: Embedding patch v2

Patch puts conditional test around including FindBin when using cygwin.

The embedding makefile is still failing later in make-jars.pl which cannot find
mozLock.pm. It looks like @INC turns the -Ic:/m/source/mozilla/config into c
/m/source/mozilla/config for some reason.

Comment 11

[Alec Flett]

I'm not a huge fan of this solution - seawood, isn't there some sort of utility
to convert a DOS-style path to a cygwin-style path? It seems like it would be
better to just pass the cygwin style path to -I

Comment 12

[hacker formerly known as [e-mail]]

Alec, we already do that via mozilla/build/cygwin-wrapper.  We convert only the
include args when using cygwin perl since cygwin perl uses colon as a path
separator for -I.  The problem appears to be that FindBin is taking the name of
the script, which uses a dospath, and doing a path comparison using the include
args without taking in consideration the path differences.  I modified the
wrapper to convert all paths to cygdrive paths when using cygwin perl and that
fixes this problem.  I'll have to run it over the build to see if there are any
other side-effects.

Comment 13

[Adam Lock]

Can I get an r on the patch?

Comment 14

[hacker formerly known as [e-mail]]

Comment on attachment 97628 [details] [diff] [review]
Embedding patch v2

r=cls

Comment 15

[Asa Dotzler [:asa]]

Comment on attachment 97628 [details] [diff] [review]
Embedding patch v2

a=asa for checkin to 1.2 (on behalf of drivers).

Comment 16

[rpotts (gone)]

Comment on attachment 97628 [details] [diff] [review]
Embedding patch v2

sr=[e-mail]

Comment 17

[hacker formerly known as [e-mail]]

Attachment: Fix os-detection for make-chromelist.pl

Comment 18

[Gervase Markham [:gerv]]

cls: Making unix as (!win && !mac) is fine, but I'm not sure if cygwin should be
lumped in with Windows or with Unix. I have a feeling it uses "/" paths and so
should be in the Unix pot. But I'm not certain either way - do you have
experience which says one or the other?

Gerv

Comment 19

[hacker formerly known as [e-mail]]

Gerv:  You are correct that cygwin perl uses posix path separators internally
but that's irrelevant to this script as mac & dos paths are converted to posix
paths anyway to match the cvs paths. Plus, we need to set win32 to make sure
that the proper platform specific jar files are used.

Comment 20

[Gervase Markham [:gerv]]

OK, ignore me :-) r=gerv.

Gerv

Comment 21

[Asa Dotzler [:asa]]

Comment on attachment 104813 [details] [diff] [review]
Fix os-detection for make-chromelist.pl

a=asa for checkin to 1.2 (on behalf of drivers)

Comment 22

[hacker formerly known as [e-mail]]

Patch has been checked in.