Closed Bug 645356 Opened 9 years ago Closed 9 years ago

Use pymake builtins

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla5

People

(Reporter: Mitch, Assigned: Mitch)

References

Details

(Whiteboard: fixed-in-bs)

Attachments

(1 file)

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.
Summary: Use of pymake builtins → Use pymake builtins
Attached patch PatchSplinter Review
Attachment #522117 - Flags: review?(khuey)
Blocks: 585010
No longer blocks: 485412
Comment on attachment 522117 [details] [diff] [review]
Patch

> export:: $(TARGETS) $(HEADERS)
> 	$(INSTALL) $(IFLAGS1) $(HEADERS) $(DIST)/include
>-	-rm -f $(FINAL_LINK_COMPS) $(FINAL_LINK_LIBS) $(FINAL_LINK_COMP_NAMES)
>-	-rm -f $(DIST)/bin/chrome/chromelist.txt
>+	-$(RM) $(FINAL_LINK_COMPS) $(FINAL_LINK_LIBS) $(FINAL_LINK_COMP_NAMES)
>+	-$(RM) $(DIST)/bin/chrome/chromelist.txt

I think you can drop the chromelist.txt thing entirely.  That's been gone for a while.

Also, do builtins work right with -?  Not sure if that's tested or not.

> ifdef MKDEPEND_DIR
> clean clobber realclean clobber_all::
>-	cd $(MKDEPEND_DIR); $(MAKE) $@
>+	cd $(MKDEPEND_DIR)
>+	$(MAKE) $@

Er, this is completely wrong, no?


>@@ -1580,18 +1582,18 @@ export:: FORCE
> endif
> 
> $(IDL_DIR)::
> 	$(NSINSTALL) -D $@
> 
> # generate .h files from into $(XPIDL_GEN_DIR), then export to $(DIST)/include;
> # warn against overriding existing .h file. 
> $(XPIDL_GEN_DIR)/.done:
>-	@if test ! -d $(XPIDL_GEN_DIR); then echo Creating $(XPIDL_GEN_DIR)/.done; rm -rf $(XPIDL_GEN_DIR); mkdir $(XPIDL_GEN_DIR); fi
>-	@touch $@
>+	$(MKDIR) -p $(XPIDL_GEN_DIR)
>+	@$(TOUCH) $@

> $(CURDIR)/$(MDDEPDIR):
>-	@if test ! -d $@; then echo Creating $@; rm -rf $@; mkdir $@; else true; fi
>+	$(MKDIR) -p $@

This shell killing warms my heart.

r=me with the stuff mentioned fixed.  I'm assuming you've tested this well.
Attachment #522117 - Flags: review?(khuey) → review+
http://hg.mozilla.org/projects/build-system/rev/f066b09198b3
OS: Windows 7 → All
Hardware: x86 → All
Whiteboard: fixed-in-bs
(In reply to comment #2)
>(From update of attachment 522117 [details] [diff] [review])
>> ifdef MKDEPEND_DIR
>> clean clobber realclean clobber_all::
>>-	cd $(MKDEPEND_DIR); $(MAKE) $@
>>+	cd $(MKDEPEND_DIR)
>>+	$(MAKE) $@
>Er, this is completely wrong, no?
How about $(MAKE) -C $(MKDEPEND_DIR) $@
http://hg.mozilla.org/mozilla-central/rev/f066b09198b3
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.2
Blocks: 727406
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.