Merge lp://staging/~mbp/bzr/557886-update-file into lp://staging/bzr

Proposed by Martin Pool
Status: Merged
Approved by: Martin Pool
Approved revision: no longer in the source branch.
Merged at revision: 6179
Proposed branch: lp://staging/~mbp/bzr/557886-update-file
Merge into: lp://staging/bzr
Diff against target: 131 lines (+72/-17)
3 files modified
bzrlib/builtins.py (+37/-16)
bzrlib/tests/blackbox/test_update.py (+29/-1)
doc/en/release-notes/bzr-2.5.txt (+6/-0)
To merge this branch: bzr merge lp://staging/~mbp/bzr/557886-update-file
Reviewer Review Type Date Requested Status
Vincent Ladeuil Needs Fixing
Review via email: mp+77286@code.staging.launchpad.net

Commit message

'bzr update PATH' makes it clear you can't update a single file

Description of the change

'bzr update -r -2 doc' doesn't do at all what you might think - it updates the
whole tree, which is fairly alarming.

With this fix, it just stops with a clear error. Also, the help is a bit better.

To post a comment you must log in.
Revision history for this message
Martin Pool (mbp) wrote :

ps, i'm doing this only in 2.5 because firstly, it's something people can avoid by using the command carefully, and secondly it's not impossible there are shell scripts that do run against a subdirectory and cope with that behaviour, and I wouldn't want to break them in a stable series.

Revision history for this message
Vincent Ladeuil (vila) wrote :

33 + If the tree's branch is bound to a master branch, bzr will also update
34
35 If the tree's branch is bound to a master branch, it will also update

Diif artefact or true duplicate ?

Overall, I'm pretty sure some people *really* want to be able to update their whole tree while being in a subdirectory.

May be you should add a check that dir != '.' for them ?

Voting needsnefixing for the typo, up to you for the suggestion about the additional check.

review: Needs Fixing
Revision history for this message
Vincent Ladeuil (vila) wrote :

Ha crap, most important bit forgotten: Thanks for updating the doc and addressing a well known pain point.

Revision history for this message
Martin Pool (mbp) wrote :

On 28 September 2011 16:49, Vincent Ladeuil <email address hidden> wrote:
> Review: Needs Fixing
>
> 33      + If the tree's branch is bound to a master branch, bzr will also update
> 34
> 35      If the tree's branch is bound to a master branch, it will also update
>
> Diif artefact or true duplicate ?

A mistake, thanks for spotting it.

> Overall, I'm pretty sure some people *really* want to be able to update their whole tree while being in a subdirectory.

They can always say 'bzr upgrade' with no arguments.

> May be you should add a check that dir != '.' for them ?

well, it does handle '.' in the root of the tree, which turns into a
relpath of ''.

I don't think 'cd subdir; bzr update .' is good to support because
that might also be taken to mean upgrading just the subdir.

m

Revision history for this message
Vincent Ladeuil (vila) wrote :

> > Overall, I'm pretty sure some people *really* want to be able to update
> their whole tree while being in a subdirectory.
>
> They can always say 'bzr upgrade' with no arguments.

Are you sure ?

>
> > May be you should add a check that dir != '.' for them ?
>
> well, it does handle '.' in the root of the tree, which turns into a
> relpath of ''.
>
> I don't think 'cd subdir; bzr update .' is good to support because
> that might also be taken to mean upgrading just the subdir.

I was talking about 'cd subdir ; bzr update' which used to work.

In other words, the 'dir' default value should be None so you can distinguish whether the user specified a 'dir' argument or not and apply your check only when the user did specify an argument and leave the previously working use case untouched.

If I read your patch correctly, people doing 'cd subdir ; bzr update' will get an error about trying to update part of the tree. I think it's a reasonable assumption for a user to expect 'bzr update' and 'bzr update .' to have different meanings and for 'bzr update' to indeed update the whole tree, 'bzr missing' and 'bzr pull' work in subdirs for example.

Hmm, 'bzr help update' while mentioning 'bzr update [DIR]' doesn't document what DIR means though...

Revision history for this message
Martin Pool (mbp) wrote :

> > > Overall, I'm pretty sure some people *really* want to be able to update
> > their whole tree while being in a subdirectory.
> >
> > They can always say 'bzr upgrade' with no arguments.
>
> Are you sure ?

Well, I added a test :-) and it turns out that you're correct, this does not work.

update' which used to work.
>
> In other words, the 'dir' default value should be None so you can distinguish
> whether the user specified a 'dir' argument or not and apply your check only
> when the user did specify an argument and leave the previously working use
> case untouched.

OK, that seems like a good way to fix it.

> Hmm, 'bzr help update' while mentioning 'bzr update [DIR]' doesn't document
> what DIR means though...

I'll address that too.

Revision history for this message
Martin Pool (mbp) wrote :

I've fixed the issues vila pointed out (thanks) and I think he verbally ok'd this.

Revision history for this message
Martin Pool (mbp) wrote :

sent to pqm by email

Revision history for this message
Martin Pool (mbp) wrote :

sent to pqm by email

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.