Closed Bug 545085 Opened 10 years ago Closed 10 years ago

Allow Mozmill to work with relative topsrcdirs

Categories

(Thunderbird :: Testing Infrastructure, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.1b2

People

(Reporter: rain1, Assigned: rain1)

References

Details

Attachments

(1 file, 2 obsolete files)

Attached patch patch (obsolete) — Splinter Review
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>.
Attachment #425949 - Flags: review?(bugzilla)
This patch is on top of the one in bug 533246.
Depends on: 533246
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.
Attachment #425949 - Flags: review?(bugzilla) → review-
Blocks: 509147
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?
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>.
Attached patch patch, v2 (obsolete) — Splinter Review
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.
Attachment #425949 - Attachment is obsolete: true
Attachment #426948 - Flags: review?
Attachment #426948 - Flags: review? → review?(bugzilla)
(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?)
(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.
(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.
(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.
(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?
(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.
(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?
Attached patch fix bitrotSplinter Review
Fixed minor bitrot due to bug 530055.
Attachment #426948 - Attachment is obsolete: true
Attachment #429387 - Flags: review?(bugzilla)
Attachment #426948 - Flags: review?(bugzilla)
Attachment #429387 - Flags: review?(bugzilla) → review+
http://hg.mozilla.org/comm-central/rev/a982d92e7565
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Flags: in-testsuite-
Target Milestone: --- → Thunderbird 3.1b2
You need to log in before you can comment on or make changes to this bug.