Opened 11 years ago
Closed 11 years ago
#39352 closed defect (fixed)
arpack @3.1.2 -openmpi cannot build without mpi
Reported by: | dstrubbe (David Strubbe) | Owned by: | macports-tickets@… |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | ports | Version: | 2.1.3 |
Keywords: | haspatch | Cc: | mamoll (Mark Moll), cooljeanius (Eric Gallager) |
Port: | arpack |
Description
configure always receives --enable-mpi, even when neither +openmpi or +mpich was set, and -openmpi was set to remove the default +openmpi. As a result, configure will fail if mpi is not installed. Instead, --enable-mpi should be set only with +openmpi or +mpich. Then, -openmpi can build successfully, just without PARPACK.
I also noticed some other improvements that can be made:
- The description and variants do not make clear that PARPACK is included too (used to be in variants but was removed at some point, not clear why).
- Rather than patching configure for atlas (as discussed in #34695), it seems wiser to specify blas explicitly. In fact, if you do that, the lapack argument is not needed. Similarly, specifying blas explicitly for accelerate makes it no longer the case that +accelerate has no effect if atlas is installed, as was stated in a comment in the Portfile.
- ARPACK doesn't seem to use threads. Therefore, it seems better to use the sequential -lsatlas rather than the threaded -ltatlas, or at least to offer the user a choice about threads. The discussion in #34695 didn't seem to have any reason for choosing -ltatlas.
- The path dependency on mpicc seems unwise: the code actually uses mpif77, not mpicc (there is no C code), so I think it is better to have a dependency on mpif77.
- If the reason there is no universal variant is because of openmpi, as stated in the comment, then it seems that should apply only with +openmpi, i.e. by a variant conflict.
- Removed some trailing spaces, as warned by nitpick.
The attached Portfile diff addresses these issues. Let me know if you have questions or doubts about any of this. (I also added a comment about why LDFLAGS is being defined, since it looks like it is just the default.)
Attachments (1)
Change History (4)
Changed 11 years ago by dstrubbe (David Strubbe)
Attachment: | Portfile-arpack.diff added |
---|
comment:1 Changed 11 years ago by dstrubbe (David Strubbe)
comment:3 Changed 11 years ago by mamoll (Mark Moll)
Resolution: | → fixed |
---|---|
Status: | new → closed |
Thanks for the patch. Applied in r106725.
Note: See
TracTickets for help on using
tickets.
I forgot to mention, due to these changes, I also recommend deletion of patch-configure-atlas.diff which is no longer necessary.