[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [PATCH] New Dpkg::Deps module to replace parsedep() and showdep()



Hi,

the latest version of my branch is online:
http://git.debian.org/?p=users/hertzog/dpkg.git;a=shortlog;h=deps-rewrite
git://git.debian.org/~hertzog/dpkg.git

On Thu, 18 Oct 2007, Guillem Jover wrote:
> No need to call textdomain inside the modules (those can only be used
> in dpkg-dev programs) and the function is called already on the programs
> anyway.

Fixed.

> > diff --git a/scripts/dpkg-gencontrol.pl b/scripts/dpkg-gencontrol.pl
> > @@ -14,10 +14,13 @@ use Dpkg::Arch qw(get_host_arch debarch_eq debarch_is);
> >  push(@INC,$dpkglibdir);
> >  require 'controllib.pl';
> >  
> > +use Dpkg::Deps qw(@pkg_dep_fields %dep_field_type);
> > +
> >  our %substvar;
> >  our (%f, %fi);
> >  our %p2i;
> >  our @pkg_dep_fields;
> > +our %dep_field_type;
> 
> Those two can be removed instead, as they have been explicitely imported
> now. Also please put the 'use' with the others, I placed the our list
> there, close to controllib, as a reminder of what we are using from it.

I put it there because I didn't remove the @pkg_dep_fields from
controllib.pl as it was also used somewhere else. Now I removed it
and converted dpkg-scanpackage to use it from Dpkg::Deps too so that I
could move the use with the others.

> > +    my $provides = Dpkg::Deps::parse(substvars($fi{"C$myindex Provides"}),
> > +				     reduce_arch => 1, union => 1);
> 
> Do not use tabs for alignment (only for indenting).

Done everywhere although I don't understand the rationale.

> > +	    &error(sprintf(_g("error occurred while parsing %s"), $_)) unless defined $dep;
> 
> The error function calls will have to be fixed for the new format string
> support.

Done. That last change created several other conflicts during my rebase,
fixed them all.

> > +		# XXX: Should use %dep_field_type to decide if we parse
> > +		# as union or not but since case-insensitive matching is
> > +		# used, I can't rely on $_ to have the very same
> > +		# capitalization...
> > +		if (lc($1) eq "depends") {
> > +		    $dep = Dpkg::Deps::parse($v);
> > +		} else {
> > +		    $dep = Dpkg::Deps::parse($v, union => 1);
> > +		}
> 
> You can use capit() from controllib.pl. Although it might be better to
> normalize all fields at parse/input time instead of all over the place
> and switch to case-sensitive matching, but we can fix that later globally.

Yeah, left that for later.

> > +	* scripts/dpkg-{checkbuilddeps.pl,gencontrol.pl,source.pl}:
> > +	adapted to use the new Dpkg::Deps module. dpkg-gencontrol gains
> > +	new features such as automatic simplification of dependencies.
> > +	* scripts/controllib.pl: remove unused parsedep() and showdep().
> 
> Please do not use globs in changelogs, as it makes using grep more
> difficult.

Fixed.

Cheers,
-- 
Raphaël Hertzog

Premier livre français sur Debian GNU/Linux :
http://www.ouaza.com/livre/admin-debian/



Reply to: