ReproNim / neurodocker

Generate custom Docker and Singularity images, and minimize existing containers
https://www.repronim.org/neurodocker/
Apache License 2.0
327 stars 97 forks source link

[ENH] add templates bids_validator #586

Closed Remi-Gau closed 11 months ago

Remi-Gau commented 11 months ago

closes #565

could build an image with a dockerfile generated with the following

neurodocker generate docker --pkg-manager apt --base-image ubuntu:jammy-20221130 --bids_validator version=1.13
Remi-Gau commented 11 months ago

not tested with "yum" images

codecov[bot] commented 11 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

see 1 file with indirect coverage changes

:loudspeaker: Thoughts on this report? Let us know!

Remi-Gau commented 11 months ago

pinging @lamoglia who inspired me to finally open this PR (also because once this is part of neurodocker this will simplify the recipe for several BIDS app, including the base_validator image).

kaczmarj commented 11 months ago

this is a great addition! one suggestion is that the base image could have nodejs and npm installed. could we check if npm exists before installing node? if npm already exists, we can use the existing installation to install the bids validator.

Remi-Gau commented 11 months ago

this is a great addition! one suggestion is that the base image could have nodejs and npm installed. could we check if npm exists before installing node? if npm already exists, we can use the existing installation to install the bids validator.

just to understand, why would you want to check this?

to avoid wasting build time or to avoid conflicting node / npm versions?

AFAIK the typical base image we use ubuntu, debian, fefora do not have node installed, right?

kaczmarj commented 11 months ago

It’s possible that a user could provide a different base image, for example one that already has nodejs in it. Or perhaps a user could have a custom RUN instruction that installs nodejs. We wouldn’t want to overwrite or reinstall nodejs and npm in those cases. The runtime is less of a concern imho — I would be more concerned about potentially overwriting an existing nodejs and causing unexpexted behavior.On Nov 10, 2023, at 11:42, Remi Gau @.***> wrote:

this is a great addition! one suggestion is that the base image could have nodejs and npm installed. could we check if npm exists before installing node? if npm already exists, we can use the existing installation to install the bids validator.

just to understand, why would you want to check this? to avoid wasting build time or to avoid conflicting node / npm versions? AFAIK the typical base image we use ubuntu, debian, fefora do not have node installed, right?

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because your review was requested.Message ID: @.***>

Remi-Gau commented 11 months ago

this is a great addition! one suggestion is that the base image could have nodejs and npm installed. could we check if npm exists before installing node? if npm already exists, we can use the existing installation to install the bids validator.

Done

kaczmarj commented 11 months ago

thanks @Remi-Gau :) merge at will!