Skip to content

Conversation

@Lestropie
Copy link
Member

@Lestropie Lestropie commented Feb 7, 2021

As discussed in MRtrix3/containers#14.

Plan is to have the primary container building recipes inside the main repository, and for the "containers" repository to only be responsible for building the minified dependencies.

  1. In order for the recipe file contained within any particular tagged version of MRtrix3 to be guaranteed to clone that very same version of MRtrix3 during the build process, the recipe files must be manually updated to reflect the upcoming tag name, similarly to the gymnastics already done with core/version.h. The Wiki instructions have already been updated accordingly.

  2. I have moved some of the contents of README.md from the containers repo into a new page in the online documentation page.

  3. Reflecting on the contents of README.md in this repository, there's quite a lot of stripped-down instructions for compilation from source. Not only is it less comprehensive that the online documentation for building from source, but we expect over time that less people will be building from source. It might be worth contemplating removing these instructions altogether and instead relying on selection of which hyperlink to follow based on the desired installation route.

  4. It would be preferable to add instructions for running mrview via containers on OSX and Windows prior to merge.

  5. This is currently targeted for master. I don't think there should actually be any problem with either merging this to master prior to tagging, or with bundling it up with whatever hotfixes go into the next macro release, though I don't actually recall the result of discussion on the latter. Documentation may not cover all bases if it's included in a macro release, but that can nevertheless be revised on master if necessary. Note that dwi2mask overhaul (replacement PR) #2197 will necessitate the construction of new container dependency downloads, but that targets 3.1.0, so can be addressed on a separate branch on the MRtrix3/containers repo.

  6. This PR does not yet address the prospect of automatic building of containers. Given the relative occurrence of updates, such automation is not necessarily a prerequisite for this PR.

@Lestropie Lestropie added this to the 3.0.3 hotfix milestone Feb 7, 2021
@Lestropie Lestropie self-assigned this Feb 7, 2021
jdtournier
jdtournier previously approved these changes Feb 8, 2021
Observed to be necessary to permit .sif file to be utilised directly as executable rather than explicitly invoking through singularity command.
@Lestropie
Copy link
Member Author

@MRtrix3/mrtrix3-devs: Could consider merging this into 3.0.3 but not immediately broadcasting publicly, and the code is then there in master that we can go through the process of setting up on DockerHub etc., get feedback from any sysadmins who try to use it, and then automate those processes via Actions to happen from the next update onwards.

Copy link
Member

@jdtournier jdtournier left a comment

Choose a reason for hiding this comment

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

Quite happy to merge this - it adds functionality without altering anything else, and it would be good to get feedback from container admins as early as possible. 👍

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.

4 participants