Skip to content

Conversation

@octomike
Copy link

Matlab/MCR is doing a weird thing (there should be a name for it) when using fileread() on a file directly in /, which results in the -v/--version switch to output garbage on the terminal.

Minimal example:

docker run --entrypoint /bin/bash -it --rm bids/spm:v0.0.20 -c "echo -e \"fileread('/version')\nquit\" > test.m ; /opt/spm12/run_spm12.sh /opt/mcr/v97 script test"
------------------------------------------
Setting up environment variables
---
LD_LIBRARY_PATH is .:/opt/mcr/v97/runtime/glnxa64:/opt/mcr/v97/bin/glnxa64:/opt/mcr/v97/sys/os/glnxa64:/opt/mcr/v97/sys/opengl/lib/glnxa64
SPM12, version 7771 (standalone)
MATLAB, version 9.7.0.1319299 (R2019b) Update 5
 ___  ____  __  __                                            
/ __)(  _ \(  \/  )                                           
\__ \ )___/ )    (   Statistical Parametric Mapping           
(___/(__)  (_/\/\_)  SPM12 - https://www.fil.ion.ucl.ac.uk/spm/


ans =

    'V1MCC4000MEC1000MCR1000x  ���V=�W���i�{��H �k�����G������w������;B�+��]v����
[...]

However, adding a second / resulting in //version, everything works out again:

docker run --entrypoint /bin/bash -it --rm bids/spm:v0.0.20 -c "echo -e \"fileread('//version')\nquit\" > test.m ; /opt/spm12/run_spm12.sh /opt/mcr/v97 script test"
------------------------------------------
Setting up environment variables
---
LD_LIBRARY_PATH is .:/opt/mcr/v97/runtime/glnxa64:/opt/mcr/v97/bin/glnxa64:/opt/mcr/v97/sys/os/glnxa64:/opt/mcr/v97/sys/opengl/lib/glnxa64
SPM12, version 7771 (standalone)
MATLAB, version 9.7.0.1319299 (R2019b) Update 5
 ___  ____  __  __                                            
/ __)(  _ \(  \/  )                                           
\__ \ )___/ )    (   Statistical Parametric Mapping           
(___/(__)  (_/\/\_)  SPM12 - https://www.fil.ion.ucl.ac.uk/spm/


ans =

    'v0.0.20-2-gd739e10
     '

This trivial PR puts version in SPM_DIR, as I couldn't find words for a comment explaining fileread('//version')

In an MCR environment directly reading a file from / fails, work around
that.
@gllmflndn
Copy link
Collaborator

This is indeed very odd - it seems to return the (compiled) content of a file version.m stored elsewhere in the filesystem:

root:/# /opt/spm12/run_spm12.sh /opt/mcr/v97 eval "which('/version','-all')"
SPM12, version 7771 (standalone)
MATLAB, version 9.7.0.1319299 (R2019b) Update 5
 ___  ____  __  __                                            
/ __)(  _ \(  \/  )                                           
\__ \ )___/ )    (   Statistical Parametric Mapping           
(___/(__)  (_/\/\_)  SPM12 - https://www.fil.ion.ucl.ac.uk/spm/

/opt/mcr/v97/mcr/toolbox/matlab/codetools/@mtree/version.m  % mtree method

version is also a built-in MATLAB command. I would prefer to fix the root cause of the issue (as it used to work with previous versions of the MCR) and thought that /version was expected to be found in BIDS Apps but it doesn't seem to be the case in prominent apps:
https://github.com/poldracklab/fmriprep/blob/8ea6db905d4b0a86ab65b3e8f8213e29f70e27c0/Dockerfile#L184-L186
so I am OK to use /opt/spm12/VERSION. Two further comments:

@octomike
Copy link
Author

It would be preferable if spm_BIDS_App.m was also working outside of a docker container

Sure, I can catch this and amend the PR.

The content of VERSION should also probably be the SHA-1 checksum of the latest commit, i.e. relying on ARG as in:

I'm a bit puzzled here. When and where should the hash be written, exactly? Wouldn't this only work in the Dockerfile (assuming it is always build from within a git repo) or circleCI.
Otherwise, how would you persist the hash in the repo?

@gllmflndn
Copy link
Collaborator

Thanks! If I'm not mistaken, there wouldn't be any VERSION file in this repository but an environment variable would be passed on at build time (docker build ... --build-arg VERSION=XXX). We would then need to be robust in spm_BIDS_App.m if VERSION file does not exist (not in a Docker environment) or is empty (the variable was not defined during build).

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.

2 participants