Closed Bug 363485 Opened 13 years ago Closed 13 years ago

Build break in JavaXPCOM when building with MSYS


(Core Graveyard :: Java to XPCOM Bridge, defect)

Windows XP
Not set


(Not tracked)



(Reporter: jhpedemonte, Assigned: jhpedemonte)




(2 files, 3 obsolete files)


"/c/Progra~1/Java/jdk1.5.0_08/bin/javac" -source 1.4 -g -classpath ".;MozillaInterfaces.jar" \
                        -sourcepath ".;/c/builds/trunk/mozilla/extensions/java/xpcom/interfaces" -d _java /c/builds/trunk/mozilla/extensions/java/xpcom/interfaces/org/mozilla/xpcom/
c:/builds/trunk/mozilla/extensions/java/xpcom/interfaces/org/mozilla/xpcom/ cannot find symbol
symbol: class IMozilla
public class Mozilla implements IMozilla, IGRE, IXPCOM, IXPCOMError {

Seems that javac doesn't like paths of the style "/c/..." in the sourcepath.
Attached patch patch (obsolete) — Splinter Review
Attachment #248293 - Flags: review?(benjamin)
Comment on attachment 248293 [details] [diff] [review]

pwd -W is MSYS-only... we're not dropping cygwin support yet, and this would kill linux builds as well I think.
Attachment #248293 - Flags: review?(benjamin) → review-
This patch should work for both Cygwin and MSYS, in order to fix the current build breaks.

I would also like to do the same thing for JAVA_CLASSPATH.  However, those paths may include non-directories (such as JAR files);  so using "pwd -W" for MSYS will not work for that.  It would be best if there was a utility equivalent to 'cygpath' for MSYS.  Maybe write our own perl/python script?
Attachment #248293 - Attachment is obsolete: true
(This is just a side comment, not a review comment or anything like that)
Would it be possible to test against HOST_OS_ARCH instead of OS_ARCH? That will make it easier to cross-compile for WINNT from non-WINNT hosts.  You are after all running javac on the host system :)
Attached file msyspath.c (obsolete) —
According to

"You can create a posix path to win32 path conversion utility simply by
creating a binary that does nothing more than output it's argv array. 
As long as that binary doesn't exist in /bin or /usr/bin then MSYS will
do the path conversion for argv before executing the program."

So I created this 'msyspath' util, which will work for both directories and files.

What do you think, Benjamin?  Is it OK to add another binary tool, or would you rather only take scripts at this point?
Comment on attachment 257541 [details] [diff] [review]
patch v2 (checked in)

please use HOST_OS_ARCH in place of OS_ARCH, but otherwise this looks good.
Attachment #257541 - Flags: review+
I don't think I'd like to deal with extra binaries at this point.
While putting the finishing touches on this patch and testing it on my new machine with MSVC 2005, I ran into bug 373047.  So even if I fix this, the MozillaExperimental tbox will still fail.  If I can't fix bug 373047 quickly, I'll just disable the tests.
Comment on attachment 257541 [details] [diff] [review]
patch v2 (checked in)

Checked this patch into the trunk.  Also disabled the tests until bug 373047 is resolved.

Leaving this open for now, until I figure out a better solution for MSYS.
Attachment #257541 - Attachment description: patch v2 → patch v2 (checked in)
Attachment #257561 - Attachment is obsolete: true
Attached patch script patch (obsolete) — Splinter Review
Here is my attempt at a perl script that converts an MSYS path to a Windows path (ignoring relative paths).  I don't know perl that well, but this works for me.
Attachment #257880 - Flags: review?(benjamin)
Comment on attachment 257880 [details] [diff] [review] script patch

perl is overkill. To convert files, you could do something like

$(foreach p,$(1),$(shell cd $(dirname $(p)) && pwd -W)/$(basename $(p)))

That's off the cuff, so it might need some additional/fewer parens or something.
Attachment #257880 - Flags: review?(benjamin) → review-
It's not as easy as that.  Technically, the path that we are given could not exist, in which case 'cd' would fail.  In order to minimize this, I got the very root directory and then called 'pwd -W' on it.  This was easier for me to do in Perl, at the time, but if you want, I can probably get it working in Makefile code using sed.
OK, this seems to work.
Attachment #257880 - Attachment is obsolete: true
Attachment #260761 - Flags: review?(benjamin)
Attachment #260761 - Flags: review?(benjamin) → review+
Attachment #260761 - Attachment description: msys makefile-based patch → msys makefile-based patch (checked in)
Both patches checked in.  -> FIXED
Closed: 13 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.