545085 - Allow Mozmill to work with relative topsrcdirs

Status: RESOLVED FIXED

(Thunderbird :: Testing Infrastructure, defect)

Description

[Rain [:rain1] (inactive)]

Attachment: patch

I hit this while trying to build with pymake -- while everything in mozilla/ builds fine, pymake does currently require a relative topsrcdir on Windows.

The definition for core_abspath is taken from <http://mxr.mozilla.org/comm-central/source/mozilla/config/config.mk#86>.

Comment 1

[Rain [:rain1] (inactive)]

This patch is on top of the one in bug 533246.

Comment 2

[Mark Banner (:standard8)]

Comment on attachment 425949 [details] [diff] [review]
patch

Per irc discussion I think we want to not duplicate the definition of core_abspath which we've already defined in several places:

http://mxr.mozilla.org/comm-central/search?string=core_abspath

The duplicate definitions need moving into one place anyway and this goes part way to resolving bug 509147.

Comment 3

[Rain [:rain1] (inactive)]

I noticed in bug 481374 that you're carrying around both MOZ_APP_VERSION and MOZ_APP_VERSION_TXT, only to cat it wherever required. This of course causes problems with relative topsrcdirs, since MOZ_APP_VERSION_TXT is relative to the objdir root.

Will there be a problem if we simply use MOZ_APP_VERSION everywhere and throw away MOZ_APP_VERSION_TXT?

Comment 4

[Rain [:rain1] (inactive)]

Note that MOZ_APP_VERSION is simply defined as whatever's in MOZ_APP_VERSION_TXT at <http://mxr.mozilla.org/comm-central/source/mail/confvars.sh#67>.

Comment 5

[Rain [:rain1] (inactive)]

Attachment: patch, v2

OK, here's a patch that does what is proposed in comment 3. An alternative would be to define MOZ_APP_VERSION_TXT as $MOZ_BUILD_APP/config/version-foo.txt, and to then cat $(topsrcdir)/MOZ_APP_VERSION_TXT.

Comment 6

[Serge Gautherie (:sgautherie)]

(In reply to comment #2)
> this goes part way to resolving bug 509147.

Shouldn't bug 509147 block this one rather than depend on it?
(Could we at least separate the two patches, even in case they would need to be checked in simultaneously?)

Comment 7

[Mark Banner (:standard8)]

(In reply to comment #6)
> (In reply to comment #2)
> > this goes part way to resolving bug 509147.
> 
> Shouldn't bug 509147 block this one rather than depend on it?
> (Could we at least separate the two patches, even in case they would need to be
> checked in simultaneously?)

I thought it was reasonable to request it in this bug rather than adding another set of duplication and then removing it later. Whilst I agree this isn't the case for many bugs, it just seemed reasonable to do here.

Comment 8

[Serge Gautherie (:sgautherie)]

(In reply to comment #7)
> adding another set of duplication and then removing it later.

Which duplications would these be?
From what I see, the current patch here is a subset of the one in bug 509147 with additional/"separate" fixes.

Comment 9

[Rain [:rain1] (inactive)]

(In reply to comment #8)
> (In reply to comment #7)
> > adding another set of duplication and then removing it later.
> 
> Which duplications would these be?
> From what I see, the current patch here is a subset of the one in bug 509147
> with additional/"separate" fixes.

As far as I can tell, the current patch is what is required to get a build with a relative topsrcdir working, plus an additional fix or two (e.g. spawning fewer pwd processes) that make immediate sense.

Comment 10

[Serge Gautherie (:sgautherie)]

(In reply to comment #9)
> As far as I can tell, the current patch is what is required to get a build with

Do you mean the other "fixes" in bug 509147 patch are unwanted?

Comment 11

[Rain [:rain1] (inactive)]

(In reply to comment #10)
> (In reply to comment #9)
> > As far as I can tell, the current patch is what is required to get a build with
> 
> Do you mean the other "fixes" in bug 509147 patch are unwanted?

The only other fix in bug 509147 is to use Preprocessor.py instead of Preprocessor.pl, right? That is probably wanted, but it isn't related to getting a build working with a relative topsrcdir.

Comment 12

[Serge Gautherie (:sgautherie)]

(In reply to comment #11)

> The only other fix in bug 509147 is to use Preprocessor.py instead of

No, there are other diffs.

> Preprocessor.pl, right? That is probably wanted, but it isn't related to
> getting a build working with a relative topsrcdir.

Then, that's exactly why I suggest to split the patch here to be only the additional fixes needed on top of (all) what is (being) ported from m-c...
What good does the current mix do?

Comment 13

[Rain [:rain1] (inactive)]

Attachment: fix bitrot

Fixed minor bitrot due to bug 530055.

Comment 14

[Rain [:rain1] (inactive)]

http://hg.mozilla.org/comm-central/rev/a982d92e7565