MYSQL/BZR - P3: How to avoid that people commit .OTHER/.BASE/.THIS/.moved files?

Bug #322767 reported by Peter Matulis on 2009-01-29
4
Affects Status Importance Assigned to Milestone
Bazaar
High
Vincent Ladeuil
Declined for 2.0 by Vincent Ladeuil
Declined for 2.1 by Robert Collins
Declined for 2.2 by Robert Collins
Declined for Trunk by Robert Collins
bzr (Ubuntu)
Undecided
Unassigned

Bug Description

This is a Sun/MySQL - Canonical escalation imported into Launchpad by the Canonical Support Team (internal case 3580).

Original escalation: 15/12/2008 9:58 AM EST

Public bug: https://bugs.launchpad.net/bzr/+bug/308403

===

Guilhem Bichot

Hello,
Out of the 100 MySQL developers who do bzr merges, we see that sometimes some accidentally commit .OTHER/.BASE/.THIS/.moved files to the repository. Most often, *.moved. To excuse them, there are sometimes path conflicts which shouldn't be (ok, let's not get started on this :), so .moved files are common.
I imagined that I could just do
bzr ignore *.moved
then commit that change and push, and it would prevent my colleagues, once they pull my change, to ever commit some *.moved files. But no. After doing the commit above, I can "touch a.moved; bzr add a.moved" and bzr does not complain, I can commit a.moved, it gets in the branch. I can rename a file to b.moved, no complaint either.
Whereas if I first add a.moved and then do "bzr ignore '*.moved' then I get a warning:
[INS 15:46 /tmp/weirdbug/br3 $] bzr ignore '*.moved'
Warning: the following files are version controlled and match your ignore pattern:
a.moved

Why don't "bzr add" and "bzr mv" just loudly fail if the new file name should be ignored according to ignore rules?

===

So the immediate answer is that ignore, consistently with cvs and svn,
the ignore patterns only determine whether unversioned files are treated
as unknown or ignored. The idea is that people may sometimes want to
add exceptions, and in any case it may be possible that a previously
added file will later match an ignore pattern.

It does seem reasonable that we could upgrade the warning given by 'bzr
add' to be a failure, at least if there is an option to override it, and
add a check for 'bzr mv' against renames to ignored names.

I filed <https://bugs.launchpad.net/bzr/+bug/308403> for this.

Normally commit will stop with an error if there are any unresolved
conflicts in the tree, and this is intended to prevent accidental
commits of conflict files. Running 'bzr resolve' should remove the
.BASE/.THIS/.OTHER files, and they will not be versioned unless you
explicitly add them.

*.moved files work a bit differently because they are already added.
However, they should be guarded from commit by the conflict list. So
I'd be interested to hear if this is not working in some way, or if
people are forcing bzr resolve on these files, and then committing. If
so, it may be 'bzr resolve' that should give a warning or error.

===

> "How to avoid that people commit .OTHER/.BASE/.THIS/.moved files?

That could be a commit policy implemented via a pre_commit_hook
that will abort the commit if any change looks like an unresolved
conflict...

But since commit already checks that conflicts have been
resolved, I'm afraid people will also work around that hook..

> Out of the 100 MySQL developers who do bzr merges, we
> see that sometimes some accidentally commit
> .OTHER/.BASE/.THIS/.moved files to the
> repository. Most often, *.moved. To excuse them, there
> are sometimes path conflicts which shouldn't be (ok,
> let's not get started on this :) ,

I really think the root cause is that conflicts could (should ?)
be better explained and easier to solve so that people really
solve them and don't commit .OTHER/.BASE/.THIS/.moved files.

If the people committing these files don't realize that they are
making other people life harder, I'm not sure that making *their*
life harder is the solution.

> so .moved files are common. I imagined that I could
> just do bzr ignore *.moved then commit that change and
> push, and it would prevent my colleagues, once they
> pull my change, to ever commit some *.moved files. But
> no. After doing the commit above, I can "touch
> a.moved; bzr add a.moved" and bzr does not complain, I
> can commit a.moved, it gets in the branch. I can
> rename a file to b.moved, no complaint either.
> Whereas if I first add a.moved and then do "bzr ignore
> '*.moved' then I get a warning: [INS 15:46
> /tmp/weirdbug/br3 $] bzr ignore '*.moved' Warning: the
> following files are version controlled and match your
> ignore pattern: a.moved

> Why don't "bzr add" and "bzr mv" just loudly fail if
> the new file name should be ignored according to
> ignore rules?"

That's by design so that ignore rules can be overridden when
needed.

===

Guilhem Bichot (18/12/2008 9:22 AM)

Hello. Thanks for the replies. So what remains mostly is those .moved files; yes it's possible to commit them "by accident"; what happened several times already is that in some bzr criss-cross merges, some non-bzr-expert people get path conflicts which should not be and then just "bzr resolve". In that case, the .moved file stays, and "bzr commit" does not complain.

How about making "bzr resolve" fail (always fail, no option) if the .moved file is still present, giving an informative error message?

In the case at point, it was not a path conflict, it was
"Duplicate paths
---------------

Typical message:

Conflict adding file FILE. Moved existing file to FILE.moved.

Sometimes Bazaar will attempt to create a file using a pathname that has
already been used. The existing file will be renamed to "FILE.moved". If
you wish, you can rename either one of these files, or combine their contents.
When you are satisfied, you can run "bzr resolve FILE" to mark the conflict as
resolved.
"

Related branches

description: updated
Vincent Ladeuil (vila) on 2009-05-16
affects: bazaar (Ubuntu) → bzr (Ubuntu)
Martin Pool (mbp) on 2009-06-18
Changed in bzr:
importance: Undecided → High
status: New → Confirmed
tags: added: mysql
Martin Pool (mbp) wrote :

I'll work on a scheme to avoid this.

Changed in bzr:
assignee: nobody → Martin Pool (mbp)
Martin Pool (mbp) wrote :

Regarding the .BASE/.THIS/.OTHER files, I'm not sure I understand the case where they could end up being committed. If you get a conflict then do

 bzr add foo.c.OTHER
 bzr resolve foo.c
 bzr commit

The resolve command should have deleted foo.c.OTHER so it seems like it'd be pretty hard to end up with them being committed, unless you specially save the file and restore it after resolving the conflicts?

If the .moved file is left behind when resolve succeeds that is a bug.

Martin Pool (mbp) wrote :

So it's possible to generate .moved files or directories if you have a conflict adding a directory because eg two different people have added it. It's possible that both of them will be present and versioned, in which case the scenario can look like this:

 bzr merge ../other
 bzr resolve --all
   (now the .moved file still exists and is versioned)
 bzr commit

resolve --all means to mark everything as resolved regardless of what's in the directory. It's possible people are doing that when they don't know what else to do.

It's also possible that they do have a .moved file and are explicitly adding it, in which case blocking that like in bug 308403 may help. Also, handling of tree shape conflicts could be better in general, both as far as what's marked as a conflict and what needs to be done to resolve them.

What specifically needs to be done to close this support issue? Is warning/blocking add of ignored files enough?

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

Martin Pool wrote:
> Regarding the .BASE/.THIS/.OTHER files, I'm not sure I understand the
> case where they could end up being committed. If you get a conflict
> then do
>
> bzr add foo.c.OTHER
> bzr resolve foo.c
> bzr commit

I don't know if you realize, Martin, but if you get a content conflict
with a deleted file, I believe the new file ends up as a versioned file
named foo.c.OTHER

$ bzr merge ../y
+N a.OTHER
Contents conflict in a
1 conflicts encountered.

jameinel@samus ~/dev/,tmp/x {x}
$ bzr st
added:
  a.OTHER
unknown:
  a.BASE
conflicts:
  Contents conflict in a
pending merge tips: (use -v to see all merge revisions)
  John Arbash Meinel 2009-06-18 bar

Note that there is no "a.THIS" file, because the file was deleted.

One issue is that we just say "Contents conflict" and not "deleted file
was modified", or something that the user can then understand the
conflict. (When discussing this in the past, it required a WT format
bump because we have to add more information to the Conflict structure
at the time of merge, so that 'status' can then report it.)

Anyway, that is a "trivial" way to get a versioned .OTHER. If someone
does "bzr resolve a.OTHER" or "bzr resolve a" (or bzr resolve --all, I
think) it goes away. The file is still marked as added, but the object
on disk is deleted. (Doing touch a.OTHER shows it as added again.)

John
=:->

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

iEYEARECAAYFAko6PHkACgkQJdeBCYSNAAOLqgCgyd+QUbvh/IhIKtkuVvb9XaEW
t1wAoMIRuhnFjKs4DA8XaF2N1pD1KOLH
=Gji2
-----END PGP SIGNATURE-----

No, I didn't think of that case. Thanks for explaining it, John.

This seems like one of those bugs for which you can do either a
smaller or a larger (longer) fix, so I'd like to work out now what
Guilhem wants. The small fix he originally requested is to block adds
of ignored files. The larger work is to generally improve how shape
conflicts are generated, reported and resolved -- this would
definitely be good to do but it's nontrivial.

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

Its worth noting explicitly I think:
- the requested change wouldn't easily prevent the .OTHER files being
versioned, because that is happening in conflict resolution, not via the
'bzr add' codepath.

-Rob

Robert Collins (lifeless) wrote :

Does this need to be private?

Peter Matulis (petermatulis) wrote :

We can make it public if the customer approves.

Aaron Bentley (abentley) wrote :

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

Martin Pool wrote:
> No, I didn't think of that case. Thanks for explaining it, John.
>
> This seems like one of those bugs for which you can do either a
> smaller or a larger (longer) fix, so I'd like to work out now what
> Guilhem wants. The small fix he originally requested is to block adds
> of ignored files.

I wouldn't like that much. If someone specifies that they want to add
an ignored file, why shouldn't we? They have to manually specify it,
(or use a wildcard) after all. A warning would be fine, but if we can
do what the user asks, I think we should.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkpd7I8ACgkQ0F+nu1YWqI0BYACePo/CBXiA2co+e+PkqalWz6Ek
UJwAnA4jaQAHw+LaP8f275H+mq+JbWpO
=1CMp
-----END PGP SIGNATURE-----

2009/7/16 Aaron Bentley <email address hidden>:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Martin Pool wrote:
>> No, I didn't think of that case.  Thanks for explaining it, John.
>>
>> This seems like one of those bugs for which you can do either a
>> smaller or a larger (longer) fix, so I'd like to work out now what
>> Guilhem wants.  The small fix he originally requested is to block adds
>> of ignored files.
>
> I wouldn't like that much.  If someone specifies that they want to add
> an ignored file, why shouldn't we?  They have to manually specify it,
> (or use a wildcard) after all.  A warning would be fine, but if we can
> do what the user asks, I think we should.

It is question that comes up a bit when less-advanced users seem to
run into trouble. I've probably seen "why did that happen" 10+ times
on the list or irc. So you could make a case that it should be
disabled by default and then advanced users can say --no-really.

However, after the rest of the discussion of this bug, it seems that
change would not really fix the problem, so I'm not suggesting we do
it now. The problem seems to be more to do with how tree shape
conflicts are reported and handled.

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

On Wed, 2009-07-15 at 12:17 +0000, Peter Matulis wrote:
> We can make it public if the customer approves.

I'll assume you're enquiring already, but just to be explicit - could
you please enquire?

-Rob

Peter Matulis (petermatulis) wrote :

Robert Collins wrote:
> On Wed, 2009-07-15 at 12:17 +0000, Peter Matulis wrote:
>> We can make it public if the customer approves.
>
> I'll assume you're enquiring already, but just to be explicit - could
> you please enquire?
>
> -Rob
>

The customer is subscribed during bug creation (via 'MySQL and Canonical
Support/Bazaar' team). They're listening.

--
Peter Matulis | GPG 34F740E8
Ubuntu Support Team | Canonical Ltd. (canonical.com)

GuilhemBichot (guilhem-bichot) wrote :

Bug can be made public.
After the multiple explanations, I understand why "bzr ignore" works the way it does. But I'd like to revive one of my comments in this thread, which was:
     How about making "bzr resolve" fail (always fail, no option) if the .moved file is still present,
     giving an informative error message?
To me there is no valid use case where you would want to commit a .moved file...

visibility: private → public
Vincent Ladeuil (vila) wrote :

Another aspect of the same problem can be seen in bug #414589.

The rule should be that bzr should *at least* warn (optionally loudly) if files, generated by bzr for conflict resolution purposes, are added or committed (#414589 is about add only).

Martin Pool (mbp) on 2009-11-24
Changed in bzr:
assignee: Martin Pool (mbp) → nobody
Martin Pool (mbp) on 2009-12-04
tags: added: conflicts
GuilhemBichot (guilhem-bichot) wrote :

Agree with Vincent's comment that it should at least warn.
This is less a burning problem now as our devs are more fluent in bzr than before, so they do sensible things when merging.
Though there will always be beginners.

Vincent Ladeuil (vila) on 2010-02-08
Changed in bzr:
status: Confirmed → In Progress
Changed in bzr:
assignee: nobody → bzr-qa (bzr-qa)
Changed in bzr (Ubuntu):
assignee: nobody → bzr-qa (bzr-qa)
Vincent Ladeuil (vila) on 2010-02-08
Changed in bzr:
assignee: bzr-qa (bzr-qa) → Vincent Ladeuil (vila)
Robert Collins (lifeless) wrote :

Rhett, please only assign bugs to yourself, never to a team unless you manage the time of the people on that team. Assigning - giving responsibility to someone - doesn't make sense unless you control how they spend their time.

Changed in bzr (Ubuntu):
assignee: bzr-qa (bzr-qa) → nobody
Vincent Ladeuil (vila) on 2010-02-11
Changed in bzr:
milestone: none → 2.2.0b1
Vincent Ladeuil (vila) on 2010-02-11
Changed in bzr:
status: In Progress → Fix Released
GuilhemBichot (guilhem-bichot) wrote :

Bonjour Vincent; does your branch cover the case of .moved files?

GuilhemBichot (guilhem-bichot) wrote :

Hello. I have a bzr.dev with Vincent's fix, but don't see the bug as fixed.
First I create a conflict which moves a file:
cd /tmp
rm -rf test
mkdir test
cd test
bzr init br1
cd br1
echo > bar
bzr add bar
bzr commit -m r0
bzr branch . ../br2
echo > foo
bzr add .
bzr commit -m r1
cd ../br2
echo a > foo
bzr add .
bzr commit -m r2
bzr merge ../br1
+N foo
R foo => foo.moved
Conflict adding file foo. Moved existing file to foo.moved.
1 conflicts encountered.

bzr st
added:
  foo
renamed:
  foo => foo.moved
conflicts:
  Conflict adding file foo. Moved existing file to foo.moved.
pending merge tips: (use -v to see all merge revisions)
  Guilhem Bichot 2010-04-28 r1

bzr resolve --all # playing role of a new user not well aware
bzr st
added:
  foo
renamed:
  foo => foo.moved
pending merge tips: (use -v to see all merge revisions)
  Guilhem Bichot 2010-04-28 r1

bzr commit -m bug
Committing to: /tmp/test/br2/
added foo
renamed foo => foo.moved
Committed revision 3.

So I can commit, no warning.
I think I don't understand what has been fixed exactly.
The changelog says
http://doc.bazaar.canonical.com/bzr.dev/en/release-notes/bzr-2.2.0b1.html
"bzr add will not add conflict related files unless explicitly required. (Vincent Ladeuil, #322767, #414589)"
which to me should cover auto-added .moved files.

Maybe something with OTHER/BASE/THIS files instead?
Thanks!

Vincent Ladeuil (vila) wrote :

>>>>> GuilhemBichot <email address hidden> writes:

   > So I can commit, no warning.
    > I think I don't understand what has been fixed exactly.
    > The changelog says
    > http://doc.bazaar.canonical.com/bzr.dev/en/release-notes/bzr-2.2.0b1.html
    > "bzr add will not add conflict related files unless explicitly required. (Vincent Ladeuil, #322767, #414589)"
    > which to me should cover auto-added .moved files.

.moved are not auto-added, they are already versioned.

This type of conflict occurs when two 'foo' files are involved, one is
present in the 'THIS' branch while other is coming from the 'OTHER'
branch, both files are versioned in this case.

    > Maybe something with OTHER/BASE/THIS files instead?

OTHER/BASE/THIS files are created to help resolve the text conflicts.
They are not versioned so they need to be added via 'bzr add'.

The work in progress is still to help people *avoid* doing
'bzr resolve --all'

GuilhemBichot (guilhem-bichot) wrote :

Merci Vincent. A simple question: this bug is "fix released": what has been fixed? Could you please show a concrete scenario, for which the fix makes a difference (and what difference?). I need to look at this because Canonical's Support team would like to be able to close the incident. Thanks!

Vincent Ladeuil (vila) wrote :

What has been fixed is that a warning is now generated if a file helper generated for a text conflict (.OTHER, .BASE, .THIS) was (previsously) added via 'bzr add'.

I.e. bzr don't add this files blindly now, they have to be added explicitly (because we can't *forbid* that).

This should lead to *fewer* (hopefully none) .BASE/.THIS/.OTHER files being mistakenly added.

.moved can't be addressed this way since they are not *added* they are already present.

Vincent Ladeuil (vila) wrote :

See bug #414589 for the rationale

Vincent Ladeuil (vila) wrote :

In other words:

   bzr add

won't add foo.BASE/OTHER/THIS

  bzr add foo.BASE

will add it because the user explicitly request it

Launchpad Janitor (janitor) wrote :

This bug was fixed in the package bzr - 2.2.0-1

---------------
bzr (2.2.0-1) unstable; urgency=low

  * New upstream release.
   + Adds support for setting timestamps to originating revisions.
     Closes: #473450
   + Removes remaining string exception. Closes: #585193, LP: #586926
   + Add C extension to work around Python issue 1628205. LP: #583941,
     Closes: #577110
   + Avoids showing progress bars when --quiet is used. Closes: #542105,
     LP: #320035
   + No longer creates ~/.bazaar as root when run under sudo. LP: #376388
   + 'bzr commit' now supports -p as alternative for --show-diff. LP: #571467
   + 'bzr add' no longer adds .THIS/.BASE/.THEIRS files unless
     explicitly requested. LP: #322767
   + When parsing patch files, Bazaar now supports diff lines before each
     patch. LP: #502076
   + WorkingTrees now no longer requires using signal.signal, so can
     be used in a threaded environment. LP: #521989
   + An assertion error is no longer triggered when pushing to a pre-1.6
     Bazaar server. LP: #528041
  * Bump standards version to 3.9.1.

bzr (2.2.0~b4-1) experimental; urgency=low

  * New upstream beta.
  * Bump standards version to 3.9.0.
  * Drop build dependency on zlib.

bzr (2.2.0~b2-1) experimental; urgency=low

  * New upstream release.
  * Recommend python-launchpadlib. Closes: #568937

bzr (2.1.2-1.1) unstable; urgency=low

  * Non-maintainer upload.
  * Fix access via http_proxy. Closes: #588430
 -- Jelmer Vernooij <email address hidden> Sat, 07 Aug 2010 00:54:52 +0200

Changed in bzr (Ubuntu):
status: New → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers