Merge lp:~lasall/ppa-purge/dryrun into lp:ppa-purge

[Dominique Lasserre]

Add dryrun option.

[Tormod Volden (tormodvolden)]

Thanks, looks good. It would have been even more useful if it could have shown what apt-get would be going to do, but that shouldn't stop this from going in - it is better than nothing.

review: Approve

[Dominique Lasserre (lasall)]

Do you mean the --simulate switch of apt-get/aptitude?

[Tormod Volden (tormodvolden)]

Yes, but that won't help much if apt-get update hasn't been run with the modified sources list, will it?

Maybe more useful than your current patch, which only helps to verify the PPA name, would be to actually disable the PPA in the sources list (requiring superuser) and run the apt-get install with --simulate, then restore the sources list?

[Dominique Lasserre (lasall)]

On 19/03/13 23:09, Tormod Volden wrote:
> Yes, but that won't help much if apt-get update hasn't been run with the modified sources list, will it?
Hm, didn't thought at this.

> Maybe more useful than your current patch, which only helps to verify the PPA name, would be to actually disable the PPA in the sources list (requiring superuser) and run the apt-get install with --simulate, then restore the sources list?
I would rather suggest to use an other switch for that. Because this
operation is much more non-trivial than a "dryrun". If you agree to make
this a new option (and name the switch), I will add the functionality:

 0. Update.
 1. Disable sources but make a backup (where a change is needed),
something like foobar.list.ppa-purge.save (and make sure this file not
already exists).
 2. Update.
 3. --simulate
 4. Restore backup.
 5. Update.

At that point it would be useful to display a message as long the update
is running and remove it with control sequences, e.g.:
  echo -n "Update sources..."; echo -n "\rUpdate finished. \n"

My first motivation for the "dryrun"-switch was to debug the file list
name and see if it's possible to disable other sources than a Launchpad
PPA. (At the moment this is not fully possible - everything works except
disabling the sources entry.)

[Tormod Volden (tormodvolden)]

Thanks, that sounds like a nice functionality.

Why do you need step 0?

I don't see the need to remove a message after it has been displayed, only if it would be a pure progress indicator (bars) or something repeated over and over again. This is one advantage with command-line interfaces that you can see afterwards what happened.

I agree it is a little more than a dry-run since it actually updates the package lists. But I am still not convinced we would need your original dry-run. You use it to see if the given repo name will match any apt source entries? As long as there is not many false positives there is not much harm in trying.

I understand you would like to have something symmetric to add-apt-repository. This might be a good idea. In my opinion it would make sense to have an remove-apt-repository that just removes the entries in the sources lists, but with a --revert option to "downgrade" back to package versions available in other sources and a --purge option to remove packages that are not available from the remaining sources. And "ppa-purge" would just be remove-apt-repository --revert --purge.

[Dominique Lasserre (lasall)]

On 20/03/13 21:55, Tormod Volden wrote:
> Thanks, that sounds like a nice functionality.
>
> Why do you need step 0?
It is already implemented: line 107

>
> I don't see the need to remove a message after it has been displayed, only if it would be a pure progress indicator (bars) or something repeated over and over again. This is one advantage with command-line interfaces that you can see afterwards what happened.
Ok (I thought at some "." ".." "..." points to indicate something is
running but that is really only some sugar and not important).

>
> I agree it is a little more than a dry-run since it actually updates the package lists. But I am still not convinced we would need your original dry-run. You use it to see if the given repo name will match any apt source entries? As long as there is not many false positives there is not much harm in trying.
Yes that was my usecase. And I didn't want to run an update of my
sources every time (because a single update takes over a minute).

>
> I understand you would like to have something symmetric to add-apt-repository. This might be a good idea. In my opinion it would make sense to have an remove-apt-repository that just removes the entries in the sources lists, but with a --revert option to "downgrade" back to package versions available in other sources and a --purge option to remove packages that are not available from the remaining sources. And "ppa-purge" would just be remove-apt-repository --revert --purge.
Sounds great. I think it would make sense to use python-apt for this
task because it uses the APT API (for either sources entries and package
list files). Imho the right way is to extend the existing
add-apt-repository and eventually use remove-apt-repository as a
shorthand to it. add-apt-repository already resolves its arguments to
valid source entries (either ppa:... or 'deb ...' or 'protocol://...').

[Tormod Volden (tormodvolden)]

OK, meanwhile we would be happy to have your --simulate implementation :)

(I won't be reviewing for the next 1-2 weeks, but others might jump in.)

review: Needs Fixing

[lp:~lasall/ppa-purge/dryrun]

60. By Dominique Lasserre on 2013-03-22

Simulate removal with --simulate switch.

61. By Dominique Lasserre on 2013-03-22

Fixup variable name.

[Dominique Lasserre (lasall)]

I have updated the patch and -n now uses the --simulate switch of apt-get/aptitude.

[Dominique Lasserre (lasall)]

Are there any issues with this updated patch?

[Tormod Volden (tormodvolden)]

Hi Dominique,
Can you rebase 59-60-61 into one new commit? It is just a bit confusing with commits undoing another.

You have also spelled "disable" wrong at least twice.

Are you serious about checking for 9223372036854775807 backup files? If loads of backup files are already present it will be wrong to add another IMO.

If you are not reusing NUM it is not necessary to unset it.

[Tim Lunn (darkxst)]

I guess this branch is dead, but incase anyone else decides to work on it.

You don't need to back up UNIT64_MAX files, just the one. the changes should also be reverted in the trap handler. Alternatively it may be sufficent to just revert the changes i.e remove the "# " in a trap handler and exit failure.

Unmerged revisions

61. By Dominique Lasserre on 2013-03-22

Fixup variable name.

60. By Dominique Lasserre on 2013-03-22

Simulate removal with --simulate switch.

59. By Dominique Lasserre on 2013-03-18

Add dryrun switch "-n" (lp: #802853)

=== modified file 'ppa-purge'
--- ppa-purge	2012-11-21 09:30:49 +0000
+++ ppa-purge	2013-03-22 17:08:26 +0000
@@ -8,7 +8,8 @@
 F_ARCHS=$(dpkg --print-foreign-architectures)
 PPA_PKGS=$(mktemp)
 REVERTS=$(mktemp)
-trap "rm $PPA_PKGS $REVERTS" 0
+BACKUPLISTS=$(mktemp)
+trap "rm $PPA_PKGS $REVERTS $BACKUPLISTS" 0
 
 # Functions to write output nicely.
 write_msg() {
@@ -35,6 +36,7 @@
 	echo "	-d [distribution]	Override the default distribution choice."
 	echo "	-y 			Pass "-y --force-yes" to apt-get or "-y" to aptitude"
 	echo "	-i			Reverse preference of apt-get upon aptitude."
+	echo "	-n			Simulate removal (no changes)."
 	echo "	-h			Display this help text"
 	echo
 	echo "Example usage commands:"
@@ -55,13 +57,14 @@
 }
 
 # Command line options
-while getopts "p:s:d:yih\?" opt; do
+while getopts "p:s:d:yinh\?" opt; do
 	case "$opt" in
 		p ) PPANAME="$OPTARG"				;;
 		s ) PPAHOST="$OPTARG"				;;
 		d ) DIST="$OPTARG"				;;
 		y ) FORCEINSTALL="true"				;;
 		i ) APTALT="true" 				;;
+		n ) SIMULATE="true"				;;
 		h ) usage 0;					;;
 		\?) usage 1;					;;
 		* ) warn "Unknown option '$opt'"; usage 1;	;;
@@ -82,6 +85,9 @@
 	fi
 	APT=apt-get; APTALT=aptitude
 fi
+if [ -n "$SIMULATE" ]; then
+	APTARG="$APTARG --simulate"
+fi
 
 if echo $1 | grep -q "^ppa:"; then
 	PPAOWNER=$(echo $1 | sed "s#^ppa:\(.*\)/\(.*$\)#\1#")
@@ -156,7 +162,28 @@
 # Disable PPA from sources.list files
 for LIST in $(find /etc/apt/ -name "*.list" -exec readlink -f '{}' \;); do
 	if [ -e $LIST ] && grep -q $PPAOWNER/$PPANAME $LIST; then
-		msg "Disabling $PPAOWNER PPA from $LIST"
+		if [ -z "$SIMULATE" ]; then
+			msg "Disabling $PPAOWNER PPA from $LIST"
+		else
+			NUM=""
+			if [ -f "${LIST}.ppa-purge.save" ]; then
+				NUM=1
+				while [ -f "${LIST}.ppa-purge${NUM}.save" ]; do
+					# Take care about the very rare case all files
+					# in range $LIST.ppa-purge{0,ULONG64_MAX}.save
+					# already exist and avoid endless loop (will
+					# not try "negative files" though).
+					if [ $((NUM++)) = 9223372036854775807 ]; then
+						warn "Could not find place for a backup file."
+						exit 1
+					fi
+				done
+			fi
+			msg "Disabe (and backup) $PPAOWNER PPA from $LIST (to ${LIST}.ppa-purge${NUM}.save)"
+			echo "${LIST}.ppa-purge${NUM}.save $LIST" >> $BACKUPLISTS
+			cp -- $LIST "${LIST}.ppa-purge${NUM}.save"
+			unset NUM
+		fi
 		sed -ri "\:^[^#]+/${PPAOWNER}/${PPANAME}/:s/^deb/# deb/" $LIST
 	fi
 done
@@ -170,11 +197,26 @@
 # from $REINSTALL directly.
 
 if $APT $APTARG install $REINSTALL; then
-	msg "PPA purged successfully"
+	STATE=0
 elif $APTALT $APTARG install $REINSTALL; then
-	msg "PPA purged successfully using $APTALT fallback"
+	STATE=1
 else
-	warn "Something went wrong, packages may not have been reverted"
-	exit 1
-fi
+	STATE=2
+fi
+
+if [ -n "$SIMULATE" ]; then
+	msg "Restore disalbed source entries"
+	while read BACKUP; do
+		eval mv -- $BACKUP
+	done < $BACKUPLISTS
+	msg "Updating packages lists"
+	$APT update > /dev/null || warn "$APT update failed for some reason"
+fi
+
+case $STATE in
+	0 ) msg "PPA purged successfully"						;;
+	1 ) msg "PPA purged successfully using $APTALT fallback"			;;
+	2 ) warn "Something went wrong, packages may not have been reverted"; exit 1;	;;
+esac
+
 exit 0