Merge lp:~eugenesan/ppa-purge/trunk into lp:ppa-purge

[Eugene San]

Adding support for compressed and empty packages lists

[Tormod Volden (tormodvolden)]

Hi, thanks for the code. This could also have been three commits though: 1) whitespace clean-up, 2) adding support for compressed package lists, 3) add support for empty package lists.

Anyway, does it really work? $LIST is always an existing file (as long as the * in PPA_LIST successfully expands, otherwise it is useless, that was what the original -e checked for), so 1) the elif's can not be reached and 2) $LIST.gz can not exist at the same time as $LIST, right? I don't have any compressed lists on my machine so I can not verify this case :)

Basically, 1) the PPA_LIST does not match anything ending in .gz etc, and 2) if it would, $LIST.gz would not exist since the .gz would already be included in $LIST.

I can suggest instead adding * to the end of the PPA_LIST so that any filename suffix gets expanded, then use lesspipe to decompress $LIST whatever its file type is (kind of a hack).

[Eugene San (eugenesan)]

Hi, thanks for prompt review.

Sorry for combining all changes together, wasn't sure they deserve separate
commits.
After reading your points I realized that the shouldn't work, but some how
it does!
I probably was still sleeping while committing :-)
I am going to review the code one more time later.

Just in case you willing to play with it, this is how you enable compressed
lists:
echo "
Acquire::GzipIndexes "true";
Acquire::CompressionTypes::Order:: "gz";
" >> /etc/apt/apt.conf

And for empty repository:
add-apt-repository unity/ppa

On Tue, Jan 18, 2011 at 11:25, Tormod Volden <email address hidden>wrote:

> Hi, thanks for the code. This could also have been three commits though: 1)
> whitespace clean-up, 2) adding support for compressed package lists, 3) add
> support for empty package lists.
>
> Anyway, does it really work? $LIST is always an existing file (as long as
> the * in PPA_LIST successfully expands, otherwise it is useless, that was
> what the original -e checked for), so 1) the elif's can not be reached and
> 2) $LIST.gz can not exist at the same time as $LIST, right? I don't have any
> compressed lists on my machine so I can not verify this case :)
>
> Basically, 1) the PPA_LIST does not match anything ending in .gz etc, and
> 2) if it would, $LIST.gz would not exist since the .gz would already be
> included in $LIST.
>
> I can suggest instead adding * to the end of the PPA_LIST so that any
> filename suffix gets expanded, then use lesspipe to decompress $LIST
> whatever its file type is (kind of a hack).
> --
> https://code.launchpad.net/~eugenesan/ppa-purge/trunk/+merge/46570<https://code.launchpad.net/%7Eeugenesan/ppa-purge/trunk/+merge/46570>
> You are the owner of lp:~eugenesan/ppa-purge/trunk.
>

[Eugene San (eugenesan)]

Hi,

I followed Tormod Volden's great suggestion and used lesspipe, now everything is working.

Also I've added "err" function, since till now we used "warn" also for fatal errors.

[Tormod Volden (tormodvolden)]

What are you filtering out here? I would filter away as little as possible, so that the day something unexpected happens, the error message can be seen.
119 done 2> /dev/null > /dev/null

This won't work:
+[ -z "$LIST" ] && err "Could not find packages list for PPA: $PPAOWNER $PPANAME"
because $LIST is always non-zero. If no match it will just not expand the *.

What is exactly the use case for empty apt lists? What do we want to happen in this case? Asking since it is a long time since I worked on this, and also check that we agree on the overall logic.

review: Needs Fixing

[Tormod Volden (tormodvolden)]

And by the way, if you are preparing a new release in debian/changelog, and you made all the changes, put yourself on the signing line! Don't be shy :) It will have to be sponsored for uploading anyway.

[Eugene San (eugenesan)]

Hi,

I've got your points.
This is what I propose instead:
for LIST in /var/lib/apt/lists/${PPAHOST}_${PPAOWNER}_${PPANAME}_*_Packages* ; do
        if [ -e "$LIST" ]; then
                lesspipe ${LIST} | grep "^Package: " | cut -d " " -f2 | sort >> $PPA_PKGS
        else
                err "Could not find packages list for PPA: $PPAOWNER $PPANAME"
        fi
done

In case there is no lists by wildcard, [ -e ] will fail.

Regarding empty/missing lists.
Today I've tried to remove empty repo (without knowing it is empty) and ppa-purge failed. So I added treatment of this case. And missing lists treated to distinguish that from case when list is just empty.

[Tormod Volden (tormodvolden)]

Thanks, that looks very good. Note that we don't use {} around variables other places unless it is needed, and it is not needed on this lesspipe command either.

The empty repo is something that ppa-purge can not handle. It is a bad PPA which removes packages that have been published :) The hacky logic in ppa-purge uses the existing packages in the PPA to figure out which packages need to be reverted. So if there are no packages there, ppa-purge should fail in my opinion. All it can do in this case is removing the PPA from the sources.list, but the user should rather use Software Sources for this. Well, somebody should write apt-delete-repository to pair with apt-add-repository :) Then we could also just call that internally when things are ok, and bail out and tell the user to just use that when the PPA is empty, and not give the user the impression that ppa-purge did its job, which is mainly the reverting packages part.

There are tricks like "aptitude search '~i!~Oubuntu' to find packages that are not installed from Ubuntu repositories, but I don't know a way to find packages installed from a PPA.

[lp:~eugenesan/ppa-purge/trunk]

60. By Eugene San on 2011-01-21

Fixng failed empty list detection

61. By Eugene San on 2011-02-22

Update debian/changelog

Unmerged revisions

61. By Eugene San on 2011-02-22

Update debian/changelog

60. By Eugene San on 2011-01-21

Fixng failed empty list detection

59. By Eugene San on 2011-01-18

Fixing nonblocking typos

58. By Eugene San on 2011-01-18

Finalizing support for compressed and empty apt lists

57. By Eugene San on 2011-01-18

Merged main branch

56. By Eugene San on 2011-01-18

Adding support for compressed and empty packages lists

55. By eugenesan <eugenesan@portsan> on 2010-08-23

Fixing typo in changelog

54. By eugenesan <eugenesan@portsan> on 2010-08-23

Fixing typo causing permanent aptitude usage

53. By eugenesan <eugenesan@portsan> on 2010-08-23

Few changes for upcoming 0.2.8

52. By eugenesan <eugenesan@portsan> on 2010-08-22

Adding reverse order of apt tool usage and support for symlinked lists

=== modified file 'debian/changelog'
--- debian/changelog	2010-11-10 20:13:14 +0000
+++ debian/changelog	2011-02-22 14:57:19 +0000
@@ -1,3 +1,12 @@
+ppa-purge (0.2.8+bzr59) natty; urgency=low
+
+  [ Eugene San (eugenesan) ]
+  * Replace "warn() && exit 1" with err()
+  * Cosmetic changes
+  * Add support for compressed and empty apt lists
+
+ -- Eugene San (eugenesan) <[e-mail]>  Tue, 22 Feb 2011 16:43:00 +0300
+
 ppa-purge (0.2.8+bzr56) natty; urgency=low
 
   * Really implement the "-y" option.

=== modified file 'debian/control'
--- debian/control	2010-10-15 21:22:59 +0000
+++ debian/control	2011-02-22 14:57:19 +0000
@@ -8,7 +8,7 @@
 
 Package: ppa-purge
 Architecture: all
-Depends: ${misc:Depends}, aptitude
+Depends: ${misc:Depends}, aptitude, less
 Description: disables a PPA and reverts to official packages
  This program disables a PPA from your Software Sources and reverts your
  system back to the official Ubuntu packages. You can use this to return your 

=== modified file 'ppa-purge'
--- ppa-purge	2010-11-10 20:13:14 +0000
+++ ppa-purge	2011-02-22 14:57:19 +0000
@@ -20,7 +20,12 @@
 }
 
 warn() {
-	write_msg "Warning:  $*" 1>&2
+	write_msg "Warning: $*" 1>&2
+}
+
+err() {
+	write_msg "Error: $*" 1>&2
+	exit 1
 }
 
 usage() {
@@ -61,7 +66,7 @@
 		s ) PPAHOST="$OPTARG"				;;
 		d ) DIST="$OPTARG"				;;
 		y ) FORCEINSTALL="true"				;;
-		i ) APTALT="true" 				;;
+		i ) APTALT="true"				;;
 		h ) usage 0;					;;
 		\?) usage 1;					;;
 		* ) warn "Unknown option '$opt'"; usage 1;	;;
@@ -73,12 +78,12 @@
 APTARG=""
 if [ ! -z "$APTALT" ]; then
 	if [ ! -z "$FORCEINSTALL" ]; then
-	APTARG="-y"
+		APTARG="-y"
 	fi
 	APT=aptitude; APTALT=apt-get
 else
 	if [ ! -z "$FORCEINSTALL" ]; then
-	APTARG="-y --force-yes"
+		APTARG="-y --force-yes"
 	fi
 	APT=apt-get; APTALT=aptitude
 fi
@@ -109,36 +114,36 @@
 msg "PPA to be removed: $PPAOWNER $PPANAME"
 
 # Make list of all packages in PPA
-PPA_LIST=/var/lib/apt/lists/${PPAHOST}_${PPAOWNER}_${PPANAME}_*_Packages
-for LIST in $PPA_LIST; do
-	if [ -e $LIST ]; then
-		grep "^Package: " $LIST | cut -d " " -f2 | sort >> $PPA_PKGS
+for LIST in /var/lib/apt/lists/${PPAHOST}_${PPAOWNER}_${PPANAME}_*_Packages* ; do
+	if [ -e "$LIST" ]; then
+		lesspipe ${LIST} | grep "^Package: " | cut -d " " -f2 | sort >> $PPA_PKGS
+	else
+		err "Could not find packages list for PPA: $PPAOWNER $PPANAME"
 	fi
 done
 
 if [ ! -s $PPA_PKGS ]; then
-	warn "Could not find package list for PPA: $PPAOWNER $PPANAME"
-	exit 1
-fi
-
-# Ignore the ppa-purge package
-if grep -q "ppa-purge" $PPA_PKGS; then
-	sed -i '/ppa-purge/d' $PPA_PKGS
-	msg "Note: Not removing ppa-purge package"
-fi
-
-dpkg --get-selections | awk '/install$/{print $1}' |
-     sort | comm -12 - $PPA_PKGS > $REVERTS
-
-# Create apt argument list for reverting packages
-REINSTALL=""
-for PACKAGE in $(cat $REVERTS); do
-	REINSTALL="$REINSTALL $PACKAGE/$DIST"
-done
-
-msg "Package revert list generated:"
-msg "$REINSTALL"
-echo
+	warn "PPA: $PPAOWNER $PPANAME is empty, skipping packages treatment."
+else
+	# Ignore the ppa-purge package
+	if grep -q "ppa-purge" $PPA_PKGS; then
+		sed -i '/ppa-purge/d' $PPA_PKGS
+		msg "Note: Not removing ppa-purge package"
+	fi
+
+	dpkg --get-selections | awk '/install$/{print $1}' |
+		sort | comm -12 - $PPA_PKGS > $REVERTS
+
+	# Create apt argument list for reverting packages
+	REINSTALL=""
+	for PACKAGE in $(cat $REVERTS); do
+		REINSTALL="$REINSTALL $PACKAGE/$DIST"
+	done
+
+	msg "Package revert list generated:"
+	msg "$REINSTALL"
+	echo
+fi
 
 # Disable PPA from sources.list files
 for LIST in $(find /etc/apt/ -name "*.list" -exec readlink -f '{}' \;); do
@@ -161,7 +166,7 @@
 elif $APTALT $APTARG install $REINSTALL; then
 	msg "PPA purged successfully using $APTALT fallback"
 else
-	warn "Something went wrong, packages may not have been reverted"
-	exit 1
+	err "Something went wrong, packages may not have been reverted"
 fi
+
 exit 0