Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unnecessary post-scripts in RPM packages of MRO #20

Closed
iavael opened this issue Nov 27, 2016 · 4 comments
Closed

Unnecessary post-scripts in RPM packages of MRO #20

iavael opened this issue Nov 27, 2016 · 4 comments
Milestone

Comments

@iavael
Copy link

iavael commented Nov 27, 2016

In microsoft-r-open-mro-3.3 and microsoft-r-open-mkl-3.3 rpm-s there're unnecessary scripts in pre/post sections, see comments

microsoft-r-open-mro-3.3:

postinstall scriptlet (using /bin/sh):

#!/bin/bash
VERSION="3.3"
INSTALL_PREFIX="${RPM_INSTALL_PREFIX}/microsoft-r/${VERSION}"

mkdir -p ${INSTALL_PREFIX}/lib64/R/backup/lib # Should be done during rpm build, not during install
mv ${INSTALL_PREFIX}/lib64/R/lib/*.so ${INSTALL_PREFIX}/lib64/R/backup/lib # Same here
cp ${INSTALL_PREFIX}/lib64/R/backup/lib/libR.so ${INSTALL_PREFIX}/lib64/R/lib # Same here
cp ${INSTALL_PREFIX}/stage/mkl_install_stage/*.so ${INSTALL_PREFIX}/lib64/R/lib # Same here
preuninstall scriptlet (using /bin/sh):
#!/bin/bash
VERSION="3.3"
DIRECTORY="${RPM_INSTALL_PREFIX}/microsoft-r/${VERSION}/lib64/R"
if [ -d "$DIRECTORY" ]; then
  rm ${DIRECTORY}/lib/*.so # Same here
  mv ${DIRECTORY}/backup/lib/*.so ${DIRECTORY}/lib # Same here
fi

So whole pre and post sections are unnecessary, beacuse everything they do could (moreover should!) be done during build. One mistake in "rm" statement and you destroy user's system like in MrMEEE/bumblebee-Old-and-abbandoned#123

postinstall scriptlet (using /bin/sh):

#!/bin/bash

VERSION="3.3"
INSTALL_PREFIX="${RPM_INSTALL_PREFIX}/microsoft-r/${VERSION}"

ln -s "${INSTALL_PREFIX}/lib64/R/bin/R" /usr/bin/R # Should be done through "alternatives" mechanism
ln -s "${INSTALL_PREFIX}/lib64/R/bin/Rscript" /usr/bin/Rscript # same here

rm /bin/sh # WHAT!!! This is insane! Never touch files of other packages please
ln -s /bin/bash /bin/sh
preuninstall scriptlet (using /bin/sh):
#!/bin/bash

VERSION=3.3
INSTALL_PREFIX="${RPM_INSTALL_PREFIX}/microsoft-r/${VERSION}"

rm /usr/bin/R # See comment about alternatives
rm /usr/bin/Rscript # same here

rm -rf "${INSTALL_PREFIX}/lib64/R/backup" # Should be done during rpm build, not during install
postuninstall scriptlet (using /bin/sh):
#!/bin/bash

# remove if empty, leave if not empty
rmdir "${RPM_INSTALL_PREFIX}/microsoft-r" 2>/dev/null # RPM will do this for you automatically

These pre/post sections can be reduced to couple of update-alternatives calls.
And playing with /bin/sh risking of cripple user's system is wrong from any point of view, please never do this.

@nathansoz nathansoz added this to the MRO 3.3.3 milestone Nov 30, 2016
@nathansoz
Copy link
Contributor

We will look at these comments and make changes. Particularly:

-Adding update-alternatives support instead of symlinking ourselves in
-The /bin/sh stuff is due to Ubuntu symlinking dash to /bin/sh and that reacting badly with both the R shell wrapper and some Microsoft R Server features. I believe we fixed the R shell wrapper features last release by explicitly requesting bash and we will be fixing the R server features next release. 3.3.3 should be updated with these fixes.

@iavael
Copy link
Author

iavael commented Nov 30, 2016

Adding update-alternatives support instead of symlinking ourselves in

I investigated a bit more. If there's already R package in distro and it doesn't support update-alternatives mechanism (in Fedora/CentOS/RHEL it doesn't), then alternatives won't help anyway, but direct manipulation with /usr/bin/R and /usr/bin/Rscript will break distro packages if they are installed. It's better to pick one of variants:

  • obsolete and replace R-core package for rhel-based distros in your rpm metadata like
Replaces: R-core
Obsoletes: R-core
Provides: R-core
  • don't touch /usr/bin/R/Rscript at all and avoid conflict by symlinking your binaries to path like /usr/bin/MR and /usr/bin/MRscript (but this can break integration with other tools that just look for "R" and "Rscript" in $PATH and have no params to override this)

The /bin/sh stuff is due to Ubuntu symlinking dash to /bin/sh...

Well, in RPM-based systems (at least in RHEL-based like RHEL itself, CentOS and Fedora, I don't remember about SuSE-based) /bin/sh is already symlink to /bin/bash, so you can avoid replacing in in MRO's rpm packages. For debian-based systems there is dpkg-divert mechanism [1][2] which can help you replace /bin/sh without conflicting distro's settings. Fixing R shell wrapper looks as better option but maybe you can use provided info for fastfix.

[1] https://www.debian.org/doc/debian-policy/ap-pkg-diversions.html
[2] https://wiki.debian.org/Adding%20and%20removing%20diversions

@voronoipotato
Copy link

https://lobste.rs/s/btycfp/microsoft_s_failed_attempt_at_debian

This issue was reffered to on Lobsters which was discussing this article https://www.preining.info/blog/2018/06/microsofts-failed-attempt-on-debian-packaging/ . Perhaps you could reach out to someone for help on this.

@richcalaway
Copy link

We have re-released MRO 3.5.0 with new mro postinst scripts that use the alternatives approach; most of the other changes had been made for MRO 3.3.3.

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

No branches or pull requests

4 participants