swig: Fix issue with circular import in 2.0.9/2.0.10

Information
Submitter:	Andreas Hansson
Repository:	gem5
Branch:	default
Bugs:
Depends On:
Reviewers
Groups:	Default
People:

Description

Changeset 9856:5222bd5462f2
---------------------------
swig: Fix issue with circular import in 2.0.9/2.0.10

This patch fixes an issue which prevented gem5 from running when built
using swig 2.0.9 and 2.0.10. The generated event.py tried to import
m5.internal which in turn relied on importing event. This patch seems
to fix the problem, and so far has not caused any other issues.

Testing Done

Built on Ubuntu 10.04, RHE5, Fedora 19, Fedora RawHide, OpenSuse 12.3, OSX 10.8, with clang 3.2, 3.3, gcc 4.7, 4.8 and swig 2.0.4 and 2.0.9 and 2.0.10

I'm surprised this is an issue... normally a circular import dependency in Python is not a problem, as long as the import isn't used until both modules have had a chance to fully load.

What error message do you get?

Andreas Hansson Sept. 6, 2013, 10:11 a.m. (Sept. 6, 2013, 10:11 a.m.)

When trying to run gem5, it dies on trying to import m5.internal in event.py. It claims that the module has no such member.

This is due to the fully qualified names used by swig as of 2.0.9.

I don't claim to fully understand why this caused an issue, but I propose we adopt this for now to enable use on recent versions of popular Linux distros.

Steve Reinhardt Sept. 6, 2013, 10:33 a.m. (Sept. 6, 2013, 10:33 a.m.)

I see... I expect it's really the lookup of 'internal' in the m5 package that's failing because that import is not complete.

I'm a little concerned that we might be changing the semantics here... I don't recall all the trickeries of our Python import framework.  Is the event module still accessible from user scripts via m5.internal.event?  If so, then (1) I have no problem with this and (2) I expect that means that none of the import statements in this file are necessary.

Andreas Hansson Sept. 9, 2013, 7:31 p.m. (Sept. 9, 2013, 7:31 p.m.)

The regressions work, and a few basic runs with se.py and fs.py also show no issues. Is there something specific you think might break?

Andreas Hansson Sept. 10, 2013, 9:15 a.m. (Sept. 10, 2013, 9:15 a.m.)

The regressions work with only "import core" present in the __init__.py

Steve Reinhardt Sept. 10, 2013, 10 a.m. (Sept. 10, 2013, 10 a.m.)

Yes, I recently discovered that myself :-).  Although regressions are still running on my end, it appears you can get rid of 'import core' too with this (sensible-seeming) patch:

--- a/src/python/m5/__init__.py Mon Sep 09 18:52:23 2013 -0500
+++ b/src/python/m5/__init__.py Tue Sep 10 12:56:01 2013 -0400
@@ -32,7 +32,7 @@

 try:
     # Try to import something that's generated by swig
-    import internal
+    import internal.core

     # Try to grab something from it in case demandimport is being used
     internal.core.curTick

Ali Saidi Sept. 10, 2013, 3:12 p.m. (Sept. 10, 2013, 3:12 p.m.)

Note there aren't any tests that create checkpoints and restore in the regressions... Might want to just try that first.

Andreas Hansson Sept. 11, 2013, 8:22 a.m. (Sept. 11, 2013, 8:22 a.m.)

I have tried checkpoint/restore after the latest changes (according to Steve's comments) and it all works fine.

Andreas Hansson Sept. 15, 2013, 11:42 a.m. (Sept. 15, 2013, 11:42 a.m.)
```
All happy?
```

Description:

~		Changeset 9856:aa9d9d4f8f2d
	~	Changeset 9856:5222bd5462f2

		swig: Fix issue with circular import in 2.0.9/2.0.10

		This patch fixes an issue which prevented gem5 from running when built
		using swig 2.0.9 and 2.0.10. The generated event.py tried to import
		m5.internal which in turn relied on importing event. This patch seems
		to fix the problem, and so far has not caused any other issues.

Diff:

Revision 2 (+1 -8)

Show changes

	src/python/m5/__init__.py
	src/python/m5/internal/__init__.py

Thanks for the tweaks!

You have a pending review.

Review Board 2.0.15

This change has been marked as submitted.

Screenshots

Files

Description:

Changeset 9856:aa9d9d4f8f2d

Changeset 9856:5222bd5462f2

Diff:

Status: Closed (submitted)