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

Re: file filtering in dpkg -i



Hi,

On Wed, 19 May 2010, Martin Pitt wrote:
> 0001-Add-two-new-dpkg-options-exclude-and-include.patch is against
> curent git head, converts the former filter.d/ logic into new options
> --include and --exclude, and adds those to --help and the manpage. Did
> I miss any other place where this should be documented?

No, I think you covered everything relevant.

> > - new corresponding non-regression tests in pkg-tests.git
> 
> In 0001-Add-filtering-test-cases.patch.

Can you push that to some public git repository so that we can pull from
it?

> well, but t-disappear-depended and another one currently fail for me.
> However, those two fail with the original dpkg as well, and since I do
> not touch these code paths at all, I don't think it's a regression.

It fails here too, so it's not your fault no.

Some quick comments on the patch (I don't have time for a full review
right now):

> From b355fd747d349615ffe48d65045bbb8cd0b41c13 Mon Sep 17 00:00:00 2001
> From: Martin Pitt <martin.pitt@ubuntu.com>
> Date: Wed, 19 May 2010 12:41:28 +0200
> Subject: [PATCH] Add two new dpkg options --exclude and --include
> 
> This provides support for filtering files on package installation. This allows
> embedded systems to skip /usr/share/doc, manpages, etc. Based on work from
> Tollef Fog Heen, thanks!

We tend to use a pseudo-header "Based-on-patch-by: ".

> +Do not install files which match a shell pattern. `*' matches any sequence of
> +characters, including the empty string and also `/'. For example,
> +`/usr/*/READ*' matches `/usr/share/doc/mypackage/README`\fP.
> +As usual, `?' matches any single character (again, including `/').

We try to avoid using `' and prefer '' for new content.

> +++ b/src/filters.c
[..]
> +struct filterlist {
> +  int positive;
> +  char *glob;
> +  struct filterlist *next;
> +};
> +
> +static struct filterlist *filters = NULL;
> +static struct filterlist **filtertail = &filters;
> +
> +void add_filter(const char* glob, int positive)
> +{
> +  struct filterlist *filter;

For new files we use 8 char indentation with a hard tab.

> +  filter->glob = strdup(glob);
> +  if (!filter->glob)
> +    ohshite(_("error allocating memory for filter entry"));

There's m_strdup() now, please use that.

Cheers,
-- 
Raphaël Hertzog

Like what I do? Sponsor me: http://ouaza.com/wp/2010/01/05/5-years-of-freexian/
My Debian goals: http://ouaza.com/wp/2010/01/09/debian-related-goals-for-2010/


Reply to: