Merge lp:~shnatsel/ppa-purge/drop-awk-dependency into lp:ppa-purge

Proposed by Sergey "Shnatsel" Davidoff
Status: Needs review
Proposed branch: lp:~shnatsel/ppa-purge/drop-awk-dependency
Merge into: lp:ppa-purge
Diff against target: 27 lines (+3/-3)
2 files modified
debian/control (+1/-1)
ppa-purge (+2/-2)
To merge this branch: bzr merge lp:~shnatsel/ppa-purge/drop-awk-dependency
Reviewer Review Type Date Requested Status
Stefano Rivera (community) Needs Information
Review via email: mp+85466@code.launchpad.net

Description of the change

I've replaced the only AWK command with a combination of grep and cut for readability and consistency with the rest of the script. This also drops additional dependency on awk.

To post a comment you must log in.
Revision history for this message
Stefano Rivera (stefanor) wrote :

Thanks for the patch, but I'm not entirely convinced of its necessity.
mawk is Priority: required, so it's present on every image and the dependency isn't a big deal.

Also, you want to grep for 'install$', otherwise you'll catch packages with "install" in their name.

review: Needs Information
Revision history for this message
Sergey "Shnatsel" Davidoff (shnatsel) wrote :

Good catch with grep!
Actually, both this version and the one with awk also match "deinstall". Fixing.

58. By Sergey "Shnatsel" Davidoff

fixed matching "install" in package name and "deinstall" state

Revision history for this message
Sergey "Shnatsel" Davidoff (shnatsel) wrote :

Fixed matching "install" in package name
Fixed matching "deinstall" package state

59. By Sergey "Shnatsel" Davidoff

added dependencies to debian/control - depending only on aptitude and hoping everything else is in place is not a good idea

60. By Sergey "Shnatsel" Davidoff

well, it seems to be sorted to me, but comm disagrees
this will make it happy for sure

Revision history for this message
Tormod Volden (tormodvolden) wrote :

Thanks for refining the scripts!

awk can also do it properly without regexp fiddling:
 dpkg --get-selections | awk '{if ($2=="install") print $1}'

Revision history for this message
Sergey "Shnatsel" Davidoff (shnatsel) wrote :

Yeah, I believe awk is a powerful tool, now merge this please :)

Or fix the trunk with this awk tweak and add some dependencies to debian/control at least.

I don't care about this particular merge anymore because I ended up rewriting most of the code anyway: lp:~shnatsel/ppa-purge/ppa-purge-aptdaemon

Revision history for this message
Sergey "Shnatsel" Davidoff (shnatsel) wrote :

The new lp:~shnatsel/ppa-purge/ppa-purge-aptdaemon branch seems fairly stable and robust to me now, but because of an upstream bug that was fixed just yesterday it requires a very recent aptdaemon version that's available only in Ubuntu Precise. Which means that the current version still will be around until Precise release, which means that this branch still needs a merge ;)

Revision history for this message
Stefano Rivera (stefanor) wrote :

Dependency should probably be apt-get | aptitude, right?

Of course, aptitude is a really poor choice until bug #831768 is dealt with...

Revision history for this message
Sergey "Shnatsel" Davidoff (shnatsel) wrote :

Right now it depends on aptitude because of its reinstallation behavior (see a FIXME in the code), and on apt for collecting data, so it should depend on both apt-get and aptitude.

One of the possible solutions is determining the exact version to install and force installing that particular version (I did that in the aptdaemon branch), then it can depend on only one of the tools.

Unmerged revisions

60. By Sergey "Shnatsel" Davidoff

well, it seems to be sorted to me, but comm disagrees
this will make it happy for sure

59. By Sergey "Shnatsel" Davidoff

added dependencies to debian/control - depending only on aptitude and hoping everything else is in place is not a good idea

58. By Sergey "Shnatsel" Davidoff

fixed matching "install" in package name and "deinstall" state

57. By Sergey "Shnatsel" Davidoff

replaced the only awk command with combination of grep and cut for readability; this also drops awk dependency

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/control'
2--- debian/control 2010-10-15 21:22:59 +0000
3+++ debian/control 2011-12-13 20:11:24 +0000
4@@ -8,7 +8,7 @@
5
6 Package: ppa-purge
7 Architecture: all
8-Depends: ${misc:Depends}, aptitude
9+Depends: ${misc:Depends}, apt, aptitude, sed, bash
10 Description: disables a PPA and reverts to official packages
11 This program disables a PPA from your Software Sources and reverts your
12 system back to the official Ubuntu packages. You can use this to return your
13
14=== modified file 'ppa-purge'
15--- ppa-purge 2010-11-10 20:13:14 +0000
16+++ ppa-purge 2011-12-13 20:11:24 +0000
17@@ -127,8 +127,8 @@
18 msg "Note: Not removing ppa-purge package"
19 fi
20
21-dpkg --get-selections | awk '/install$/{print $1}' |
22- sort | comm -12 - $PPA_PKGS > $REVERTS
23+dpkg --get-selections | grep --perl-regexp '\tinstall$' | cut -f 1 | sort |
24+ comm -12 - $PPA_PKGS > $REVERTS
25
26 # Create apt argument list for reverting packages
27 REINSTALL=""

Subscribers

People subscribed via source and target branches