{"action":"created","issue":{"url":"https://api.github.com/repos/LLNL/spack/issues/552","repository_url":"https://api.github.com/repos/LLNL/spack","labels_url":"https://api.github.com/repos/LLNL/spack/issues/552/labels{/name}","comments_url":"https://api.github.com/repos/LLNL/spack/issues/552/comments","events_url":"https://api.github.com/repos/LLNL/spack/issues/552/events","html_url":"https://github.com/LLNL/spack/pull/552","id":141010681,"number":552,"title":"enhancement proposal : customization of module files ","user":{"login":"alalazo","id":4199709,"avatar_url":"https://avatars.githubusercontent.com/u/4199709?v=3","gravatar_id":"","url":"https://api.github.com/users/alalazo","html_url":"https://github.com/alalazo","followers_url":"https://api.github.com/users/alalazo/followers","following_url":"https://api.github.com/users/alalazo/following{/other_user}","gists_url":"https://api.github.com/users/alalazo/gists{/gist_id}","starred_url":"https://api.github.com/users/alalazo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alalazo/subscriptions","organizations_url":"https://api.github.com/users/alalazo/orgs","repos_url":"https://api.github.com/users/alalazo/repos","events_url":"https://api.github.com/users/alalazo/events{/privacy}","received_events_url":"https://api.github.com/users/alalazo/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":12,"created_at":"2016-03-15T15:36:06Z","updated_at":"2016-03-18T12:06:07Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/LLNL/spack/pulls/552","html_url":"https://github.com/LLNL/spack/pull/552","diff_url":"https://github.com/LLNL/spack/pull/552.diff","patch_url":"https://github.com/LLNL/spack/pull/552.patch"},"body":":warning: This is a work in progress displayed to receive early feedback and agree on implementation details\r\n\r\n##### Objective \r\nBeing able to customize what gets written in a module file\r\n\r\n##### TLDR\r\nCurrently the modifications that gets written in a module file are:\r\n\r\n- generic modifications to some list of paths in the environment\r\n\r\nThese modifications are obtained inspecting `pkg.prefix` according to the same set of rules for every package. I would like to extend this process allowing:\r\n\r\n- package-specific modifications to the environment (this PR)\r\n- site-specific modifications via a configuration file (a subsequent PR)\r\n\r\n##### Current Status\r\n- [x] class that manages environment modifications during both installation and module file creation (module + unit tests)\r\n- [x] environment variables set in a package are written to tcl or dotkit module files\r\n- [x] polished the interface that package maintainers will need to use\r\n- [ ] validation of environment modifications (i.e. detect conflicting changes to the same variable issued by different packages)\r\n\r\n##### Examples of use\r\n1. `openmpi` and `mpich` packages in this PR set the environment variables for compiler wrappers differently depending on whether they are used as a dependency during an installation or outside of spack via module files\r\n\r\n##### Details\r\nThe biggest change done in this PR is the introduction of a class (`EnvironmentModifications`) to manage environment modifications both during install and module file creation. An object of this class is essentially a FIFO queue containing requests to modify the environment. Those requests can be executed at a later time and from a scope other than where they were created (in which case the environment is modified and the list is cleared.) \r\n\r\nAlternatively the requests can be inspected and processed. This is what is done during module file creation when each module file writer (`TclModule`, `Dotkit`) maps each single request to a formatted line and writes that line to a file.\r\n\r\nAn interesting side effect of this design is the possibility to inspect the whole set of requests done by different packages before applying them. This could permit to reveal inconsistencies (e.g.  variables that are set by a package and unset by another) and output diagnostic messages to the user."},"comment":{"url":"https://api.github.com/repos/LLNL/spack/issues/comments/198325462","html_url":"https://github.com/LLNL/spack/pull/552#issuecomment-198325462","issue_url":"https://api.github.com/repos/LLNL/spack/issues/552","id":198325462,"user":{"login":"citibeth","id":1924215,"avatar_url":"https://avatars.githubusercontent.com/u/1924215?v=3","gravatar_id":"","url":"https://api.github.com/users/citibeth","html_url":"https://github.com/citibeth","followers_url":"https://api.github.com/users/citibeth/followers","following_url":"https://api.github.com/users/citibeth/following{/other_user}","gists_url":"https://api.github.com/users/citibeth/gists{/gist_id}","starred_url":"https://api.github.com/users/citibeth/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/citibeth/subscriptions","organizations_url":"https://api.github.com/users/citibeth/orgs","repos_url":"https://api.github.com/users/citibeth/repos","events_url":"https://api.github.com/users/citibeth/events{/privacy}","received_events_url":"https://api.github.com/users/citibeth/received_events","type":"User","site_admin":false},"created_at":"2016-03-18T12:06:07Z","updated_at":"2016-03-18T12:06:07Z","body":"I agree, multiple inheritance is distasteful.  Although maybe it would be\nOK if we set up a pattern and just tell users to use it.\n\nAn alternative would be to always inherit from Package and use monkey\npatching to provide mix-in capability.  For example, suppose we have a\ncommand use() that finds all methods defined in a mix-in and monkey-patches\nthem into the current class instance.  Then I could write:\n\n```\nclass MyPackage(Package):\n    def __init__(self):\n        self.use(CMakePackage)\n        self.use(DependentEnvPackage)\n    ...\n```\n\n\nOr alternatively, maybe we can set things up so we do this at the class\nlevel, rather than instance level (probably possible):\n\n\n```\nclass MyPackage(Package):\n        use(CMakePackage)\n        use(DependentEnvPackage)\n    ...\n```\n-- Elizabeth\n\n\nOn Fri, Mar 18, 2016 at 3:04 AM, Massimiliano Culpo <\nnotifications@github.com> wrote:\n\n> @citibeth <https://github.com/citibeth> I guess not in a package :\n> actually @tgamblin <https://github.com/tgamblin> made a very good point\n> above of maintaining code writing simple for packages. I think that in this\n> PR I will either find a way to patch python extension packages or I'll\n> revert to the previous behavior.\n>\n> Then : it's not much I code in python (so take what I say now with a grain\n> of salt), but I guess that using mixins will require some strict\n> conventions on how method calls are coded. The first thing that comes to my\n> mind is a pervasive use of super whenever you override a call to avoid\n> breaking the MRO chain.\n>\n> In the example you posted e.g. it makes no sense to add Mixin1 as it adds\n> nothing to behavior. If Mixin1 and Mixin2 behaviors happen to be\n> orthogonal (i.e. they are composable), then the test method should have\n> been written like :\n>\n> def test():\n>     super().test()\n>     print <tag>\n>\n> The same is true for every method overridden by the mixin. Order starts to\n> count in your inheritance and in the placement of the super() call within\n> the method, if somebody doesn't call super it breaks the chain of calls\n> for anything that comes after him in the MRO... I am not sure we should ask\n> to know such things to package maintainers...\n>\n> —\n> You are receiving this because you were mentioned.\n> Reply to this email directly or view it on GitHub\n> <https://github.com/LLNL/spack/pull/552#issuecomment-198233075>\n>\n"}}