Last Comment Bug 727406 - Port bug 645356 - Use pymake builtins
: Port bug 645356 - Use pymake builtins
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Thunderbird 17.0
Assigned To: Mark Banner (:standard8)
:
Mentors:
Depends on: 645356
Blocks: 668869 760353
  Show dependency treegraph
 
Reported: 2012-02-15 03:11 PST by Mark Banner (:standard8)
Modified: 2012-08-24 10:08 PDT (History)
4 users (show)
standard8: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
13+
fixed


Attachments
The fix (7.92 KB, patch)
2012-02-15 03:11 PST, Mark Banner (:standard8)
bugspam.Callek: review+
standard8: approval‑comm‑esr10+
Details | Diff | Review
Re-do rules.mk (4.66 KB, patch)
2012-08-16 14:14 PDT, Mark Banner (:standard8)
kairo: review+
Details | Diff | Review

Description Mark Banner (:standard8) 2012-02-15 03:11:06 PST
Created attachment 597357 [details] [diff] [review]
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) 2012-02-27 04:42:25 PST
Checked in: http://hg.mozilla.org/comm-central/rev/003487521298
Comment 2 Justin Wood (:Callek) 2012-02-28 10:43:51 PST
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) 2012-02-28 10:54:46 PST
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) 2012-02-28 12:07:39 PST
(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) 2012-02-29 02:48:28 PST
(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) 2012-06-01 00:45:42 PDT
(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) 2012-06-01 01:15:01 PDT
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) 2012-08-16 14:14:40 PDT
Created attachment 652563 [details] [diff] [review]
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 (not working on stability any more) 2012-08-16 16:50:25 PDT
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) 2012-08-17 02:08:13 PDT
Checked in with comment addressed: https://hg.mozilla.org/comm-central/rev/29ac9144a9fc

Note You need to log in before you can comment on or make changes to this bug.