guysoft / CustomPiOS

A Raspberry Pi and other ARM devices distribution builder
GNU General Public License v3.0
514 stars 149 forks source link

Unpack does chmod on entire target dir #63

Closed EternityForest closed 4 years ago

EternityForest commented 5 years ago

When I unpack directly into a folder that has existing content, that content also gets chmoded, and I believe that process may also mess with the setuid bit. Is this intended behavior?

cp -v -r --preserve=mode,ownership,timestamps $from/. $to
  if [ -n "$owner" ]
  then
    chown -hR $owner:$owner $to
  fi

Seems to be the issue.

I am trying this untested workaround, but I do not like it because of the extra disk activity:

function unpack() {
  # call like this: unpack /path/to/source /target user -- this will copy
  # all files & folders from source to target, preserving mode and timestamps
  # and chown to user. If user is not provided, no chown will be performed

  from=$1
  to=$2
  owner=
  if [ "$#" -gt 2 ]
  then
    owner=$3
  fi
  mkdir -p /tmp/unpack/
  # $from/. may look funny, but does exactly what we want, copy _contents_
  # from $from to $to, but not $from itself, without the need to glob -- see 
  # http://stackoverflow.com/a/4645159/2028598
  cp -v -r --preserve=mode,timestamps $from/. /tmp/unpack/

  if [ -n "$owner" ]
  then
    chown -hR $owner:$owner $to
  fi

  cp -v -r --preserve=mode,ownership,timestamps /tmp/unpack/. $to
  rm -r /tmp/unpack

}
guysoft commented 5 years ago

Yes, seems to be an issue. You might be able to save most of the disk activity if you use mv. But I dont know if mv preserves timestamps mode and ownership. It might. Unlike cp mv just changes the innode so it should not be a big addition as cp is.

I would love a PR if you have the time for that.

EternityForest commented 5 years ago

This is slightly out of my depth (I haven't used Bash as an actual programming language in a long time) but there seems to be a glob workaround in a stack overflow post

mv -- .[!.] ..?* /dest/

That handles dotfiles semi-correctly without including . and ..

Perhaps three separate commands for normal, dot, and dotdot files?

Dear acts differently with and without trailing slash, I think we want an error if it doesn't exist and not to clobber seat if it's actually a file, so we'd need a way to normalize to one single trailing slash too?

On Thu, Nov 7, 2019, 3:07 AM Guy Sheffer notifications@github.com wrote:

Yes, seems to be an issue. You might be able to save most of the disk activity if you use mv. But I dont know if mv preserves timestamps mode and ownership. It might. Unlike cp mv just changes the innode so it should not be a big addition as cp is.

I would love a PR if you have the time for that.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/guysoft/CustomPiOS/issues/63?email_source=notifications&email_token=AAFZCH57O3LRCQQ4LFIF3H3QSPZF5A5CNFSM4JKDILCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDMBT6Q#issuecomment-551033338, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFZCH7S6APPVVHR5P5SS5TQSPZF5ANCNFSM4JKDILCA .

guysoft commented 4 years ago

Fixed with #65