makeutil library func: checkIfEmpty

RESOLVED FIXED

Status

()

Core
Build Config
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: joey, Assigned: joey)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

5 years ago
Add a checkIfEmpty makefile library function that will accept a list of variable names and report "Variable ${var} does not contain a value" if undef, empty or contains spaces.
Default behavior should invoke $(error) and force failure.
An optional parameter can be passed to alter this behavior by calling $(warning).

This function will mainly be used for testing values after asynchronous shell commands have been run and failed to propagate failure { in the absence of SHELL += -e }.

A unit test will also be needed, both positive and negative, to validate behavior of the function.
(Assignee)

Updated

5 years ago
Assignee: nobody → joey
(Assignee)

Comment 1

5 years ago
Let's make this easy by defining three macros, two for named access:
  warnIfEmpty
  errorIfEmpty

and a 3rd that will accept values and a callback function $(error|warning) as $(lastword)
checkIfEmpty,@vars [warn|empty])
(Assignee)

Updated

5 years ago
Depends on: 734121
(Assignee)

Comment 2

5 years ago
Created attachment 605769 [details] [diff] [review]
make assert macros {library flavor} for checking shell defined make variables

makeutil library functions for warning about or failing on make macros w/o a value.

A unit test has been setup to verify functionality but is currently wrappered with a 'MANUAL_TEST' conditional until syntax can be found able to test errorIfEmpty w/o aborting the make run.
Attachment #605769 - Flags: review?(khuey)
(Assignee)

Comment 3

5 years ago
Created attachment 605786 [details] [diff] [review]
toolkit/xre/Makefile.in - reduce shell command use

Same patch as last time but copied config/makefiles/makeutils.mk into js/src/config/makefiles/makeutils.mk to keep the check-sync-dirs.py test at bay.
Attachment #605786 - Flags: review?(khuey)
(Assignee)

Updated

5 years ago
Attachment #605769 - Flags: review?(khuey)
(Assignee)

Comment 4

5 years ago
code review ping
Comment on attachment 605786 [details] [diff] [review]
toolkit/xre/Makefile.in - reduce shell command use

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

r=me provided manual.sh is still not getting checked in.
Attachment #605786 - Flags: review?(khuey) → review+
(Assignee)

Comment 6

5 years ago
Created attachment 608329 [details] [diff] [review]
add utility function checkIfEmpty

r=kyle carried forward.  Only change was to remove manual.sh from the patch.  735638 and 734121 should have been the only patches that included the script.
Attachment #605769 - Attachment is obsolete: true
Attachment #605786 - Attachment is obsolete: true
(Assignee)

Comment 7

5 years ago
these makefile edits should simmer in b-s for a while.

Comment 8

5 years ago
Comment on attachment 608329 [details] [diff] [review]
add utility function checkIfEmpty

https://hg.mozilla.org/projects/build-system/rev/329bd787a846
(Assignee)

Comment 9

5 years ago
Patch landed on mozilla-central yesterday by Coop & Philor:
https://hg.mozilla.org/mozilla-central/rev/c598b7b202e7
(Assignee)

Updated

5 years ago
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Updated

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