485409 - Substitute windows-style paths in configure when building with pymake

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Benjamin Smedberg]

Attachment: Hack configure to accept windows-style absolute paths, rev. 1

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.

Comment 1

[Benjamin Smedberg]

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.

Comment 2

[Benjamin Smedberg]

Attachment: Hack configure to accept windows-style absolute paths, rev. 1.1

Comment 3

[(not currently active) Ted Mielczarek]

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.

Comment 4

[(not currently active) Ted Mielczarek]

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

Comment 5

[(not currently active) Ted Mielczarek]

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

Comment 6

[Rob Arnold [:robarnold]]

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?

Comment 7

[Rob Arnold [:robarnold]]

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

Comment 8

[Benjamin Smedberg]

That's bug 518401

Comment 9

[(not currently active) Ted Mielczarek]

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

Comment 10

[Benjamin Smedberg]

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

Comment 11

[Justin Wood (:Callek)]

Attachment: Dummy file for changeset f655090e14c3 approval

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

Low risk, helps Comm-Central.

Comment 12

[Daniel Veditz [:dveditz]]

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

unorthodox, but approved for 1.9.2.4, a=dveditz

Comment 13

[Justin Wood (:Callek)]

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

Comment 14

[Marco Bonardo [:mak]]

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

Comment 15

[Justin Wood (:Callek)]

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

Comment 16

[Marco Bonardo [:mak]]

(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"

Comment 17

[Daniel Veditz [:dveditz]]

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

Comment 18

[Daniel Veditz [:dveditz]]

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

Comment 19

[Marco Bonardo [:mak]]

thanks, appreciate it.