search

furrer-lab / r-containers

GNU General Public License v3.0
1 stars 0 forks source link

33 remove abn from the installed packages #39

Closed matteodelucchi closed 1 week ago

matteodelucchi commented 3 weeks ago

Don't install 'abn' in the container, only its dependencies (#33 ). This is redundant since the package will be installed in the workflow that applies this container to ensure that the package version of interest is tested.

matteodelucchi commented 3 weeks ago

@j-i-l Do we need the commented line 12 in the Dockerfile(s), which would receive the package name as an argument? I.e., would it be cleaner to provide the package name by the workflow file?

j-i-l commented 2 weeks ago

@j-i-l Do we need the commented line 12 in the Dockerfile(s), which would receive the package name as an argument? I.e., would it be cleaner to provide the package name by the workflow file?

Hm, receiving the package name through the workflow file was how we did this initially. However, I was not really happy with this, especially because we parse the DESCRIPTION file anyways and in there is the relevant package name.

With https://github.com/furrer-lab/r-containers/commit/c87278279ded5c54525833b3a6b3e764ac463191 we actually switched to this approach. For some reason I don't remember undid this then in https://github.com/furrer-lab/r-containers/commit/0fa19a03b40829a0f27830159d8cd808620fff5f . I guess I thought that getting the actual name of the package was not really needed and simply dropped it.

Instead of https://github.com/furrer-lab/r-containers/pull/39/files#diff-eeb2c54ffbd45ce9141bce4bcfc750792782e4e40903b0ec4465cc7a63b2e10bR86 I would probably revert to https://github.com/furrer-lab/r-containers/commit/c87278279ded5c54525833b3a6b3e764ac463191#diff-f446c50d6295c9c66ad8bc9b36d06183089895205c59ede0779fd3dda1f8063fR91 which takes the package name from the DESCRIPTION file.

j-i-l commented 1 week ago

Looks OK, I think.

I'm used to define env variables in a docker with

ENV PACKAGE_NAME=... but I do not see why running a classical declaration (i.e. RUN PACKAGE_NAME=...) should not work as well.

https://github.com/furrer-lab/r-containers/actions/runs/9663922149/job/26657338485#step:6:36217

it wasn't OK it seems :see_no_evil:

We should add a dev pipeline that builds the containers on pull requests... I'll add this!