[pacman-dev] [PATCH 1/5] makepkg: warn about dotfiles in package root
libalpm reserves paths starting with '.' for its own use and will not
extract any other than those it recognizes.
Signed-off-by: Andrew Gregory
By passing a NUL-separated filelist, this also fixes a bug where files
that look like bsdtar options in the package root could break the
package ("-C" was particularly troublesome because bsdtar interprets it
as an option anywhere in the file list, even following "--").
Signed-off-by: Andrew Gregory
On 10/12/16 02:14, Andrew Gregory wrote:
By passing a NUL-separated filelist, this also fixes a bug where files that look like bsdtar options in the package root could break the package ("-C" was particularly troublesome because bsdtar interprets it as an option anywhere in the file list, even following "--").
Signed-off-by: Andrew Gregory
--- scripts/makepkg.sh.in | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 2000451..3700307 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -686,6 +686,15 @@ write_buildinfo() { write_kv_pair "installed" "${pkglist[@]}" }
+# build a sorted NUL-separated list of the full contents of the current +# directory suitable for passing to `bsdtar --files-from` +# database files are placed at the beginning of the package regardless of +# sorting +list_package_files() { + (find . -path './.*' \! -name '.'; find . \! -path './.*' \! -name '.' | LANG=C sort) | + sed -e 's|^\./||' | tr '\n' '\0' +}
I think we would need LC_ALL or at least LC_COLLATE here. If either is set, LANG will not overwrite them. (I also tried to make this a multi-line function rather than a single line monstrosity but failed printing an array with null delimiters...)
+ create_package() { (( NOARCHIVE )) && return
@@ -702,8 +711,6 @@ create_package() { write_pkginfo > .PKGINFO write_buildinfo > .BUILDINFO
- local comp_files=('.PKGINFO' '.BUILDINFO') - # check for changelog/install files for i in 'changelog/.CHANGELOG' 'install/.INSTALL'; do IFS='/' read -r orig dest < <(printf '%s\n' "$i") @@ -715,7 +722,6 @@ create_package() { exit 1 fi chmod 644 "$dest" - comp_files+=("$dest") fi done
@@ -727,15 +733,10 @@ create_package() { [[ -f $pkg_file ]] && rm -f "$pkg_file" [[ -f $pkg_file.sig ]] && rm -f "$pkg_file.sig"
- # when fileglobbing, we want * in an empty directory to expand to - # the null string rather than itself - shopt -s nullglob - msg2 "$(gettext "Generating .MTREE file...")" - LANG=C bsdtar -czf .MTREE --format=mtree \ + list_package_files | LANG=C bsdtar -cnzf .MTREE --format=mtree \ --options='!all,use-set,type,uid,gid,mode,time,size,md5,sha256,link' \ - "${comp_files[@]}" * - comp_files+=(".MTREE") + --null --files-from - --exclude .MTREE
Why do you need --exclude .MTREE here?
msg2 "$(gettext "Compressing package...")" # TODO: Maybe this can be set globally for robustness @@ -743,7 +744,7 @@ create_package() { # bsdtar's gzip compression always saves the time stamp, making one # archive created using the same command line distinct from another. # Disable bsdtar compression and use gzip -n for now. - LANG=C bsdtar -cf - "${comp_files[@]}" * | + list_package_files | LANG=C bsdtar -cnf - --null --files-from - | case "$PKGEXT" in *tar.gz) ${COMPRESSGZ[@]:-gzip -c -f -n} ;; *tar.bz2) ${COMPRESSBZ2[@]:-bzip2 -c -f} ;; @@ -756,7 +757,6 @@ create_package() { "$PKGEXT"; cat ;; esac > "${pkg_file}" || ret=$?
- shopt -u nullglob shopt -u -o pipefail
if (( ret )); then
On 01/03/17 at 03:09pm, Allan McRae wrote:
On 10/12/16 02:14, Andrew Gregory wrote:
By passing a NUL-separated filelist, this also fixes a bug where files that look like bsdtar options in the package root could break the package ("-C" was particularly troublesome because bsdtar interprets it as an option anywhere in the file list, even following "--").
Signed-off-by: Andrew Gregory
--- scripts/makepkg.sh.in | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 2000451..3700307 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -686,6 +686,15 @@ write_buildinfo() { write_kv_pair "installed" "${pkglist[@]}" }
+# build a sorted NUL-separated list of the full contents of the current +# directory suitable for passing to `bsdtar --files-from` +# database files are placed at the beginning of the package regardless of +# sorting +list_package_files() { + (find . -path './.*' \! -name '.'; find . \! -path './.*' \! -name '.' | LANG=C sort) | + sed -e 's|^\./||' | tr '\n' '\0' +}
I think we would need LC_ALL or at least LC_COLLATE here. If either is set, LANG will not overwrite them.
Agreed.
(I also tried to make this a multi-line function rather than a single line monstrosity but failed printing an array with null delimiters...)
Yes, it's ugly, but it's POSIX-compliant!
+ create_package() { (( NOARCHIVE )) && return
@@ -702,8 +711,6 @@ create_package() { write_pkginfo > .PKGINFO write_buildinfo > .BUILDINFO
- local comp_files=('.PKGINFO' '.BUILDINFO') - # check for changelog/install files for i in 'changelog/.CHANGELOG' 'install/.INSTALL'; do IFS='/' read -r orig dest < <(printf '%s\n' "$i") @@ -715,7 +722,6 @@ create_package() { exit 1 fi chmod 644 "$dest" - comp_files+=("$dest") fi done
@@ -727,15 +733,10 @@ create_package() { [[ -f $pkg_file ]] && rm -f "$pkg_file" [[ -f $pkg_file.sig ]] && rm -f "$pkg_file.sig"
- # when fileglobbing, we want * in an empty directory to expand to - # the null string rather than itself - shopt -s nullglob - msg2 "$(gettext "Generating .MTREE file...")" - LANG=C bsdtar -czf .MTREE --format=mtree \ + list_package_files | LANG=C bsdtar -cnzf .MTREE --format=mtree \ --options='!all,use-set,type,uid,gid,mode,time,size,md5,sha256,link' \ - "${comp_files[@]}" * - comp_files+=(".MTREE") + --null --files-from - --exclude .MTREE
Why do you need --exclude .MTREE here?
When used with --repackage, the .MTREE file will already exist. Attempting to add the .MTREE file to itself upsets bsdtar. apg
By passing a NUL-separated filelist, this also fixes a bug where files
that look like bsdtar options in the package root could break the
package ("-C" was particularly troublesome because bsdtar interprets it
as an option anywhere in the file list, even following "--").
Signed-off-by: Andrew Gregory
Signed-off-by: Andrew Gregory
Signed-off-by: Andrew Gregory
Signed-off-by: Andrew Gregory
On 10/12/16 02:14, Andrew Gregory wrote:
Signed-off-by: Andrew Gregory
--- This patch obviously won't do any good until packages start being built with their file lists presorted, so we may want to defer it, but in the handful of packages I tested, it did not cause a significant delay either.
Would it be best to just recommend that repo databases are regenerated with the updated repo-add? Not including this would result in the qsort operations on the pre-sorted file lists having the worst possible performance. A
On 01/03/17 at 02:20pm, Allan McRae wrote:
On 10/12/16 02:14, Andrew Gregory wrote:
Signed-off-by: Andrew Gregory
--- This patch obviously won't do any good until packages start being built with their file lists presorted, so we may want to defer it, but in the handful of packages I tested, it did not cause a significant delay either.
Would it be best to just recommend that repo databases are regenerated with the updated repo-add? Not including this would result in the qsort operations on the pre-sorted file lists having the worst possible performance.
I wouldn't expect pre-sorting to degrade performance for modern qsort implementations (it certainly didn't for me), but the performance hit for unsorted dbs/packages is probably modest enough that it's better to just go ahead and merge this. apg
On 10/12/16 02:14, Andrew Gregory wrote:
libalpm reserves paths starting with '.' for its own use and will not extract any other than those it recognizes.
Signed-off-by: Andrew Gregory
OK.
participants (2)
-
Allan McRae
-
Andrew Gregory