| Message ID | 20171030180329.24189-2-eschwartz@archlinux.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [pacman-dev,v2,1/2] makepkg: reorganize the restoration of settings by precedence | expand |
On 31/10/17 04:03, Eli Schwartz wrote: > Additionally provide a separate error for the confusing if unlikely > event that the user tries to use an existing file as a package output > directory. > > This also means we now consistently try to create any nonexistent *DEST > directories as needed before aborting with E_FS_PERMISSIONS. Previously > only $BUILDDIR received that kindness. > > Fixes FS#43537 > > Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> > --- > > mkdir -p has a really bad error when a file exists that conflicts with > the path to be created. But I'm still not sure whether to include that > code in ensure_writable_dir > > scripts/libmakepkg/util/util.sh.in | 19 +++++++++++++++++++ > scripts/makepkg.sh.in | 19 ++++++------------- > 2 files changed, 25 insertions(+), 13 deletions(-) > > diff --git a/scripts/libmakepkg/util/util.sh.in b/scripts/libmakepkg/util/util.sh.in > index d676249d..8a6149ed 100644 > --- a/scripts/libmakepkg/util/util.sh.in > +++ b/scripts/libmakepkg/util/util.sh.in > @@ -83,3 +83,22 @@ cd_safe() { > exit 1 > fi > } > + > +# Try to create directory if one does not yet exist. Fails if the directory > +# exists but has no write permissions, or if there is an existing file with > +# the same name. > +ensure_writable_dir() { > + local parent=$1 dirname=$1 > + > + until [[ -e $parent ]]; do > + parent="${parent%/*}" > + done > + if [[ -f $parent ]]; then > + error "$(gettext "Cannot create %s due to conflicting file.")" "$parent" > + return 1 > + fi > + if ( [[ ! -d $dirname ]] && ! mkdir -p "$dirname" ) || [[ ! -w $dirname ]]; then > + return 1 > + fi > + return 0 > +} > diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in > index 20cc22ad..35665f16 100644 > --- a/scripts/makepkg.sh.in > +++ b/scripts/makepkg.sh.in > @@ -1360,34 +1360,27 @@ else > fi > > > -if [[ ! -d $BUILDDIR ]]; then > - if ! mkdir -p "$BUILDDIR"; then > - error "$(gettext "You do not have write permission to create packages in %s.")" "$BUILDDIR" > - plain "$(gettext "Aborting...")" > - exit 1 > - fi > - chmod a-s "$BUILDDIR" As discussed on IRC, this chmod is important. If the build dir gets created with a sticky bit, that propagates through the package. A
diff --git a/scripts/libmakepkg/util/util.sh.in b/scripts/libmakepkg/util/util.sh.in index d676249d..8a6149ed 100644 --- a/scripts/libmakepkg/util/util.sh.in +++ b/scripts/libmakepkg/util/util.sh.in @@ -83,3 +83,22 @@ cd_safe() { exit 1 fi } + +# Try to create directory if one does not yet exist. Fails if the directory +# exists but has no write permissions, or if there is an existing file with +# the same name. +ensure_writable_dir() { + local parent=$1 dirname=$1 + + until [[ -e $parent ]]; do + parent="${parent%/*}" + done + if [[ -f $parent ]]; then + error "$(gettext "Cannot create %s due to conflicting file.")" "$parent" + return 1 + fi + if ( [[ ! -d $dirname ]] && ! mkdir -p "$dirname" ) || [[ ! -w $dirname ]]; then + return 1 + fi + return 0 +} diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 20cc22ad..35665f16 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1360,34 +1360,27 @@ else fi -if [[ ! -d $BUILDDIR ]]; then - if ! mkdir -p "$BUILDDIR"; then - error "$(gettext "You do not have write permission to create packages in %s.")" "$BUILDDIR" - plain "$(gettext "Aborting...")" - exit 1 - fi - chmod a-s "$BUILDDIR" -fi -if [[ ! -w $BUILDDIR ]]; then +# check that all settings directories are user-writable +if ! ensure_writable_dir "$BUILDDIR"; then error "$(gettext "You do not have write permission to create packages in %s.")" "$BUILDDIR" plain "$(gettext "Aborting...")" exit 1 fi -if (( ! (NOBUILD || GENINTEG) )) && [[ ! -w $PKGDEST ]]; then +if (( ! (NOBUILD || GENINTEG) )) && ! ensure_writable_dir "$PKGDEST"; then error "$(gettext "You do not have write permission to store packages in %s.")" "$PKGDEST" plain "$(gettext "Aborting...")" exit 1 fi -if [[ ! -w $SRCDEST ]] ; then +if ! ensure_writable_dir "$SRCDEST" ; then error "$(gettext "You do not have write permission to store downloads in %s.")" "$SRCDEST" plain "$(gettext "Aborting...")" exit 1 fi if (( SOURCEONLY )); then - if [[ ! -w $SRCPKGDEST ]]; then + if ! ensure_writable_dir "$SRCPKGDEST"; then error "$(gettext "You do not have write permission to store source tarballs in %s.")" "$SRCPKGDEST" plain "$(gettext "Aborting...")" exit 1 @@ -1398,7 +1391,7 @@ if (( SOURCEONLY )); then IGNOREARCH=1 fi -if (( LOGGING )) && [[ ! -w $LOGDEST ]]; then +if (( LOGGING )) && ! ensure_writable_dir "$LOGDEST"; then error "$(gettext "You do not have write permission to store logs in %s.")" "$LOGDEST" plain "$(gettext "Aborting...")" exit 1
Additionally provide a separate error for the confusing if unlikely event that the user tries to use an existing file as a package output directory. This also means we now consistently try to create any nonexistent *DEST directories as needed before aborting with E_FS_PERMISSIONS. Previously only $BUILDDIR received that kindness. Fixes FS#43537 Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> --- mkdir -p has a really bad error when a file exists that conflicts with the path to be created. But I'm still not sure whether to include that code in ensure_writable_dir scripts/libmakepkg/util/util.sh.in | 19 +++++++++++++++++++ scripts/makepkg.sh.in | 19 ++++++------------- 2 files changed, 25 insertions(+), 13 deletions(-)