Benjamin Smedberg Assignee
	Description • 16 years ago

When configure is called with a Windows-style absolute path, as it is with pymake, there are some minor adjustments that must be made:

* configure uses : as a separator by default. We rewrite autoconf macros to avoid this
* configure detects absolute paths using a glob /* . We rewrite autoconf macros to avoid this
* MSYS perl expects MSYS paths everywhere, and can't deal with windows-style paths at all. We use a wrapper to get around this until we can excise perl entirely
* use $(abspath) in a few Windows-only codepaths to get the right directory style

If this patch is acceptable, I have a matching NSPR version.

	Benjamin Smedberg Assignee
	Comment 1 • 16 years ago

Comment on attachment 369556 [details] [diff] [review]
Hack configure to accept windows-style absolute paths, rev. 1

This is slightly borken, please hold off reviewing it.

	(not currently active) Ted Mielczarek
	Comment 3 • 15 years ago

Comment on attachment 370676 [details] [diff] [review]
Hack configure to accept windows-style absolute paths, rev. 1.1

FWIW, I reviewed most of this by generating configure before/after this patch, then running both copies through `sed -re "s/^#line [0-9]+/#line /" -e "s/configure:[0-9]+:
/configure:/"` to get rid of line number differences.

+++ b/build/msys-perl-wrapper

This is kind of gross, but I guess we don't have any other choices until we ditch perl completely, eh?

     dnl MinGW/MSYS does not need CYGWIN_WRAPPER
+        PERL="/bin/sh ${srcdir}/build/msys-perl-wrapper"

Perhaps add a more descriptive comment there?

 ifeq (,$(filter-out WINNT WINCE,$(OS_ARCH)))
-_VPATH_SRCS = $(if $(filter /%,$<),$<,$(CURDIR)/$<)
+_VPATH_SRCS = $(abspath $<)
 else
 _VPATH_SRCS = $<
 endif

Can we just use core_abspath there and ditch the ifdef?

Otherwise, looks as good as a patch full of m4 can look.

	(not currently active) Ted Mielczarek
	Comment 4 • 15 years ago

I decided against changing that, since it'd change behavior and I wasn't 100% sure on it. Pushed to m-c:
http://hg.mozilla.org/mozilla-central/rev/1c4751f81b1d

	(not currently active) Ted Mielczarek
	Comment 5 • 15 years ago

bsmedberg pushed a followup fix:
http://hg.mozilla.org/mozilla-central/rev/f655090e14c3

	Rob Arnold [:robarnold]
	Comment 6 • 15 years ago

This change appears to have broken all my object directories. I have them set them up as sibling directories of the source directories. Here's how I set one up:
MOZCONFIG=../mozconfig ../mozilla-central/configure && make -s -j2

The build complains about being unable to find ../mozilla-central/build/msys-perl-wrapper despite the fact that it seems to exist - I suspect that the relative path is to blame since when I run:
MOZCONFIG=../mozconfig /c/Users/Rob/mozilla/mozilla-central/configure && make -s -j2

It appears to build just fine (build hasn't finished yet, but it appears to have made more progress).

Is there any chance that this can be fixed to work with relative paths?

	Rob Arnold [:robarnold]
	Comment 7 • 15 years ago

Oh, I should not that I am not using pymake.

	Benjamin Smedberg Assignee
	Comment 8 • 15 years ago

That's bug 518401

	(not currently active) Ted Mielczarek
	Comment 9 • 15 years ago

(In reply to comment #0)
> If this patch is acceptable, I have a matching NSPR version.

Did you ever find what happened to that patch? We still need it.

	Benjamin Smedberg Assignee
	Comment 10 • 15 years ago

This is for bug 524785, and the latest version I can find is http://hg.mozilla.org/users/bsmedberg_mozilla.com/mozilla-with-pymake/rev/e11e2873f293

	Justin Wood (:Callek)
	Comment 11 • 15 years ago

Requesting approval for changeset http://hg.mozilla.org/mozilla-central/rev/f655090e14c3 to land on 1.9.2

Low risk, helps Comm-Central.

	Daniel Veditz [:dveditz]
	Comment 12 • 15 years ago

Comment on attachment 435819 [details]
Dummy file for changeset f655090e14c3 approval

unorthodox, but approved for 1.9.2.4, a=dveditz

	Justin Wood (:Callek)
	Comment 13 • 15 years ago

Comment on attachment 435819 [details]
Dummy file for changeset f655090e14c3 approval

Pushed as: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/2b486c962f2f

(Bug is not 1.9.2 fixed, just this VERY minor followup is.)

	Marco Bonardo [:mak]
	Comment 14 • 14 years ago

(In reply to comment #13)
> (Bug is not 1.9.2 fixed, just this VERY minor followup is.)

status of the bug is splitted this way, should it be marked wontfix on 1.9.2 since it's not fixed and looks like there is no will to do that? having an approval but not having a status pollutes searches, the patch should have got its own bug.

	Justin Wood (:Callek)
	Comment 15 • 14 years ago

(In reply to comment #14)
> (In reply to comment #13)
> > (Bug is not 1.9.2 fixed, just this VERY minor followup is.)
> 
> status of the bug is splitted this way, should it be marked wontfix on 1.9.2
> since it's not fixed and looks like there is no will to do that? having an
> approval but not having a status pollutes searches, the patch should have got
> its own bug.

IMO its not worth its own bug either; I did not update status as I don't even think that its needed to show up on 1.9.2-based searches. But if you want a bug to track it I won't stop you from filing one with appropriate status/settings/keywords etc.

	Marco Bonardo [:mak]
	Comment 16 • 14 years ago

(In reply to comment #15)
> IMO its not worth its own bug either; I did not update status as I don't even
> think that its needed to show up on 1.9.2-based searches.

the point is not if it should appear in 1.9.2 based searches, the point is that it appears in 1.9.2 searches as "with approval, but not landed"

	Daniel Veditz [:dveditz]
	Comment 17 • 14 years ago

I guess "wontfix" is closer to the truth than .4-fixed.

	Daniel Veditz [:dveditz]
	Comment 18 • 14 years ago

Nope, that didn't clean up the query. Let's try fixed+whiteboard comment.

	Marco Bonardo [:mak]
	Comment 19 • 14 years ago

thanks, appreciate it.