Bug#860238: [dpkg] Improve debugging experience for piupart
On Thu, Apr 20, 2017 at 10:00 AM, Guillem Jover wrote:
> Control: forcemerge 813454 -1
>
> Hi!
>
> On Thu, 2017-04-13 at 12:02:07 +0200, Bastien ROUCARIÈS wrote:
>> Package: dpkg
>> Version: 1.18.23
>> Severity: important
>> affects: piuparts.debian.org
>> affects: src:imagemagick
>> control: tags -1 + patch
>
>> I know it is late on release but it will really help to add this patch.
>>
>> Could you consider for release ?
>
> When you first filed this, I took a quick look, but producing a proper
> fix seemed involved, given the contstrains I set for myself:
>
> Thanks for the patch, although I think this is not enough.
>
>> diff --git a/scripts/dpkg-maintscript-helper.sh
>> b/scripts/dpkg-maintscript-helper.sh
>> index b4b3ac1b3..0b867d805 100755
>> --- a/scripts/dpkg-maintscript-helper.sh
>> +++ b/scripts/dpkg-maintscript-helper.sh
>> @@ -412,14 +412,8 @@ prepare_dir_to_symlink()
>>
>> # If there are locally created files or files owned by another package
>> # we should not perform the switch.
>> - find "$PATHNAME" -print0 | xargs -0 -n1 sh -c '
>> - package="$1"
>> - file="$2"
>> - if ! dpkg-query -L "$package" | grep -F -q -x "$file"; then
>> - exit 1
>> - fi
>> - exit 0
>> - ' check-files-ownership "$PACKAGE" || \
>> + find "$PATHNAME" -print0 | xargs -0 -n1 \
>> + dpkg-maintscript-helper package_owns_file_or_error $PACKAGE ||
>> \
>
> The problem with this is that it aborts on first error, so any other
> remaining problematic files are missed, which might make debugging
> difficult, miss files, or require several iterations.
Thanks for this remark but it I think you are wrong: xargs will
execute next command if error code is < 125 (and it is POSIX and
portable).
Try for instance
echo '/ /tmp' | xargs -n1 chmod +x
So this implementation will print the name of all not owned files.
>
> I also considered using a command like this, but it seemed wrong, as
> that is exposing a private implementation detail as part of the
> interface, so I'd rather not do this.
>
>> error "directory '$PATHNAME' contains files not owned by" \
>> "package $PACKAGE, cannot switch to symlink"
>>
>> @@ -515,6 +509,18 @@ ensure_package_owns_file() {
>> return 0
>> }
Does adding a private suffix and passing a magic number as arg1 will
solve this ?
I means:
MAGICK_PRIVATE="This is private interface do not use"
package_owns_file_or_error() {
local MAGICK="$1"
local PACKAGE="$2"
local FILE="$3"
if test x$MAGICKL != x$MAGICK_PRIVATE; then
error "$MAGICK_PRIVATE";
fi
if ! ensure_package_owns_file $PACKAGE $FILE ; then
error "File '$FILE' not owned by package " \
"'$PACKAGE'"
return 1
> error() already «exits 1». :)
Ok
>> + fi
>> + return 0
}
>> +
>> +package_owns_file_or_error() {
>> + local PACKAGE="$1"
>> + local FILE="$2"
>> + if ! ensure_package_owns_file $PACKAGE $FILE ; then
>> +error "File '$FILE' not owned by package " \
>> + "'$PACKAGE'"
>> +return 1
>
> error() already «exits 1». :)
Yes but for clarification it is better.
>> + fi
>> + return 0
>> +}
Thanks for the review
Bastien
> Thanks,
> Guillem
Processed: Re: Bug#860238: [dpkg] Improve debugging experience for piupart
Processing control commands: > forcemerge 813454 -1 Bug #813454 [dpkg] [dpkg] dpkg-maintscript-helper should list dir_to-symlink file not owned by $PACKAGE Bug #813454 [dpkg] [dpkg] dpkg-maintscript-helper should list dir_to-symlink file not owned by $PACKAGE Marked as found in versions dpkg/1.18.23. Added tag(s) patch. Bug #860238 [dpkg] [dpkg] Improve debugging experience for piuparts Severity set to 'wishlist' from 'important' 860237 was blocked by: 860238 860237 was not blocking any bugs. Removed blocking bug(s) of 860237: 860238 Removed indication that 860238 affects piuparts Marked as found in versions dpkg/1.18.4. Merged 813454 860238 -- 813454: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=813454 860237: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=860237 860238: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=860238 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems
Bug#860238: [dpkg] Improve debugging experience for piupart
Control: forcemerge 813454 -1
Hi!
On Thu, 2017-04-13 at 12:02:07 +0200, Bastien ROUCARIÈS wrote:
> Package: dpkg
> Version: 1.18.23
> Severity: important
> affects: piuparts.debian.org
> affects: src:imagemagick
> control: tags -1 + patch
> I know it is late on release but it will really help to add this patch.
>
> Could you consider for release ?
When you first filed this, I took a quick look, but producing a proper
fix seemed involved, given the contstrains I set for myself:
Thanks for the patch, although I think this is not enough.
> diff --git a/scripts/dpkg-maintscript-helper.sh
> b/scripts/dpkg-maintscript-helper.sh
> index b4b3ac1b3..0b867d805 100755
> --- a/scripts/dpkg-maintscript-helper.sh
> +++ b/scripts/dpkg-maintscript-helper.sh
> @@ -412,14 +412,8 @@ prepare_dir_to_symlink()
>
> # If there are locally created files or files owned by another package
> # we should not perform the switch.
> - find "$PATHNAME" -print0 | xargs -0 -n1 sh -c '
> - package="$1"
> - file="$2"
> - if ! dpkg-query -L "$package" | grep -F -q -x "$file"; then
> - exit 1
> - fi
> - exit 0
> - ' check-files-ownership "$PACKAGE" || \
> + find "$PATHNAME" -print0 | xargs -0 -n1 \
> + dpkg-maintscript-helper package_owns_file_or_error $PACKAGE || \
The problem with this is that it aborts on first error, so any other
remaining problematic files are missed, which might make debugging
difficult, miss files, or require several iterations.
I also considered using a command like this, but it seemed wrong, as
that is exposing a private implementation detail as part of the
interface, so I'd rather not do this.
> error "directory '$PATHNAME' contains files not owned by" \
> "package $PACKAGE, cannot switch to symlink"
>
> @@ -515,6 +509,18 @@ ensure_package_owns_file() {
> return 0
> }
>
> +
> +package_owns_file_or_error() {
> + local PACKAGE="$1"
> + local FILE="$2"
> + if ! ensure_package_owns_file $PACKAGE $FILE ; then
> +error "File '$FILE' not owned by package " \
> + "'$PACKAGE'"
> +return 1
error() already «exits 1». :)
> + fi
> + return 0
> +}
Thanks,
Guillem
Processed: Re: Bug#860238: [dpkg] Improve debugging experience for piupart
Processing commands for cont...@bugs.debian.org: > affects 860238 piuparts Bug #860238 [dpkg] [dpkg] Improve debugging experience for piuparts Added indication that 860238 affects piuparts > thanks Stopping processing here. Please contact me if you need assistance. -- 860238: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=860238 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems
Bug#860238: [dpkg] Improve debugging experience for piupart
Package: dpkg
Version: 1.18.23
Severity: important
affects: piuparts.debian.org
affects: src:imagemagick
control: tags -1 + patch
Hi guillem,
I know it is late on release but it will really help to add this patch.
Could you consider for release ?
Thank you
Bastien
commit 14814cd71ca2fac0857c51615dfcb0f6fd13655b
Author: Bastien ROUCARIÈS
Date: Wed Mar 15 12:10:07 2017 +0100
Factorize owns_file
diff --git a/scripts/dpkg-maintscript-helper.sh b/scripts/dpkg-maintscript-helper.sh
index b4b3ac1b3..0b867d805 100755
--- a/scripts/dpkg-maintscript-helper.sh
+++ b/scripts/dpkg-maintscript-helper.sh
@@ -412,14 +412,8 @@ prepare_dir_to_symlink()
# If there are locally created files or files owned by another package
# we should not perform the switch.
- find "$PATHNAME" -print0 | xargs -0 -n1 sh -c '
- package="$1"
- file="$2"
- if ! dpkg-query -L "$package" | grep -F -q -x "$file"; then
- exit 1
- fi
- exit 0
- ' check-files-ownership "$PACKAGE" || \
+ find "$PATHNAME" -print0 | xargs -0 -n1 \
+ dpkg-maintscript-helper package_owns_file_or_error $PACKAGE || \
error "directory '$PATHNAME' contains files not owned by" \
"package $PACKAGE, cannot switch to symlink"
@@ -515,6 +509,18 @@ ensure_package_owns_file() {
return 0
}
+
+package_owns_file_or_error() {
+ local PACKAGE="$1"
+ local FILE="$2"
+ if ! ensure_package_owns_file $PACKAGE $FILE ; then
+ error "File '$FILE' not owned by package " \
+ "'$PACKAGE'"
+ return 1
+ fi
+ return 0
+}
+
symlink_match()
{
local SYMLINK="$1"
@@ -614,6 +620,9 @@ symlink_to_dir)
dir_to_symlink)
dir_to_symlink "$@"
;;
+package_owns_file_or_error)
+package_owns_file_or_error "$@"
+;;
--help|help|-?)
usage
;;
signature.asc
Description: This is a digitally signed message part.
