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

Discussion of uscan enhancement 1 (Was: uscan enhancement take 3: script hook)



Hi,

I repost some extract from some private discussion about the
Files-Excluded enhancement of uscan where Nicolas Boulenguez found
some issues.  (Nicolas, I hope you don't mind if I quote some of
your non-private remarks in public.)

Nicolas problem is mainly that if you specify

    "Files-Excluded: foo"

and foo is a directory nothing is removed because the algorithm
requires to specify rather

    "Files-Excluded: foo/"

I wonder what you might think about this.

On Thu, Aug 30, 2012 at 02:32:56AM +0200, Nicolas Boulenguez wrote:
> > I can not confirm that anything is removed if you only specify
> >  "Files-Excluded: foo"
> > (and your example below does not provide a testcase for this statement)
> 
> You are right. Apologizes. I was confused because I could never
> imagine that two behaviours were intended depending on the existence
> of an internal slash.

Well, the '/' simply marks a directory - that's all.

> Assume that "a" and "b" are directories, if I understand well, the
> current behaviour is to recursively remove "a/b/", "a/b" and "a/" but
> to ignore "a". I do not find that intuitive, and would suggest to
> document it clearly.

Could you suggest a wording which fits the requirement of "clearly
documented".  It is required that there is distinction between files and
directories (see the discussion on debian-devel, for instance [1] and
[2]).  If I would feed 'a' to the removal algorithm it could simply
happen, that a file c/a would be removed as well but it should not.

> If you keep it, the implementation could avoid
> splitting and grepping twice:
> 
>         foreach (split /\s+/, $data->{"files-excluded"}) {
>            if (grep {/\//}) {
>                 # delete trailing '/' because otherwise find -path will fail
>                 s?/+$?? ;
>                 # use rm -rf to enable deleting non-empty directories
>                 `find $main_source_dir -path "$main_source_dir/$_" -print0 | xargs -0 rm -rf`;
>             } else {
>                 `find $main_source_dir -type f -name "$_" -delete`;
>             };
>         };
> 
> In the specific case of Files-Excluded, the packager is supposed to
> know well whether (s)he removes a file or a directory,

Yes, exactly this is the sense.  The maintainer is supposed to know what
he is doing.  That's intended. :-)

I could imagine to do a

  if (grep {/[^\/]\/[^\/]/}) {

to make sure the slash is *inside* the string because if it is only at the
beginning or the end we can safely files and directories.

> and I would
> rather imitate "rm -rf", which accepts "a/b/", "a/b", "a/" and "a":
> 
>              foreach (split /\s+/, $data->{"files-excluded"}) {
>                  # delete trailing '/' because otherwise find -path will fail
>                  s?/+$?? ;
>                  # use rm -rf to enable deleting non-empty directories
>                  `find $main_source_dir -path "$main_source_dir/$_" -print0 | xargs -0 rm -rf`;
>              };
> 
> Did I miss something important?

In this case you can not specify something like

    *.jar

for jar files hanging around in lib/*.jar or so.  The code which tries
to distinguish between files and pathes enables removing files in all
directories.

> In any case, you may want to double-quote "$main_source_dir" as you
> did for "$main_source_dir/$_", in case the parent directory name
> contains a space.

Right. Commited and pushed.

Hope this clarifies things a bit.

Thanks for your testing

     Andreas.

[1] http://lists.debian.org/debian-devel/2012/08/msg00512.html
[2] http://lists.debian.org/debian-devel/2012/08/msg00449.html 

-- 
http://fam-tille.de

----- End forwarded message -----

-- 
http://fam-tille.de


Reply to: