Merge lp://staging/~sil2100/cupstream2distro/fix_reverter_paths into lp://staging/cupstream2distro

FAILED: Continuous integration, rev:864
http://jenkins.qa.ubuntu.com/job/cu2d-choo-choo-ci/408/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/cu2d-choo-choo-ci/408/rebuild

I support this idea in theory, but you'll need to move the SILO_DIR assignment down to the first line of main() in order to appease the tests.

PASSED: Continuous integration, rev:865
http://jenkins.qa.ubuntu.com/job/cu2d-choo-choo-ci/409/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/cu2d-choo-choo-ci/409/rebuild

Moved it to the main() function and mentioning how ugly this is (with the whole rationale added) - since I suppose importing something in the middle of a function is not really perfect. But it has to be like that for the override to work.

But at least tests are happy and the reverter is happy - with not too many changes needed anywhere.

Wait, what? Did you actually test it before writing this? You shouldn't need to import packagemanager in the main() function. The whole reason that SILO_DIR is a function instead of just a string is that other functions which need that value call it *when they need it*, not at import time.

I just doublechecked, packagemanager doesn't run any code at import time, it only defines functions. There should be no problem at all to do your imports properly and then just define SILO_DIR at the start of main().

But there is a problem, sadly. You can try it yourself - if you move the import statement to the standard place up with other imports, the source_package_download_dir (which is generated with the use of SILO_DIR) is wrong, as it points to ~/silos (and the revert fails)
Did you try it before actually rejecting?

I don't know Python internals, but it looks to me that when an import happens python fetches the list and definitions of functions/classes at the current time, so actually the order of imports is important. Since packagemanager.py imports SILO_DIR in the way of explicitly taking from cupstream2distro.utils, before we ever use packagemanager in the reverter we need to make sure that the override for SILO_DIR() is already in place. Otherwise it's just using the old one. Not a python expert but this make sense for me in my C-oriented brain, as the #include statement works somewhat similar.

Maybe there is a less ugly way of doing it, I don't know. So far this is the only working solution I found that uses the concept of a function override.

No I didn't check it, sorry ;-)

The thing is, if you look at the code in packagemanager.py it contains only `def` statements. That means it defines functions, but it doesn't actually run any of the code you see in the function bodies at import time. Python is a dynamic language, so you can override functions at any time. So when you call those functions and then they call SILO_DIR, it will call whatever version you tell it.

If it doesn't work without moving the import statement, my first guess would be that you need to redefine SILO_DIR function in more than one place.

I'll poke at this a little bit today. I'm confident that there exists a solution to this that doesn't require moving the import statement into the main() function, which indeed is quite ugly.

Oh, I see what's happening, it's a namespace issue. When you override utils.SILO_DIR, it doesn't change the value of SILO_DIR in packagemanager class, because it's already been imported (ie, packagemanager module and utils module both contain references to the SILO_DIR function, when you replace it in utils, you're not "redefining the function", you're just replacing the reference in utils module from one function to another, and packagemanager module keeps the reference to the old function.

By changing the order of imports like this, you're ensuring that packagemanager class gets the new reference to the newly defined function. If you instead redefine packagemanager.SILO_DIR it should work much more easily. Try this out:

https://code.launchpad.net/~robru/cupstream2distro/fix-reverter-silo-dir/+merge/248285