[Bug 425510] Re: 'bzr mv' should do wildcard expansion on windows

Martin Pool mbp at canonical.com
Wed Sep 9 09:48:27 BST 2009


2009/9/9 Maritza Mendez <martitzam at gmail.com>:
>
> I have made a one-line fix for this bug
> (https://bugs.launchpad.net/bugs/425510) and a similar one-line fix for
> https://bugs.launchpad.net/bugs/426410) in a private branch.  In each case
> it is a simple matter of invoking Commnd._maybe_expand_globs() on the file
> arguments although in the case of 'bzr mv' I am not sure I've thought enough
> about clever side effects.  (Ultimately I decided not to protect the user
> and just go ahead and expand blindly just like bash would.

I think that's fine.

Please, go ahead and push the branch to Launchpad, attach it to this
bug, and propose to merge it, and we'll have a look.

> So I went looking for some tests.  It appears to me that the blackbox tests
> generally do not include tests for globbing, which I guess would explain how
> these issues could exist.  Is there actually a policy of keeping the tests
> "clean" of wildcards, or is this only an oversight?

No, not at all, it's just an artifact of those tests being written
before we'd considered that issue.

> Second question: for two "simple" bugs of essentially the same type is it
> necessary to submit two separate patches?

No.

> I'm not sure when I'll have time to write the tests and get patches,
> especially since that will take longer than the fixes did!  So if anyone
> wants to claim these fixes for themself, I'd be happy with that.

Push what you have and at least it'll be visible and we'll see about
adding the tests.

>
> Also, I am honestly not very comfortable with these ad hoc fixes for 'bzr
> mv' and 'bzr rm'.  I did them because I need them now.  But it really seems
> like the Right Thing To Do would be a high-level glob expansion which
> applies a consistent policy for all input so that bzrlib would never have to
> deal with platform-dependent shells and all new commands would automatically
> work for all platforms.

It does seem a bit ad hoc.  However, when we talked about it before,
it did seem that the argument expansion needs to vary somewhat per
command: if you say "bzr ignore *.obj" then you don't want the star
expanded, and unlike on Unix there is no easy quoting mechanism.
Though I suppose we could get the user to say "bzr ignore \*.obj", but
that seems imperfect.

Perhaps a better way to tackle this is for the declaration of the
argument pattern taken by the command to declare which arguments will
be subject to expansion?

-- 
Martin <http://launchpad.net/~mbp/>



More information about the bazaar mailing list