637034 - Add a native mkdir to pymake.builtins

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mitchell Field [:Mitch]]

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

Comment 1

[Mitchell Field [:Mitch]]

Attachment: Patch

This adds mkdir to pymake. Includes new mkdir tests plus makes the rm tests more thorough with subdirs.

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

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.

Comment 3

[(not currently active) 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.

Comment 4

[Mitchell Field [:Mitch]]

http://hg.mozilla.org/projects/build-system/rev/c39ecf6d18ba

Comment 5

[(not currently active) Ted Mielczarek]

Did you land this in bsmedberg's user repo as well?

Comment 6

[Mitchell Field [:Mitch]]

http://hg.mozilla.org/users/bsmedberg_mozilla.com/pymake/rev/562adc586a24
http://hg.mozilla.org/users/bsmedberg_mozilla.com/pymake/rev/944fd46f8f78

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

http://hg.mozilla.org/mozilla-central/rev/c39ecf6d18ba