727406 - Port bug 645356 - Use pymake builtins

Status: RESOLVED FIXED

(MailNews Core :: Build Config, defect)

Description

[Mark Banner (:standard8)]

Attachment: The fix

+++ This bug was initially created as a clone of Bug #645356 +++

pymake has its own built-in mkdir, rm, sleep and touch implementations. We should make use of them where we can, and remove any redundant/default parameters while we're at it.



Whilst I know we're generally not porting much from m-c, I want this bug as I'm currently trying (in bug 668869) to make Thunderbird's important files in mail/app/ as close as possible to Firefox's in terms of diffs etc. Hence I need the make variables introduced by this patch to make it happen.

Note: I've ported the <app>/installer parts, but I purposely didn't port the <app>/app parts as they are in flux at the moment (and I'm addressing mail/app in bug 668869).

Comment 1

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/003487521298

Comment 2

[Justin Wood (:Callek)]

Comment on attachment 597357 [details] [diff] [review]
The fix

Review of attachment 597357 [details] [diff] [review]:
-----------------------------------------------------------------

::: config/rules.mk
@@ -2097,4 @@
>  	$(CHECK_FROZEN_VARIABLES)
>  
>  default all::
> -	if test -d $(DIST)/bin ; then touch $(DIST)/bin/.purgecaches ; fi

ugh missed that this change (and possibly some other ones that are called from shell) break:

c:\t1\hg\comm-central\config\rules.mk:2082:0$ if test -d ../../../mozilla/dist/bin ; then %pymake.builtins touch ../../../mozilla/dist/bin/.purgecaches ; fi
/usr/bin/sh: line 0: fg: no job control
c:\t1\hg\comm-central\config\rules.mk:2082:0: command 'if test -d ../../../mozilla/dist/bin ; then %pymake.builtins touch ../../../mozilla/dist/bin/.purgecaches ; fi' failed, return code 1
c:\t1\hg\objdir-sm\suite\locales\Makefile:180:0: command 'c:/DEV/mozilla-build/python/python.exe c:/t1/hg/comm-central/mozilla/build/pymake/pymake/../make.py -C ../../editor/ui/locales AB_CD=en-US XPI_NAME=locale-en-US BOTH_MANIFESTS=1' failed, return code 2
c:\t1\hg\objdir-sm\suite\installer\Makefile:165:0: command 'c:/DEV/mozilla-build/python/python.exe c:/t1/hg/comm-central/mozilla/build/pymake/pymake/../make.py -C ../../suite/locales langpack' failed, return code 2
c:\t1\hg\comm-central\config\rules.mk:676:0: command 'c:/DEV/mozilla-build/python/python.exe c:/t1/hg/comm-central/mozilla/build/pymake/pymake/../make.py libs' failed, return code 2
c:\t1\hg\comm-central\suite\build.mk:79:0: command 'c:/DEV/mozilla-build/python/python.exe c:/t1/hg/comm-central/mozilla/build/pymake/pymake/../make.py -C suite/installer' failed, return code 2

Please backout and submit a new patch

Comment 3

[Mark Banner (:standard8)]

This can't be backed out without backing out the dependent patch which I don't want to do.

I don't have time now, but I can back out that one line change from the patch tomorrow morning my time.

Comment 4

[Justin Wood (:Callek)]

(In reply to Mark Banner (:standard8) from comment #3)
> This can't be backed out without backing out the dependent patch which I
> don't want to do.
> 
> I don't have time now, but I can back out that one line change from the
> patch tomorrow morning my time.

I explicitly asked for a backout and a new patch, since you NEED to check all the cases of line continuations for breakage here, if it tries to run more than one command in a single invocation it will invoke the shell and BREAK in pymake.

That is the whole problem here, though I do find it annoying that "I don't want to" is good enough for leaving the tree broken for normal developers for a good amount of time.

Comment 5

[Mark Banner (:standard8)]

(In reply to Justin Wood (:Callek) from comment #4)
> (In reply to Mark Banner (:standard8) from comment #3)
> > This can't be backed out without backing out the dependent patch which I
> > don't want to do.
> > 
> > I don't have time now, but I can back out that one line change from the
> > patch tomorrow morning my time.
> 
> I explicitly asked for a backout and a new patch, since you NEED to check
> all the cases of line continuations for breakage here, if it tries to run
> more than one command in a single invocation it will invoke the shell and
> BREAK in pymake.

Like I said, I didn't have time to look at it last night, and didn't want to back out both bugs (I never said I wouldn't).

Having now looked at it afresh, I've backed out just the rules.mk change. This would seem to satisfy the not breaking pymake requirement and being able to leave the additional bug in without too many extra changes.

Comment 6

[Mark Banner (:standard8)]

(In reply to Mark Banner (:standard8) from comment #5)
> (In reply to Justin Wood (:Callek) from comment #4)
> Having now looked at it afresh, I've backed out just the rules.mk change.
> This would seem to satisfy the not breaking pymake requirement and being
> able to leave the additional bug in without too many extra changes.

The backout was http://hg.mozilla.org/comm-central/rev/271f2a50cfbb

Comment 7

[Mark Banner (:standard8)]

Comment on attachment 597357 [details] [diff] [review]
The fix

[Triage Comment]
We need this and the backout on ESR to fix build bustage with the new automation, so a=me for landing.

Comment 8

[Mark Banner (:standard8)]

Attachment: Re-do rules.mk

This ports just the rules.mk changes from bug 645356. This time, I just replaced the same instances in c-c's file as were replaced in the m-c file, rather than replacing all the instances.

Try server builds will appear here:

https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=7dcfa0f4f25f

Comment 9

[Robert Kaiser]

Comment on attachment 652563 [details] [diff] [review]
Re-do rules.mk

Review of attachment 652563 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the one nit/typo fixed.

::: config/rules.mk
@@ +1019,4 @@
>  endif
>  
>  $(filter %.$(LIB_SUFFIX),$(LIBRARY)): $(OBJS) $(LOBJS) $(EXTRA_DEPS) $(GLOBAL_DEPS)
> +	$(EM) $(LIBRARY)

Umm, this one looks wrong. Should be $(RM) as well.

Comment 10

[Mark Banner (:standard8)]

Checked in with comment addressed: https://hg.mozilla.org/comm-central/rev/29ac9144a9fc