Closed Bug 637034 Opened 9 years ago Closed 9 years ago
Add a native mkdir to pymake
pymake has native, built-in implementations of "rm", "sleep" and "touch". We might as well have a native mkdir. http://hg.mozilla.org/mozilla-central/file/151d55505c10/build/pymake/pymake/builtins.py
This adds mkdir to pymake. Includes new mkdir tests plus makes the rm tests more thorough with subdirs.
Attachment #515364 - Flags: review?(khuey)
Comment on attachment 515364 [details] [diff] [review] Patch I'm going to pass this to ted since he's the python guy. I also think we decided to use PEP 8 spacing (4 spaces) in the build system.
Attachment #515364 - Flags: review?(khuey) → review?(ted.mielczarek)
Comment on attachment 515364 [details] [diff] [review] Patch We did say PEP-8, but this is existing code that has 2-space indent. I don't know that it's worth having mismatched indents or reformatting all our code. >diff --git a/build/pymake/pymake/builtins.py b/build/pymake/pymake/builtins.py >--- a/build/pymake/pymake/builtins.py >+++ b/build/pymake/pymake/builtins.py >+ parents = False >+ for o, a in opts: >+ if o in ('-p', '--parents'): >+ parents = True >+ for f in args: >+ try: >+ os.makedirs(f) This doesn't strictly match the behavior of rm. If you did $(MKDIR) foo/bar, without -p, this code would still create foo/bar instead of failing. You should just use os.mkdir instead of makedirs if parents is False. You have a test for this in mkdir-fail.mk, but not for your builtin. :) >+ except OSError, e: >+ if (e.errno == errno.EEXIST) and (parents == True): Some idiomatic Python nits: you don't need the parentheses, also, parents is a bool, so just saying "and parents:" is fine. r=me with those fixes.
Attachment #515364 - Flags: review?(ted.mielczarek) → review+
Did you land this in bsmedberg's user repo as well?
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.