On Fri, 15 May 2009, Modestas Vainius wrote:
> > > +sub get_name {
> > > + my $self = shift;
> > > + my $symbol = $self->{symbol};
> > > + $symbol =~ s/['"']//g;
> > > + return $symbol;
> > > +}
> >
> > The quotes should be stripped in parse() and not be stored ?
> > (And then restored in dump() only if necessary)
>
> No, it is not that simple. dump() is supposed to dump the same symbol
> specification as has read it not to trigger useless dpkg-gensymbols diffs.
> Which means that original tag order, quoted/not quoted status must be known.
> As tag value quoting is going away, I could probably set $self-
> >{symbol_quoted} to indicate wheather symbol name should be quoted on dump()
> or not.
For the tag order, just keep a supplementary @tagorder with the ordered
list of tag names then. Using a hash to access the tags and their values
is so much more natural. For the quoted/not-quoted status, a simple scalar
would indeed do it.
> > > @@ -73,8 +281,9 @@ unwind_cpp_pr2 uread4 uread8 uwrite4 uwrite8));
> > > sub new {
> > > my $this = shift;
> > > my $file = shift;
> > > + my %op...@_;
> >
> > If you add supplementary key-based arguments, you should convert the
> > current single argument to also be key based (key "file" for example).
>
> I somewhat wanted to keep API backwards compatible.
That's a concern for when we define that the API is stable. Currently we
haven't set that and dpkg-dev is the sole user that matters.
> > > - # We assume that the right dependency information is already
> > > - # there.
> > > - if (vercmp($minver, $info->{minver}) < 0) {
> > > - $info->{minver} = $minver;
> > > + if (!$info->is_from_arch($self->{arch})) {
> > > + # Remove arch tags because they are incorrect.
> > > + $info->delete_tags("arch", "!arch");
> >
> > A warning is in order I think.
>
> Removal of the arch tag makes the symbol appear in the dpkg-gensymbols diff.
> That's the point of all this. A simple warning might get lost in the build
> log.
Ok.
> > Once those changes/cleanup are done, we are not far from something
> > that can be committed. We still need documentation update of the
> > dpkg-gensymbols manual page for the new features.
>
> I'm very slow at writing docs.
I'll take care of it if you don't do it.
> > We probably want
> > to add an option to make dpkg-gensymbols dump out the tags too.
>
> I'm not sure what you mean here. dpkg-gensymbols dumps tags when diff'ing two
> symbol file templates. Dumping tags to DEBIAN/symbols does not make sense as
> syntax is not backwards compatible with previous versions.
Of course, the user that calls dpkg-gensymbols --include-tags is expected
to have a -O option and redirect the output somewhere else. I'm thinking
here on the mole code that uses dpkg-gensymbols to prepare initial symbols
files ready to be put in the source package. Of course it's not meant to
be used in normal builds and should not end up in DEBIAN/symbols.
BTW, it would be nice if your next patch also took care of the test suite
(fixing what needs to be fixed, and expanding the test suite to cover the
new features).
Cheers,
--
Raphaël Hertzog
Contribuez à Debian et gagnez un cahier de l'admin Debian Lenny :
http://www.ouaza.com/wp/2009/03/02/contribuer-a-debian-gagner-un-livre/
--
To UNSUBSCRIBE, email to [email protected]
with a subject of "unsubscribe". Trouble? Contact [email protected]
