The default bug view has changed. See this FAQ.

makefiles: add mkdir_dep helper functions

RESOLVED FIXED in mozilla15

Status

()

Core
Build Config
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: joey, Assigned: joey)

Tracking

unspecified
mozilla15
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
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.
(Assignee)

Updated

5 years ago
Assignee: nobody → joey
(Assignee)

Updated

5 years ago
Blocks: 734139
(Assignee)

Comment 1

5 years ago
Created attachment 625135 [details] [diff] [review]
Add mkdir_stem() to complement mkdir_deps.  Strip extraneous slashes from given paths".
(Assignee)

Comment 2

5 years ago
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.
Attachment #625135 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 3

5 years ago
ping on code review
(Assignee)

Comment 4

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

mac failures due to ssh/key problems during checksum/upload
(Assignee)

Comment 5

5 years ago
ping on code review from the 18th
(Assignee)

Updated

5 years ago
Attachment #625135 - Flags: review?(ted.mielczarek) → review?(mh+mozilla)
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,%
Attachment #625135 - Flags: review?(mh+mozilla) → review+
(Assignee)

Comment 7

5 years ago
(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.
(Assignee)

Comment 8

5 years ago
Created attachment 628450 [details] [diff] [review]
Add mkdir_stem() to complement mkdir_deps.  Strip extraneous slashes from given paths".
(Assignee)

Updated

5 years ago
Attachment #625135 - Attachment is obsolete: true
(Assignee)

Comment 9

5 years ago
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.
(Assignee)

Updated

5 years ago
Attachment #628450 - Flags: review?(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.
Attachment #628450 - Flags: review?(ted.mielczarek)
(Assignee)

Comment 11

5 years ago
Inbound push: https://hg.mozilla.org/integration/mozilla-inbound/rev/b7fd4683de77
https://hg.mozilla.org/mozilla-central/rev/b7fd4683de77
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
(Assignee)

Updated

5 years ago
Blocks: 757855
You need to log in before you can comment on or make changes to this bug.