[Patches] [ python-Patches-1692664 ] warnings.py gets filename wrong for eval/exec

[SourceForge.net]

Patches item #1692664, was opened at 2007-04-01 22:23
Message generated for change (Comment added) made by nnorwitz
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1692664&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: Library (Lib)
Group: Python 2.6
>Status: Closed
>Resolution: Rejected
Priority: 5
Private: No
Submitted By: Adam Olsen (rhamphoryncus)
>Assigned to: Neal Norwitz (nnorwitz)
Summary: warnings.py gets filename wrong for eval/exec

Initial Comment:
warnings.warn() gets the filename using the globals' __file__ attribute.  When using eval or exec this is often not the context in which the current line was compiled from.  The line number is correct, but will be applied to whatever file the globals are from, leading to an unrelated line being printed below the warning.

The attached patch makes it use caller.f_code.co_filename instead.  This also seems to remove the need to normalize .pyc/.pyo files, as well as not needing to use sys.argv[0] for __main__ modules.

It also cleans up warnings.warn() and adds three unit tests.

----------------------------------------------------------------------

>Comment By: Neal Norwitz (nnorwitz)
Date: 2007-04-23 22:04

Message:
Logged In: YES 
user_id=33168
Originator: NO

Unfortunately, test_bsddb3 has lots of problems. :-(  They vary by
platform and version of bsddb used.

Given the current code works, I'm not sure it's worth spending time
cleaning this up.  Especially since I plan to move this code to C.  It
might be worthwhile to separate out the warning guard.  I can't remember if
the __warningregistry__ was mucked with in multiple places.  If it's only
used in the one place, I'm not sure that's worth it either.

Learning import is a test of manhood. :-)

----------------------------------------------------------------------

Comment By: Adam Olsen (rhamphoryncus)
Date: 2007-04-23 15:01

Message:
Logged In: YES 
user_id=12364
Originator: YES

I "forgot", test_bsddb3 has problems too, but again they're not related to
my patch.

----------------------------------------------------------------------

Comment By: Adam Olsen (rhamphoryncus)
Date: 2007-04-23 14:26

Message:
Logged In: YES 
user_id=12364
Originator: YES

Neal:
Comment added.

regrtest -R and -u all report no problems specific to this patch.  A clean
build of trunk does show problems without it, with test_structmembers,
test_tcl, test_telnetlib, test_unicode_file, and test_uuid.  At this point
I've seen so many weird failures that I think I've started repressing them.
;)

When no stack frame is available warn() falls back to sys as the module. 
Since I'm trying to avoid claiming a warning came from a different file, I
made it appear to come from <no context> instead.  Looking at it now though
I wonder if I should go a step further, not having a __warningregistry__ or
module at all for such warnings.

Guido:
I see what you mean about stale co_filename attributes.  I'm not sure what
the best fix is now.  I'm not exactly enthused at having to learn how the
import mechanisms work. ;)
File Added: python2.6-warningfilename5.diff

----------------------------------------------------------------------

Comment By: Guido van Rossum (gvanrossum)
Date: 2007-04-23 10:31

Message:
Logged In: YES 
user_id=6380
Originator: NO

The main reason for using __file__ instead of the filename baked into the
code object is that sometimes .pyc files are moved after being compiled;
the code object is not updated, but __file__ has the correct path as it
reflects how the module was found during import.  Also, code objects may
have relative pathnames.

----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2007-04-22 23:15

Message:
Logged In: YES 
user_id=33168
Originator: NO

I'm not sure why all the code was in there to begin with it was ancient
history.  Hopefully Guido remembers.  Guido, do you see any particular
issue with this approach (ie, removing all the gyrations in warnings and
using f_code.co_filename?

There is a difference in behaviour.  If a module modifies its __file__,
the old code would pick up the modified version.  This new code will find
the real file.  I'm not sure if that's a bug or a feature though. :-) 
Similarly with __name__.

Adam, I like adding the warning_guard using a with statement.  Although I
don't think comment should be completely removed.  Could you add a comment
about why the guard is necessary (ie, the repeated calls/-R part)?

Have you tested with -R and with -u all?  ie:

  ./python -E -tt ./Lib/test/regrtest.py -R 4:3:
and
  ./python -E -tt ./Lib/test/regrtest.py -u all

(two separate runs).

In the last test case you are replacing (the old spam7), why does "sys"
become "no context"?

----------------------------------------------------------------------

Comment By: Adam Olsen (rhamphoryncus)
Date: 2007-04-15 18:34

Message:
Logged In: YES 
user_id=12364
Originator: YES

I've rewritten the unit tests I added to follow the style of
walter.doerwald's changes to test_warnings.py (commited to SVN during the
same period in which I posted my patch).

The guard_warnings_registry() context manager is now in test_support.
File Added: python2.6-warningfilename4.diff

----------------------------------------------------------------------

Comment By: Adam Olsen (rhamphoryncus)
Date: 2007-04-04 14:28

Message:
Logged In: YES 
user_id=12364
Originator: YES

sys._getframe(sys.maxint) produces an OverflowError on 64bit GCC.  Patch
changed to use sys._getframe(10**9) instead.
File Added: python2.6-warningfilename3.diff

----------------------------------------------------------------------

Comment By: Adam Olsen (rhamphoryncus)
Date: 2007-04-03 00:00

Message:
Logged In: YES 
user_id=12364
Originator: YES

The test_stackoverflow_message test I added causes warnings.py to use
sys.__warningregistry__ rather than test_warnings.__warningregistry__. 
This circumvents this self-declared hack of deleting
test_warnings.__warningregistry__ to allow regrtest -R to run.

This version of the patch uses a more general solution to allowing
regrtest -R to run.  Probably doesn't qualify as a hack anymore...
File Added: python2.6-warningfilename2.diff

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1692664&group_id=5470