Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

complete lua rework #955

Merged
merged 11 commits into from
May 17, 2016
Merged

complete lua rework #955

merged 11 commits into from
May 17, 2016

Conversation

trws
Copy link
Contributor

@trws trws commented May 14, 2016

This PR completely reworks handling of lua and lua extension packages. In fact, it should never be necessary to use activate on a lua package. The new version installs both lua and luarocks together, and uses them to provide lua and luarocks spack commands. It also sets LUA_PATH and LUA_CPATH appropriately for lua and all packages installed by it. The biggest advantage of pulling luarocks into the lua package, rather than leaving it to the side, is that it can be used as a nearly universal build command for released lua projects, most now just need luarocks('--tree=' + prefix, 'install', rockspeck_path) in their install method.

The downside: I still don't have a good way to handle the lua/luajit duality. This will work for all regular luas, but luajit will not find these modules, as they're installed in interpreter versioned paths. If anyone knows a way to have two interpreters provide a language and allow extensions from the same set of child packages, I would appreciate a hint.

@trws trws force-pushed the lua-rework branch 3 times, most recently from 0f9c953 to ec2297c Compare May 14, 2016 23:17
This is a complete rework of the lua package, it also allows the
environment modification classes to handle paths that are not separated
by colons, and uses the support for same in TCL modules as well.

The biggest difference is the handling for lua extension packages, which
now have their paths set correctly by the lua parent package, and have
access to both lua and luarocks as installation tools. See the luaposix
package for what should be required for most lua packages after this.
@trws
Copy link
Contributor Author

trws commented May 15, 2016

So @alalazo, @tgamblin, the flake8 test currently emits 4795 errors, and thus travis does not pass because various packages have lines more than 79 characters long, or blank lines at the end of a file or... I'll go in and fix up some of my packages, but this is a heck of a lot of errors.

@trws
Copy link
Contributor Author

trws commented May 15, 2016

Ok, many flake8 changes later (and a merge for my local develop branch so the run_flake8 script works properly...) and we're back in business. This should pass.

@@ -238,7 +241,7 @@ def apply_modifications(self):
x.execute()


def concatenate_paths(paths):
def concatenate_paths(paths, separator=';'):
"""
Concatenates an iterable of paths into a string of column separated paths
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor thing : the function doesn't match docstring anymore

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Fixed, and it's amusing to see that it was off to begin with:
"column" -> "colon"

On 15 May 2016, at 0:10, Massimiliano Culpo wrote:

@@ -238,7 +241,7 @@ def apply_modifications(self):
x.execute()

-def concatenate_paths(paths):
+def concatenate_paths(paths, separator=';'):
"""
Concatenates an iterable of paths into a string of column
separated paths

Just a minor thing : the function doesn't match docstring anymore


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
https://github.com/LLNL/spack/pull/955/files/a2197f3a417d63f0f13eac64f016abd0528d7748#r63287309


def install(self, spec, prefix):
rockspec = glob.glob('luaposix-*.rockspec')
print rockspec
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this print statement just for debugging purposes? If so it should be removed before this is merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quite right. Removed.

@tgamblin
Copy link
Member

Ok since I don't know too much about how lua works -- how does this handle multiple versions of extensions in the same lua installation? If it's never necessary to activate, does that just mean that Lua itself allows multiple versions of the same extension to be present? If it does, what if lua extensions have variants and there are multiple configurations of the same version? Or are you expecting all lua extension stuff to be done through environment/modules?

Evidently some readline variants are built with only a dynamic dependency on
ncurses, this addresses that problem for such systems.
@trws
Copy link
Contributor Author

trws commented May 16, 2016

It's basically like python modules, but with a different path separator and search mechanism. The activate mechanism works with these packages, but so does just loading the interpreter and the specific final module dependency. It includes all intermediate requirements in the LUA_PATH and LUA_CPATH environment variables at runtime for the module (hence the separator extension to the environment module).

As to versioning, much like python, one does need to select a specific one. These are all installed in independent paths, so it should work much like C libraries currently do. The only notable difference here is that Lua's canonical package manager, luarocks, makes it very easy to build or install a given package into an arbitrary target tree, I'm just leveraging that mechanism to do a normal spack-style install.

@tgamblin tgamblin merged commit 138307d into spack:develop May 17, 2016
olupton pushed a commit to olupton/spack that referenced this pull request Feb 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants