[PEAK] Packaging peak apps

[Phillip J. Eby]

At 05:43 PM 9/16/04 -0500, Stephen Haberman wrote:

> > That's downright weird.  The line above appears *after* a block that does
> > 'path = PySys_GetObject("path")', if path is null!  There shouldn't be a
> > way to get there unless sys.path isn't a list, or is itself null.
>
>Oh. You're right. Well, sheesh, turns out path was never null in the first
>place. I did not initialize it to null and so it was just random junk, hence
>it got by the null check but then caught by the PyList_Check.
>
>I updated the sf bug with the patch that is now just about the loader
>pointer.

Good.


>Granted, I could have been just wrong back then or remembering wrong now on
>either a) or b).
>
>Huh. I agree, its weird that supposedly that used to work. Here was the test
>section of the original patch I submitted:
>
>+        # Test reload
>+        if not importer:
>+            old_test_co = test_co
>+            test_co = compile(test_src + "\ndef get_foo(): return 1",
>"<???>", "exec")
>+            TestImporter.modules = {
>+                "hooktestmodule": (False, test_co),
>+                "hooktestpackage": (True, test_co),
>+                "hooktestpackage.sub": (True, test_co),
>+                "hooktestpackage.sub.subber": (False, test_co),
>+            }
>+
>+            reload(hooktestmodule)
>+            self.assertEqual(hooktestmodule.get_name(), "hooktestmodule")
>+            self.assertEqual(hooktestmodule.get_foo(), 1)
>
>I'm not sure what the "if not importer" was for. It works just fine without
>that check now so I took it out.
>
>So. I dunno, maybe the "reload(hooktestmodule)" section was never even being
>executed? That would have been stupid, but possible.

That's why I asked about your test methodology.  When adding a new test, 
you *must* first check that the test *fails*, because that's the only way 
to have some assurance you're testing the right thing.  If you had run the 
test *without* your patch, it should have given you an error.  If the test 
passes without the change, you obviously don't need to change 
anything.  :)  (Or your test is wrong, as was probably the case here.)

By the way, this is what I meant earlier when I requested that you "Verify 
that this reduced patch set is correct; i.e. its test case should *fail* 
under unpatched 2.3 and 2.4, and succeed under patched 2.3 or 
2.4."  (Emphasis added.)


> > Ouch.  I was hoping that you were the one.  I guess now it's up to me.
>
>My apologies. Good luck. :-)

Well, it's good to know at least that the two weird bits are now taken care 
of, so basically what I'm going to do is add the test code, verify that it 
breaks, add your patch, verify that it works, and repeat that sequence for 
both the trunk and the 2.3 maintenance branch.

I'm also going to modify PEP 302, which needs to explain reload() 
requirements, and fix the example loaders in test_importhooks to follow 
good reload() practice.