Misleading error message from global functions

[bdbaddog]

This issue was originally created at: 2008-09-05 01:37:12. This issue was reported by: russel.

russel said at 2008-09-05 01:37:12

If the statement:

Clean(".", "*~")

is used in a SConscript then everything works as expected and desired. If, however, one is careless and writes:

Clean("*~")

then the error message is:

TypeError: Clean() takes exactly 3 arguments (2 given):
  File "/home/users/russel/Docs/Talks/PyConUK_2008/SConsTheBuildFramework/SourceCode/HelloWorld_Trivial/SConstruct", line 3:
    Clean("*~")
  File "/home/Checkouts/Bazaar/SCons_Trunk/bootstrap/src/engine/SCons/Script/SConscript.py", line 596:
    return apply(method, args, kw)

Now clearly the Clean function is entirely happy with two arguments, so obviously it doesn't take exactly three. Moreover, I have only given it 1 argument, so why is it saying I gave it two?

Obviously, this is a "Python methods' implicit first argument is self" type issue, but that doesn't help with the verisimilitude of the error message. My worry here is that as SCons takes over from Make and Autotools (not to mention Ant, Maven etc.) people who are not Python programmers are going to be using it, so they will not have the background knowledge to understand why it is the Python error system appears to be lying to them.

gregnoel said at 2008-09-05 13:43:40

This issue is really about all the "global" functions that are also methods of an Environment, but don't actually need an environment to perform their actions (the only thing the bound methods do is subst() on the parameters). They are written as methods of the Environment and then some magic is exerted to make them global functions by forwarding the request to DefaultEnvironment. This has the side effect that if such global functions are used, the DefaultEnvironment must be instantiated, which does a lot of unnecessary initialization.

I propose that this is backward. They should be defined as global functions (although probably within Environment.py) and a wrapper used to attach them to the Environment class. The wrapper would do the subst() on the arguments and pass them on to the actual function.

Low priority, but not something we should lose track of.

gregnoel said at 2008-09-09 14:01:33

Bug party triage.

stevenknight said at 2008-12-06 11:30:38

No one's looking at this because its target milestone is 1.x, but it's still assigned to issues@scons. Change the target milestone to -unspecified- so we can prioritize and assign it correctly at the next Bug Party.

stevenknight said at 2008-12-06 11:31:22

Actually changing the target milestone this time.

gregnoel said at 2008-12-24 06:56:20

Bug party triage. To get it right, it needs some major rearchitecture (not to mention some features not in Python 1.5.2), so don't lose the problem, but push it out to where we'll review it again when we're scheduling the rearchitecture work.

gregnoel said at 2008-12-24 08:03:43

Actually set milestone and priority.

[mwichmann]

I changed the summary slightly. This is proposing a moderately serious bit of rework - swap how the dual gobal/environment methods are defined/attached - which seems to have little real benefit except for producing slightly better error messages. Is it actually worth keeping around?

[bdbaddog]

I don't think worth huge change. But if there's a way to give a clearer error message, that's probably worth persuing?