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

$SHARE_DIR cleanup on install??? #12

Closed
tpruzina opened this issue Dec 13, 2017 · 4 comments
Closed

$SHARE_DIR cleanup on install??? #12

tpruzina opened this issue Dec 13, 2017 · 4 comments

Comments

@tpruzina
Copy link

tpruzina commented Dec 13, 2017

While not exactly recommended by README, If I ever wanted to install into /usr or /usr/local prefix your retdec/cmake/install-share.sh would rm -rf my $SHARE_DIR.

Please fix before somebody tries to install into their /usr.

tomas@tpruzina build # sandbox.py -p make-install make install
...
-- Install configuration: "Release"
rm: cannot remove '/usr/local/share/doc/vlc/libvlc/QtPlayer': Read-only file system
...
rm: cannot remove '/usr/local/share/wine': Read-only file system

RUN: wget https://github.com/avast-tl/retdec-support/releases/download/2017-12-12/retdec-support_2017-12-12.tar.xz -O /usr/local/retdec-support_2017-12-12.tar.xz
/usr/local/retdec-support_2017-12-12.tar.xz: Read-only file system
ERROR: wget failed
rm: cannot remove '/usr/local/share/doc/vlc/libvlc/QtPlayer': Read-only file system
...
rm: cannot remove '/usr/local/share/doc/vlc/lua/http/requests': Read-only file system
rm: cannot remove '/usr/local/share/wine': Read-only file system

CMake Error at cmake_install.cmake:47 (message):
RetDec share directory installation FAILED

@tpruzina tpruzina changed the title destdir cleanup??? $SHARE_DIR cleanup on istall??? Dec 13, 2017
@PeterMatula
Copy link
Collaborator

Well, I must say that after a long time playing with it only in our set environments, we were kinda expecting something like this when released. But rm-rf-ing parts of /usr is definitely AN issue.

We will not be able to fix it today, but I at least added a warning in c1206a6, and solving this will be the first task tomorrow.

@s3rvac s3rvac changed the title $SHARE_DIR cleanup on istall??? $SHARE_DIR cleanup on install??? Dec 13, 2017
@s3rvac s3rvac added the bug label Dec 13, 2017
@mewmew
Copy link

mewmew commented Dec 13, 2017

I ran into this yesterday. Luckily, I tried to install as a regular user and did not run with sudo, so rm -rf did not remove anything.

s3rvac added a commit that referenced this issue Dec 13, 2017
…ctory (#12).

Remove only the subdirectories and files that are used by RetDec. This should
reduce the chance of unintentionally removing system directories when RetDec is
accidentally installed on Linux into e.g. `/usr` as the `arm`, `generic`, and
`x86` directories are not commonly used by Linux distributions.

Nevertheless, as mentioned in our README, until our build system properly
supports system-wide installations, RetDec should to be installed into a clean,
dedicated directory.
@s3rvac
Copy link
Member

s3rvac commented Dec 13, 2017

Thank you very much for your report! In 03ea29a, I have modified the script to not remove the entire share directory. Instead, it will now remove only subdirectories and files that are used by RetDec. This should reduce the chance of unintentionally removing system directories when RetDec is accidentally installed on Linux into e.g. /usr as the arm, generic, and x86 directories are not commonly used by Linux distributions.

Nevertheless, until our build system properly supports system-wide installation, RetDec should be installed into a clean, dedicated directory.

PeterMatula added a commit that referenced this issue Dec 15, 2017
this should solve #12. script no longer clears (rm -rf)
<install_dir>/share, it clears <install_dir>/share/retdec/support.
@PeterMatula
Copy link
Collaborator

After fb9b340 and #31, installation creates:

  • bin/
  • lib/libretdec-{libelf,libdwarf}.so (Linux)
  • share/retdec/doc
  • share/retdec/support

Only share/retdec/support is rm-rf-ed in the installation step. I looked for more potentially dangerous rm commands, but found none, so lets hope there are no more surprises like this.

After this, we are one step closer to a system-wide installation. However, I still would not recommend it -- there are to many binaries, scripts, and stuff in bin. Closing this, potential system-wide installation may be solved later in some dedicated issue.

HugoKlepsch pushed a commit to HugoKlepsch/retdec that referenced this issue Dec 20, 2017
…ctory (avast#12).

Remove only the subdirectories and files that are used by RetDec. This should
reduce the chance of unintentionally removing system directories when RetDec is
accidentally installed on Linux into e.g. `/usr` as the `arm`, `generic`, and
`x86` directories are not commonly used by Linux distributions.

Nevertheless, as mentioned in our README, until our build system properly
supports system-wide installations, RetDec should to be installed into a clean,
dedicated directory.
HugoKlepsch pushed a commit to HugoKlepsch/retdec that referenced this issue Dec 20, 2017
this should solve avast#12. script no longer clears (rm -rf)
<install_dir>/share, it clears <install_dir>/share/retdec/support.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants