-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Package foam-extend #1002
Package foam-extend #1002
Conversation
…nto packages/foam-extend
I was actually just about to start working on an OpenFOAM package. I'll see how much of this I can copy. |
|
||
include_path = join_path(self.prefix, 'include') | ||
lib_path = join_path(self.prefix, 'lib') | ||
bin_path = join_path(self.prefix, 'bin') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
join_path(self.prefix, 'bin') == prefix.bin
The same goes for lib and include. Might as well use the shorter version everywhere, no need to store them in variables.
I'm also unable to install Paraview for some reason. I'll try to investigate that further. OpenFOAM lists Paraview as "essential, without an alternative, compatible visualisation tool", so it should probably default to True, or not be a variant at all. |
In fact if the output files are generated so they could always be copied on another system to be visualized. On what I got is mainly there wrapper |
for line in proc.stdout: | ||
match_grp = variable_set.match(line) | ||
if match_grp: | ||
env[match_grp.group(1)] = match_grp.group(2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using regular expressions and requiring the re module, it might be simpler to use:
key, value = line.split('=', 1)
env[key] = value
|
||
make(*make_opts, parallel=False) | ||
|
||
mkdirp(self.prefix.include, self.prefix.lib, self.prefix.bin) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to use self.prefix because prefix is already a parameter of the install method.
If you want to rebase and incorporate #1047, I think this PR should be ready to go. Once it's merged, I should be able to copy everything and get it working for OpenFOAM with only a few changed lines. |
I'm working on it but I bumped on other problem due to the changes in arch. |
@adamjstewart should be ok if you want to use it for open-foam, |
The intel packages inherit from each other. It's kind of messy. I'm not sure what the best way to do these kinds of things is. |
@nrichart @adamjstewart: is this ready to go? |
Looks good to me |
* First try at OpenFOAM-extend * limiting package to foam-extend to start * Ignoring the flake8 error for a line too long * First try at OpenFOAM-extend * limiting package to foam-extend to start * Removing extra dependencies + minor fix according to remarks on spack#1002 * removing useless selfs * changes to use from_sourcing_file * correcting flake8 * Changes for flex 2.6.0 and intel mpi * correction for paraview paths
Introduce a new configuration key that will enable filtering the dependencies considered by 'autoload'. Disabled by default to not change the tests from upstream. Verified manually for the CI test deployment and manual module generation to not change our desired behavior. Also add some debug printout for hard-to-reproduce CI failures.
This is a first try for the package foam-extend a fork of OpenFOAM.
In theory this could work for openfoam to but I did not try.
In addition I could not install paraview, so I did not try the paraview variant. I can remove it as long as it is not tested.
In the same PR there is also a correction for a todo in the package scotch