@xjrc: RE: solution 1, we could do what we do for configure, make, and cmake -- we add a special Executable object to the package environment and put some default args on it. In this case we could add a module-scope setup_py executable that includes --no-user-cfg. That would be simpler than the patching required on the second two options, and users who tweak their python setup beyond the defaults would need to know to add the --no-user-cfg option themselves.

The cmake callable object is similar -- it adds things like -DCMAKE_BUILD_TYPE=RelWithDebInfo and -DCMAKE_INSTALL_PREFIX=$prefix.

what do you think?

@tgamblin: This sounds perfectly reasonable to me. The only concern that I have is that this will require updating all of the existing Python extension packages so that they use setup_py(*args) instead of python('setup.py', *args) in order to be unaffected by user configuration. This isn't hard to do with sed, but it will mean that a lot of packages will be changed with this update. Is that okay?

@xjrc: this would not be such a bad thing. We'd need to make sure the PR works for all the users, but we can announce this at the telcons and on the mailing list and let them try it out before we throw it in the mainline. I think that should be sufficient.

@tgamblin: Okay, that sounds good to me; I'll get to work on writing this up. Thanks for your help!

Please look at PR #543, which adds some functionality to CMake-based projects. A similar pattern might be useful for Python-based projects.

in particular, PR #543 creates a subclass of Package called CMakePackage. Package authors using CMakePackage don't implement install(), instead they implement configur_args(), which is responsible only for returning package-specific CMake configuration args. CMakePackage has its own install() method that calls configure_args(). A similar pattern for subclass PythonPackage would allow PythonPackage to do all sorts of stuff without forcing each package instance to do anything specific, or even call some specific magic function.

    def configure_args(self):
        spec = self.spec
        return [
            '-DUSE_EVERYTRACE=%s' % ('YES' if '+everytrace' in spec else 'NO'),
            '-DBUILD_PYTHON=%s' % ('YES' if '+python' in spec else 'NO'),
            '-DBUILD_GRIDGEN=%s' % ('YES' if '+gridgen' in spec else 'NO'),
            '-DBUILD_COUPLER=%s' % ('YES' if '+coupler' in spec else 'NO'),
            '-DUSE_PISM=%s' % ('YES' if '+pism' in spec else 'NO')]

@tgamblin: I finished writing the code that adds the setup_py executable to all Python extensions and a script that converts existing uses of python('setup.py', *args) to setup_py(*args) (which can be run and discarded after these changes are merged).

Just a few quick notes about some things that I noticed while making these changes:

• The py-sip and py-pyqt packages are the only Python extension packages that don't install using the standard python('setup.py', *args) method. These packages are installed using custom install scripts that don't use Python's distutils library (which coordinates with the user/system/distribution configuration files), so they will most likely ignore user configuration even though they don't use setup_py.
• The --no-user-cfg option is only supported by Python v2.7 and v3.4+ (see this Python issue for details). If any Python version outside of this range is added to Spack, a much more involved solution will be required to prevent user configuration from affecting Spack installs. I've added a warning about this in the Python package when one of these versions is installed.

@citibeth: I've looked over #543 and I think you have a good point about applying a similar subclassing approach for Python packages. My only hangup is that most Python packages just call python('setup.py', (build|install), '--prefix=%s' % prefix) with very little variation, so adding infrastructure to support further customizations seems like it won't add much value for package developers (unlike the CMake case where this is valuable because there are many variations). I suppose that it wouldn't hurt to add a PythonPackage superclass with python('setup.py', 'install', ...) as the default install behavior to remove all of the boilerplate installs from the Python extension packages, but I think that this is orthogonal enough to the intent of this PR to deserve its own PR. What do you think @tgamblin?

@tgamblin: I've updated this pull request so that it no longer conflicts with the develop branch. I retested the [email protected] and [email protected] installs and they seem to still be working after the merge. I had to resolve some nontrivial merge conflicts in the python package file, so any additional testing by other users (particularly anyone that uses the +tk and +ucs4 variants e.g. @adamjstewart, @paulhopkins) would be greatly appreciated.

Please let me know if there are any issues with the merged code and I'll try to fix them as soon as possible!

@xjrc: thanks!

@lee218llnl @adamjstewart: does this work for you guys?

There seem to be a lot of unnecessary changes to the Python package that don't affect how it installs nor do they solve flake8 issues. They also make it less future proof by reverting to the old Python 2 string formatting syntax.

@adamjstewart: Sorry about all of the unnecessary changes; this pull request has been sitting around since before the python package was refactored for flake8 checks, so I had to merge with the refactored version and I used my own changes when the refactoring was equivalent. I certainly don't mind reverting to the current version of the python package and simply introducing the important changes that I made in this PR. In fact, that would probably be better since there would be less errors to worry about. Thoughts?

I figured that was the cause. It would certainly make it easier to review.

@lee218llnl, @adamjstewart

@adamjstewart: I've updated this pull request so that it includes changes that are less intrusive on the python package. I left in a few refactoring changes that I felt were valuable (e.g. the build flag changes in the install method), but if there are any issues with these please let me know.

@tgamblin: This pull request makes the history for the python package incredibly messy, so I'd like to try and rewrite the history before this PR is merged. I'm hoping to have this done within the next hour or so.

@tgamblin: I just finished cleaning up the history, so this PR should be ready for merging after it has been reviewed.

LGTM. Aside from running that script on all of the python extensions, I think the last thing that should be done is to edit spack create to use the new default syntax. All you should need to change is this line.

@adamjstewart: good suggestion! @xjrc: can you change spack create to use this?

@xjrc: thanks for the implementation!

@adamjstewart, @tgamlin: Thanks to both of you for your help/suggestions! I'll create a pull request for the change to spack create in a couple of minutes.

Uhh, I don't think this was ready to be merged. Weren't we going to run the script that you wrote to regex replace python with setup_py for all python packages?

It seems to me like the python('setup.py', ...) to setup_py(...) transformation and spack create change would fit well in this PR. Would you prefer to revert and include these changes in this PR or to include them in separate PRs, @tgamblin?

Sorry to join in late, but this all looks OK to me. I don't maintain a personal distutils config file, so this hasn't affected me in the past, but I can see this being useful for other users.

@xjrc Either way, the script that you wrote doesn't need to be in the Spack repository. I thought it was just a way to show what you will run before things get merged. I'm glad it's backwards compatible regardless.

@adamjstewart: My motivation for including the script was to demonstrate how I was changing each Python install (as you mentioned); I didn't really intend for it to remain in the Spack repository indefinitely. In retrospect, I think that it may have been better to just run the script and have all the changes bundled into a this PR. At any rate, I'll be sure to remove this script from Spack when I submit a new PR that finalizes the changes started here.