mach -vs- 'None': no context for errors

NEW
Unassigned

Status

()

Core
Build Config
5 years ago
5 years ago

People

(Reporter: joey, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

5 years ago
Problem:  Within a moz.build file, derive a value from a non-existent dictionary value 'CONFIG[]' and add it to a list.  A value of 'None' can be generated which coupled with a large list of modified files and minimal error context can make the problem fun to track down.

CMMSRCS += [
    "%s.mm" % (CONFIG['LIBRARY_NAME']),
    'plugin_child_quirks.mm',
]

./mach build

 0:30.25 a specialpowers/chrome/specialpowers/content/specialpowersAPI.js
 0:30.25 a specialpowers/chrome/specialpowers/content/SpecialPowersObserverAPI.js
 0:35.20 gmake[7]: *** No rule to make target `None.mm', needed by `None.o'.  Stop.
 0:35.20 gmake[6]: *** [libs] Error 2
 0:35.20 gmake[5]: *** [plugins/ipc_libs] Error 2
 0:35.20 gmake[5]: *** Waiting for unfinished jobs....
 0:35.33 a workerbootstrap
 0:35.33 a workerbootstrap/bootstrap.js

'None' and 'ipc_libs' are nonexistent.  There are several 'plugins' directories to choose from.

At a minimum reporting cwd with None errors would help narrow down the search space.
I don't think we should be trying to handle this at the Makefile level. We should probably be raising an error when you try to access a nonexistent key in CONFIG.
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #1)
> I don't think we should be trying to handle this at the Makefile level. We
> should probably be raising an error when you try to access a nonexistent key
> in CONFIG.

I wonder how many things rely on the current behavior of returning None, grep shows many things relying on a false value being returned but maybe the variable is defined in all those cases to  something that converts to false.

btw why are you trying to  debug based on the output of make -s?  This should be pretty easy to figure out when running make -j1 without it and you should have a pretty good direction to look with -jn
(In reply to Trevor Saunders (:tbsaunde) from comment #2)
> I wonder how many things rely on the current behavior of returning None,
> grep shows many things relying on a false value being returned but maybe the
> variable is defined in all those cases to  something that converts to false.

I think there's a difference between "something that was AC_SUBSTed to an empty value" and "something that was never AC_SUBSTed".
(Reporter)

Comment 4

5 years ago
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #1)
> I don't think we should be trying to handle this at the Makefile level. We
> should probably be raising an error when you try to access a nonexistent key
> in CONFIG.

If key checking is limited to CONFIG and not dictionaries in general that would probably make sense to add longer term.  It will add overhead perform frequent key searches but if errors like this were prevented that would be a good thing.

But it will probably be a while before this behavior appears in mozbuild and the conversions are tripping over this problem now so I think a makefile edited would make sense.

Also I think there is a larger issue that needs to be addressed wrt reporting error context in general.  It should not require having to run find+grep most of the time to try and figure out where problems happened.  That activity can become ugly for problems like this when there are a large number of files and very little to search on.

Comment 5

5 years ago
if CONFIG['NONEXISTENT_KEY']:

is purposefully supported. Not all keys are defined for all configurations. If we were to raise KeyError on unknown keys, the code would thus become:

if 'NONEXISTENT_KEY' in CONFIG:

OR

if 'NONEXISTENT_KEY' in CONFIG and CONFIG['NONEXISTENT_KEY']:
You need to log in before you can comment on or make changes to this bug.