silnrsi / smith

font development, testing and release
Other
14 stars 5 forks source link

@defer decorator breaks inheritance and the breakage is hidden #50

Closed nrsiward closed 3 years ago

nrsiward commented 6 years ago

Reminder: defer() seems to be designed to accept a class and return a function. So, applying @defer to a class converts the class to a function. Other classes then inherit from what was a class but is now a function, so the class declaration fails (at least under Python 2.7). defer() can be used explicitly instead of as a decorator after subclassing is done.

Running smith.py instead of smith exposed this issue.

This issue was not showing in the smith built by mywaflite (and waf-light) because the wscript that's invoked handles decorators in a special way. See process_decorators(). The decorator is removed and the function it referenced is applied at the end of the file to the correct object but without reassigning the identifier for that object. Look at the end of wsiwaf.py and fonts.py in the temp directory where the unpacked smithlib is stored (/usr/lib/waf-*/waflib/extras). I don't know why this is done except maybe to support older versions of Python.

For example

@defer
class cmd(object) :

becomes defer(cmd)

This manipulation of decorators works for waf decorators (which just return the input object after registering it, IIRC), but it doesn't work for what defer() is doing. In that case, the @defer decorator is removed, and when defer() is called at the end of the file, the function object it returns is just garbage collected. Moving defer() after subclassing (and removing the @defer decorators) avoids this problem.

This leads to the question of whether the deferred_class approach works since it was never used in the smith script, AFAICT. After I worked around the above issue by moving the defer() calls after subclassing, smith (and smith.py) will run but fails while reading the wscript when building Charis

The fixdefer branch contains files where the @defer decorator has been replaced by explicit calls to defer() after subclassing, but it fails when reading the wscript for Charis as described above. I put this in a branch because if smith is built using mywaflite it will be broken.

mhosken commented 6 years ago

fixed in branch. There is much more going on than the need to defer the deferal :) Please test the branch against existing projects for a bit and once we are confident there are no regressions, merge with master.

The reason why this didn't show up before is that the wscript used by mywaflite to create smith was stripping decorators. Or at least doing something funky with them that meant that the @defer decorators were being effectively stripped. It doesn't do that any more.

bobh0303 commented 3 years ago

Was the branch merged? Was the problem fixed?

nrsiward commented 3 years ago

I thought it was merged since this was done a long time ago, but after looking at the branch, I see it was not merged. However, it's irrelevant now since we are not distributing smith the old way (using the same technique that WAF uses to create a single file distribution) but are distributing it in a standard Python way (using setup.py and smith.py). The problem described in this issue occurred when using that old technique.