[Python-checkins] r54291 - in python/trunk: Include/pystate.h Lib/test/infinite_reload.py Lib/test/test_import.py Misc/NEWS Python/import.c Python/pystate.c Python/pythonrun.c

[Neal Norwitz]

Hi Colin.

Thanks for making this change and backporting it.  I have a couple of
comments though.  Don't worry I nag everyone. :-)

It's great to see the entries in Misc/NEWS and new tests.  It would
also be nice when people make a moderate contribution, we include
their name in Misc/ACKS.  I think I got the person that submitted this
patch and a few others.  Just try to remember for future commits.


On 3/12/07, collin.winter <python-checkins at python.org> wrote:
>
> Modified: python/trunk/Python/import.c
> ==============================================================================
> --- python/trunk/Python/import.c        (original)
> +++ python/trunk/Python/import.c        Mon Mar 12 17:11:39 2007
> @@ -340,6 +340,25 @@
>         return Py_None;
>  }
>
> +PyObject *
> +PyImport_GetModulesReloading(void)
> +{
> +       PyInterpreterState *interp = PyThreadState_Get()->interp;
> +       if (interp->modules_reloading == NULL)
> +               Py_FatalError("PyImport_GetModuleDict: no modules_reloading dictionary!");
> +       return interp->modules_reloading;
> +}

This function is not exported in the header file.  It is only used in
this file and only in one place.  At the least, it seems like it
should be static.  Since it's only used in one place, I would probably
inline it too.  However you chose to fix it is fine.

> @@ -2401,8 +2421,9 @@
>  PyObject *
>  PyImport_ReloadModule(PyObject *m)
>  {
> +       PyObject *modules_reloading = PyImport_GetModulesReloading();
>         PyObject *modules = PyImport_GetModuleDict();
> -       PyObject *path = NULL, *loader = NULL;
> +       PyObject *path = NULL, *loader = NULL, *existing_m = NULL;
>         char *name, *subname;
>         char buf[MAXPATHLEN+1];
>         struct filedescr *fdp;
> @@ -2423,20 +2444,30 @@
>                              name);
>                 return NULL;
>         }
> +       if ((existing_m = PyDict_GetItemString(modules_reloading, name)) != NULL) {
> +        /* Due to a recursive reload, this module is already being reloaded. */
> +        Py_INCREF(existing_m);
> +        return existing_m;
> +       }
> +       PyDict_SetItemString(modules_reloading, name, m);

I fixed a bunch of formatting and added a check for this call failing.
 Those changes should probably be back ported from rev 54321.  (I
don't really care about the style nits, it's the check that is more
important.)

The style things were stuff like:
  * lines too long
  * wrong indentation
  * space after a function name
  * wrong function name in error string
  * simplifying some logic

n