Patches item #1720897, was opened at 2007-05-17 15:13 Message generated for change (Comment added) made by errebepe You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1720897&group_id=5470
Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: Distutils and setup.py Group: Python 2.6 Status: Open Resolution: None Priority: 5 Private: No Submitted By: Raghuram Devarakonda (draghuram) Assigned to: Nobody/Anonymous (nobody) Summary: fix 1668596: copy datafiles properly when package_dir is ' ' Initial Comment: This patch fixes http://www.python.org/sf/1668596. If package_dir is '', there is no need to remove package_dir part from data file path names. I am also attaching 1668596_test.tar.gz. This has the directory I used to test the patch (both on Linux and Windows XP). I will need some more time to come up with a real test case that can be submitted as patch. I am hoping that if there is some one out there who is familiar with distutils test setup, they may be able to come up with a test case quicker than me. ---------------------------------------------------------------------- Comment By: Rodrigo Bernardo Pimentel (errebepe) Date: 2007-05-30 14:58 Message: Logged In: YES user_id=374783 Originator: NO +1 on the fix. I do have a few minor suggestions, however. In the fix itself, the parens in "(src_dir)" are still there. If the fix will only be applied in >= 2.5, you might also use the "plen = len(src_dir)+1 if src_dir else 0" form. As for the test, since you're testing a very specific error scenario (Distribution.run_commands raising DistutilsFileError), I'd suggest catching the exception and using "self.fail", so that testing the unfixed code fails instead of issues an error. Since you expect the error to occur inside run_commands, I'd put the try...except block around the "dist.run_commands()" only, as in: # Auxiliary variable, since you need cleanup and self.fail exits before "finally": error = False try: ....dist.run_commands() except DistutilsFileError: ....error = True finally: ....os.chdir(cwd) if error: ....self.fail("run_commands fails with no package_dir") Gustavo Niemeyer also mentioned you should add a dosctring to the test explaining what exactly it's testing. To sum it up, I recommend commit of this patch regardless of my comments above, but I hope they're taken into consideration :) ---------------------------------------------------------------------- Comment By: Raghuram Devarakonda (draghuram) Date: 2007-05-30 14:19 Message: Logged In: YES user_id=984087 Originator: YES Thanks for reviewing. As for parens, they don't exist in the current patch. Right? I removed them quite some time back. ---------------------------------------------------------------------- Comment By: Phillip J. Eby (pje) Date: 2007-05-30 13:31 Message: Logged In: YES user_id=56214 Originator: NO +1 on this approach; recommend commit of this patch (note, however, that the parens in "if (src_dir):" are redundant). ---------------------------------------------------------------------- Comment By: Raghuram Devarakonda (draghuram) Date: 2007-05-21 15:24 Message: Logged In: YES user_id=984087 Originator: YES I managed to create a test case (is part of the patch now). I ran it on Linux and Windows XP and in both cases, it properly tests the problem. ---------------------------------------------------------------------- Comment By: Raghuram Devarakonda (draghuram) Date: 2007-05-21 15:22 Message: Logged In: YES user_id=984087 Originator: YES File Added: buildpy_patch.diff ---------------------------------------------------------------------- Comment By: Raghuram Devarakonda (draghuram) Date: 2007-05-17 15:25 Message: Logged In: YES user_id=984087 Originator: YES File Added: build_py.diff ---------------------------------------------------------------------- Comment By: Raghuram Devarakonda (draghuram) Date: 2007-05-17 15:17 Message: Logged In: YES user_id=984087 Originator: YES File Added: 1668596_test.tar.gz ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1720897&group_id=5470 _______________________________________________ Patches mailing list Patches@python.org http://mail.python.org/mailman/listinfo/patches
