Skip to content

mrtrix3: Fix some issues w/ 3.0.3 and add 3.0.4#41036

Merged
alalazo merged 3 commits intospack:developfrom
moloney:mrtrix3-fixes
Nov 15, 2023
Merged

mrtrix3: Fix some issues w/ 3.0.3 and add 3.0.4#41036
alalazo merged 3 commits intospack:developfrom
moloney:mrtrix3-fixes

Conversation

@moloney
Copy link
Copy Markdown
Contributor

@moloney moloney commented Nov 13, 2023

Fixes issues with the 'configure' script replacing "-I " args with "-idirafter ". Set needed "eigen" version based on mrtrix3 version. Add 3.0.4 version.

@spackbot-app
Copy link
Copy Markdown

spackbot-app bot commented Nov 13, 2023

Hi @moloney! I noticed that the following package(s) don't yet have maintainers:

  • mrtrix3

Are you interested in adopting any of these package(s)? If so, simply add the following to the package class:

    maintainers("moloney")

If not, could you contact the developers of this package and see if they are interested? You can quickly see who has worked on a package with spack blame:

$ spack blame mrtrix3

Thank you for your help! Please don't add maintainers without their consent.

You don't have to be a Spack expert or package developer in order to be a "maintainer," it just gives us a list of users willing to review PRs or debug issues relating to this package. A package can have multiple maintainers; just add a list of GitHub handles of anyone who wants to volunteer.

Comment on lines +34 to +35
depends_on("eigen@3.3", when="@3.0.3")
depends_on("eigen@3.4", when="@3.0.4:")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it necessary to pin the dependencies this specifically? Or can the ranges be broader?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

According to their docs they require eigen >= 3.2 with >= 3.3 recommended. These docs are actually slightly out-of-date as 3.0.4 does in fact require 3.4 to build.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Greater or equal is expressed as:

Suggested change
depends_on("eigen@3.3", when="@3.0.3")
depends_on("eigen@3.4", when="@3.0.4:")
depends_on("eigen@3.3:", when="@3.0.3")
depends_on("eigen@3.4:", when="@3.0.4:")

(note the additional :).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah, sorry I forgot to mention that 3.0.3 won't compile with with eigen@3.4 due to conflicting declarations (which isn't mentioned in the docs).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In that case, can you add a comment about that and use a range for the other directive

Suggested change
depends_on("eigen@3.3", when="@3.0.3")
depends_on("eigen@3.4", when="@3.0.4:")
# <Brief comment explaining why only 3.3>
depends_on("eigen@3.3", when="@3.0.3")
depends_on("eigen@3.4:", when="@3.0.4:")

?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok, should be resolved with latest commit. I also put a version upper bound on the patch since it sounds like it should be fixed upstream shortly.

@alalazo alalazo self-assigned this Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants