756443 - makefiles: add mkdir_dep helper functions

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Joey Armstrong [:joey]]

1) Add a function mkdir_stem that can strip '/.mkdir.done from a given list of paths.  This function can be called when a directory is needed as both a pre-requisite and element of the target command path.

The ability to strip the timestamp file allows use of $<, $*, $^ w/o having to hardcode directory paths within a command.

2) minor edit, not critical - scrub multiple slashes from a given path.  The simple $(subst //,/,$(val)) pattern substitution will not handle a few cases that a series of string functions can produce.

Comment 1

[Joey Armstrong [:joey]]

Attachment: Add mkdir_stem() to complement mkdir_deps. Strip extraneous slashes from given paths".

Comment 2

[Joey Armstrong [:joey]]

Comment on attachment 625135 [details] [diff] [review]
Add mkdir_stem() to complement mkdir_deps.  Strip extraneous slashes from given paths".

Added mkdir_stem() as a complement to mkdir_deps() allowing a timestmap to be used for pre-requisites which can then be morphed back into a directory for use on the command line.

Expanded the squash-directory-slashes logic to handle more cases.
Logic is able to preserve embedded whitespace within paths.
Also the logic could be used for windows directory slashes if needed by declaring a variable containing the slash type and replacing literal '/' within functions.

Comment 3

[Joey Armstrong [:joey]]

ping on code review

Comment 4

[Joey Armstrong [:joey]]

Try job: https://tbpl.mozilla.org/?tree=Try&rev=d3a260cb739f 

mac failures due to ssh/key problems during checksum/upload

Comment 5

[Joey Armstrong [:joey]]

ping on code review from the 18th

Comment 6

[Mike Hommey [:glandium]]

Comment on attachment 625135 [details] [diff] [review]
Add mkdir_stem() to complement mkdir_deps.  Strip extraneous slashes from given paths".

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

::: config/makefiles/autotargets.mk
@@ +22,5 @@
> +space = $(NULL) $(NULL)
> +
> +
> +## todo: create fileutils.mk - combine directory functions with
> +## toolkit/mozapps/installer/packager.mk: QUOTED_WILDCARD, ESCAPE_SPACE(), etc

Not sure this todo note is very useful.

@@ +45,5 @@
> +	$(call _slashSqueeze,\
> +    $(subst $(space),<--[**]-->,$(1))\
> +  )))
> +
> +# Revert pre-reqs [$<, $*, $^] to dirs for target command line use

Not sure what this comment means.

@@ +46,5 @@
> +    $(subst $(space),<--[**]-->,$(1))\
> +  )))
> +
> +# Revert pre-reqs [$<, $*, $^] to dirs for target command line use
> +mkdir_stem =$(foreach val,$(getargv),$(subst /.mkdir.done,$(NULL),$(val)))

patsubst %/.mkdir.done,%

Comment 7

[Joey Armstrong [:joey]]

(In reply to Mike Hommey [:glandium] from comment #6)
> Comment on attachment 625135 [details] [diff] [review]
> Add mkdir_stem() to complement mkdir_deps.  Strip extraneous slashes from
> given paths".
> 
> Review of attachment 625135 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: config/makefiles/autotargets.mk
> @@ +22,5 @@
> > +space = $(NULL) $(NULL)
> > +
> > +
> > +## todo: create fileutils.mk - combine directory functions with
> > +## toolkit/mozapps/installer/packager.mk: QUOTED_WILDCARD, ESCAPE_SPACE(), etc
> 
> Not sure this todo note is very useful.

File based logic from the makefiles mentioned along with related logic in autotargets can be refactored into a standalone 'fileutils.mk' library for reuse and testing.  Todo is just a reminder about this, if a bug is preferable I can open another one.

> 
> @@ +45,5 @@
> > +	$(call _slashSqueeze,\
> > +    $(subst $(space),<--[**]-->,$(1))\
> > +  )))
> > +
> > +# Revert pre-reqs [$<, $*, $^] to dirs for target command line use
> 
> Not sure what this comment means.

mkdir_deps generated pre-requisites will be a timestamp (which will be used to populate make's autovariables).  A string transform is needed to morph the timestamp back into a directory path { ~mkdir_stem($dep) } in order to use it as part of a command line. 


> @@ +46,5 @@
> > +    $(subst $(space),<--[**]-->,$(1))\
> > +  )))
> > +
> > +# Revert pre-reqs [$<, $*, $^] to dirs for target command line use
> > +mkdir_stem =$(foreach val,$(getargv),$(subst /.mkdir.done,$(NULL),$(val)))
> 
> patsubst %/.mkdir.done,%

If patterns are unique {yes I know that is subjective} -- $(patsubst ) will be safer and $(subst ) will be a little more efficient.

Comment 8

[Joey Armstrong [:joey]]

Attachment: Add mkdir_stem() to complement mkdir_deps. Strip extraneous slashes from given paths".

Comment 9

[Joey Armstrong [:joey]]

Try job: https://tbpl.mozilla.org/?tree=Try&rev=3f76bd22a57f
r=glandium carried forward

Removed todo comment and opened a new makefile bug.
Changed comment to note functionality rather than application for make autovars.

Comment 10

[(not currently active) Ted Mielczarek]

Comment on attachment 628450 [details] [diff] [review]
Add mkdir_stem() to complement mkdir_deps.  Strip extraneous slashes from given paths".

If you got r+ from glandium then you don't need to request review from me, you're clear to land.

Comment 11

[Joey Armstrong [:joey]]

Inbound push: https://hg.mozilla.org/integration/mozilla-inbound/rev/b7fd4683de77

Comment 12

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/b7fd4683de77