[Dctrl-tools-devel] Bug#476861: Bug#383921: Bug#476861: dctrl-tools: option to match against package-name boundaries

[Antti-Juhani Kaijanaho]

On Thu, Jan 29, 2009 at 11:48:42AM +0100, Stefano Zacchiroli wrote:
> The attached patch does exactly that. The name of the flag I've chose
> is "--whole-pkg", YMMV. Note that in my implementation the flag
> implies "-e", because the implementation is actually (extended) regex
> based.
> 
> More in detail, I just "dress" the given atom(s) with regex boundaries
> as yours. It might be seen as hackish, but I find it way better than
> adding specific parsing of inter-package relationship
> fields. Considering that the use case we are discussing is really
> common, I believe it is worth to implement this this way.

I do think this is a bit inelegant, but all other avenues for implementation
that I can think of are much more complex.

In other words, I think your approach is sound.

> I've changed a bit the regular expression boundaries wrt yours, mines
> are:
> 
> 	#define RE_PKG_BEGIN	"(^| )"
> 	#define RE_PKG_END	"([, \\(]|$)"
> 
> with the rationale that whitespaces, according to policy, do not
> necessarily appear between package names and optional version
> requirements; hence also '(' can denote the end of package name.

Would it make sense to instead rule out any character that would be a part of a
package name?

> Please consider applying the attached patch.

Since you are a DD, you can commit it yourself when it's ready.
(Please read debian/README, first.)

> From 13cb48a28f20a34a454b97b0b41c02b12733f988 Mon Sep 17 00:00:00 2001
> From: Stefano Zacchiroli <zack at upsilon.cc>
> Date: Thu, 29 Jan 2009 11:21:38 +0100
> Subject: [PATCH] add '--whole-pkg' to match (eregex) on whole package names
> 
> Using this flag given extended regex will not match substring of
> package names in fields expressing inter-package relationships (e.g.,
> Depends, Recommends, ...).
> 
> Passing '--whole-pkg' implies '-e'.
> 
> (Closes: #476861) actually, proposed fix for ...

You haven't signed off the patch.  Please read
  http://git.debian.org/?p=collab-maint/dctrl-tools.git;a=blob_plain;f=dco-1.1.txt;hb=dco
and if you can certify that, please add a Signed-Off-By line at the
end of the patch description (see debian/README).

> +	char * regex_pat = NULL;
> +	int regex_patlen = atom->patlen + 30;

Where does the magic number 30 come from?  The combined length of the two regexps?
If so, I would prefer making that explicit, as in:

  int regex_patlen = atom->patlen + strlen(RE_PKG_BEGIN) + strlen(RE_PKG_END) + 1;

(I believe strlen of constant strings gets optimized out, but it's not critical
since we are not in the hot loops.)

... Ah, I see you've already done this.  I would prefer combining the two code
patches, though.

>  	if (atom->mode == M_REGEX || atom->mode == M_EREGEX) {
> +		regex_pat = calloc(1, regex_patlen);

Do you use the property that the returned memory is zeroed out?

> +.IP "\-\-whole\-pkg"
> +Do an extended regular expression match on whole package names,
> +assuming the syntax of inter-package relationship fields such as
> +Depends, Recommends, ... When this flag is given you should not worry
> +about sub-package names such as "libpcre3" also matching
> +"libpcre3-dev". This flag implies \-e.

I'd prefer allowing us to later drop the -e connection, so should say
"currently implies" or similar.

-- 
Antti-Juhani Kaijanaho, Jyväskylä, Finland
http://antti-juhani.kaijanaho.fi/newblog/
http://www.flickr.com/photos/antti-juhani/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
Url : http://lists.alioth.debian.org/pipermail/dctrl-tools-devel/attachments/20090225/50e25b7a/attachment-0001.pgp