`bzr branch URL .` refuses to create new branch in empty directory

This bug affects qbranch dialog in QBzr plugin and therefore Bazaar Explorer GUI.

qbranch allows the user to select source branch and destination location. For destination location user can select new folder using native dialog to select directory. On Windows such native dialog allows user to create new empty directory. So it's natural for the user to create new empty dir and then run the operation. And as result -- got the error.

Explaining the user that he/she SHOULD NOT create new empty dir when branching via qbranch is... silly?

Is it possible to fix this easily in bzr itself? As I said before, checkout into existing directory works without any problems.

Here is relevant part of cmd_branch implementation:

            try:
                to_transport.mkdir('.')
            except errors.FileExists:
                raise errors.BzrCommandError('Target directory "%s" already'
                                             ' exists.' % to_location)
            except errors.NoSuchFile:
                raise errors.BzrCommandError('Parent of "%s" does not exist.'
                                             % to_location)

How it could be changed? Removing the check for FileExists at all? Using --force flag?
What is the right thing here?

I think that the error should only occur if the directory exists and is non-empty.

Forest Bond пишет:
> I think that the error should only occur if the directory exists and is
> non-empty.

Non-empty is not very good condition to check.
How you check it when destination is bzr+ssh URL?

Much better if bzr simply try to open branch in the existing directory
and refuse to create new branch if there is already branch.
IIUC, today check for non-existent directory is needed only to avoid
branching into existing branch.

On Thu, 2009-06-25 at 12:42 +0000, Alexander Belchenko wrote:
> Forest Bond пишет:
> > I think that the error should only occur if the directory exists and is
> > non-empty.
>
> Non-empty is not very good condition to check.
> How you check it when destination is bzr+ssh URL?
>
> Much better if bzr simply try to open branch in the existing directory
> and refuse to create new branch if there is already branch.
> IIUC, today check for non-existent directory is needed only to avoid
> branching into existing branch.

The logic and option name for use_existing_dir in push can be used.

As for bzr+ssh urls, I would expect 'bzr push' to be used for them, and
push already has --use-existing-dir.

-Rob

2009/6/26 Maritza Mendez <email address hidden>:
>
>
> On Thu, Jun 25, 2009 at 2:19 AM, Alexander Belchenko <email address hidden> wrote:
>>
>> See https://bugs.launchpad.net/qbzr/+bug/307554
>>
>> I'd like to get some comments from core bzr.devs because this bug affects
>> qbzr plugin (because qbzr launches native bzr commands under the hood). QBzr
>> is used in Bazaar Explorer GUI, and the more I'm using bzre the more I like
>> it. But such bugs just create bad expression about all 3 programs.
>>
>> It seems it could be relatively easy fix, and I'd like to see it fixed in
>> 1.17. How can I fix it?

I think if the directory is completely empty it'd be fine to just use
it. (I thought there was a much older bug asking for the same thing.)
 It should be fairly straightforward.

> Alexander: I know you are asking for iput from core dev's.  Just let me add
> my voice to saying that you understand the user experience correctly.  I
> have refrained from saying anything about this in qbzr because I do not have
> time to develop a fix that will make everyone happy.  But I will say that
> Bazaar Explorer exposes this issue very strongly.
>
> Maybe I'm thinking about htis too simply, but it seems to me that the only
> check which needs to be performed on the target filesystem when creating a
> new (non-shared) branch is to make sure the target directory does not
> already have a .bzr repository.  (Trying to create a shared repo on top of
> an existing branch (shared or non-shared) needs to be prevented, right?)

You're right that we should be careful about the case where there's
already a .bzr directory. I think we should also be careful about the
case where there's non-bzr content - if you then create a working tree
there, presumably it'll have to merge with those files. (I guess it
would be a two-way merge with no common base.) It's quite possible
this would sometimes be useful, when the directory you're extracting
into contains a slightly modified version of the tree you're
extracting, perhaps from a tarball or maintained outside of vcs.
However, it would sometimes be confusing and it would take some care
that we actually do the merge in a reasonable way. (I think at the
moment bzr branch just builds the tree regardless of what's there.)
So extracting into a non-empty directory I would split off into a
separate bug.

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

That makes good sense.

-M

On 6/25/09, Martin Pool <email address hidden> wrote:
> 2009/6/26 Maritza Mendez <email address hidden>:
>>
>>
>> On Thu, Jun 25, 2009 at 2:19 AM, Alexander Belchenko <email address hidden>
>> wrote:
>>>
>>> See https://bugs.launchpad.net/qbzr/+bug/307554
>>>
>>> I'd like to get some comments from core bzr.devs because this bug
>>> affects
>>> qbzr plugin (because qbzr launches native bzr commands under the hood).
>>> QBzr
>>> is used in Bazaar Explorer GUI, and the more I'm using bzre the more I
>>> like
>>> it. But such bugs just create bad expression about all 3 programs.
>>>
>>> It seems it could be relatively easy fix, and I'd like to see it fixed
>>> in
>>> 1.17. How can I fix it?
>
> I think if the directory is completely empty it'd be fine to just use
> it. (I thought there was a much older bug asking for the same thing.)
> It should be fairly straightforward.
>
>> Alexander: I know you are asking for iput from core dev's.  Just let me
>> add
>> my voice to saying that you understand the user experience correctly.  I
>> have refrained from saying anything about this in qbzr because I do not
>> have
>> time to develop a fix that will make everyone happy.  But I will say that
>> Bazaar Explorer exposes this issue very strongly.
>>
>> Maybe I'm thinking about htis too simply, but it seems to me that the
>> only
>> check which needs to be performed on the target filesystem when creating
>> a
>> new (non-shared) branch is to make sure the target directory does not
>> already have a .bzr repository.  (Trying to create a shared repo on top
>> of
>> an existing branch (shared or non-shared) needs to be prevented, right?)
>
> You're right that we should be careful about the case where there's
> already a .bzr directory. I think we should also be careful about the
> case where there's non-bzr content - if you then create a working tree
> there, presumably it'll have to merge with those files. (I guess it
> would be a two-way merge with no common base.) It's quite possible
> this would sometimes be useful, when the directory you're extracting
> into contains a slightly modified version of the tree you're
> extracting, perhaps from a tarball or maintained outside of vcs.
> However, it would sometimes be confusing and it would take some care
> that we actually do the merge in a reasonable way. (I think at the
> moment bzr branch just builds the tree regardless of what's there.)
> So extracting into a non-empty directory I would split off into a
> separate bug.
>
> --
> Martin <http://launchpad.net/~mbp/>
>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

...

>>> Maybe I'm thinking about htis too simply, but it seems to me that the
>>> only
>>> check which needs to be performed on the target filesystem when creating
>>> a
>>> new (non-shared) branch is to make sure the target directory does not
>>> already have a .bzr repository. (Trying to create a shared repo on top
>>> of
>>> an existing branch (shared or non-shared) needs to be prevented, right?)
>> You're right that we should be careful about the case where there's
>> already a .bzr directory. I think we should also be careful about the
>> case where there's non-bzr content - if you then create a working tree
>> there, presumably it'll have to merge with those files. (I guess it
>> would be a two-way merge with no common base.) It's quite possible
>> this would sometimes be useful, when the directory you're extracting
>> into contains a slightly modified version of the tree you're
>> extracting, perhaps from a tarball or maintained outside of vcs.
>> However, it would sometimes be confusing and it would take some care
>> that we actually do the merge in a reasonable way. (I think at the
>> moment bzr branch just builds the tree regardless of what's there.)
>> So extracting into a non-empty directory I would split off into a
>> separate bug.

I believe that "bzr checkout ." and "bzr checkout $URL ." already handle
these cases, so it shouldn't be particularly harder for "bzr branch" to
do so.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkpENfoACgkQJdeBCYSNAAMycQCfQRApVXWPsuf7Pzdb4dN+EJn2
KWIAoLSFwrwzr3o1GluZysX3ylY7vZzA
=Nx5S
-----END PGP SIGNATURE-----

qbranch now can use flag --use-existing-dir. Need to wait till patch for bzrlib landed.

merged as bzr.dev revno.4500