qlog/qdiff: "internal error" on diff encoding change

[cetus]

Now I run bzr qlog and open diff window at top log record.

In diff window I select "View Options/left side encoding/cp1251" (because source has cp1251 text) and got error with this crash log:

======
bzr: ERROR: bzrlib.errors.ReadOnlyError: A write attempt was made in a read only transaction on CHKInventoryRepository('file:///home/cetus/Projects/ksu-tests/Ksu_Ra-3.x.bzr/.bzr/repository/')

Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/bzrlib/plugins/qbzr/lib/encoding_selector.py", line 182, in triggered
    self._encodingChanged(encoding)
  File "/usr/lib/python2.7/site-packages/bzrlib/plugins/qbzr/lib/encoding_selector.py", line 73, in _encodingChanged
    self.onChanged(encoding)
  File "/usr/lib/python2.7/site-packages/bzrlib/plugins/qbzr/lib/diffwindow.py", line 313, in on_left_encoding_changed
    get_set_encoding(encoding, self.branches[0])
  File "/usr/lib/python2.7/site-packages/bzrlib/plugins/qbzr/lib/util.py", line 640, in get_set_encoding
    branch.get_config().set_user_option("encoding", encoding)
  File "/usr/lib/python2.7/site-packages/bzrlib/config.py", line 1450, in set_user_option
    self._get_branch_data_config().set_option(value, name)
  File "/usr/lib/python2.7/site-packages/bzrlib/config.py", line 1753, in set_option
    self.branch.lock_write()
  File "/usr/lib/python2.7/site-packages/bzrlib/branch.py", line 2523, in lock_write
    self.repository.lock_write()
  File "/usr/lib/python2.7/site-packages/bzrlib/repofmt/pack_repo.py", line 1771, in lock_write
    raise errors.ReadOnlyError(self)
ReadOnlyError: A write attempt was made in a read only transaction on CHKInventoryRepository('file:///home/cetus/Projects/ksu-tests/Ksu_Ra-3.x.bzr/.bzr/repository/')

bzr 2.5.0 on python 2.7.3 (linux2)
arguments: ['/usr/bin/bzr', 'qlog']
encoding: 'utf-8', fsenc: 'UTF-8', lang: 'ru_RU.UTF-8'
plugins:
  bash_completion /usr/lib/python2.7/site-packages/bzrlib/plugins/bash_completion [2.5.0]
  bzrtools /usr/lib/python2.7/site-packages/bzrlib/plugins/bzrtools [2.5.0]
  changelog_merge /usr/lib/python2.7/site-packages/bzrlib/plugins/changelog_merge [2.5.0]
  cvsps_import /home/cetus/.bazaar/plugins/cvsps_import [unknown]
  fastimport /usr/lib/python2.7/site-packages/bzrlib/plugins/fastimport [0.13.0]
  gtk /usr/lib/python2.7/site-packages/bzrlib/plugins/gtk [0.100.0]
  launchpad /usr/lib/python2.7/site-packages/bzrlib/plugins/launchpad [2.5.0]
  netrc_credential_store /usr/lib/python2.7/site-packages/bzrlib/plugins/netrc_credential_store [2.5.0]
  news_merge /usr/lib/python2.7/site-packages/bzrlib/plugins/news_merge [2.5.0]
  po_merge /usr/lib/python2.7/site-packages/bzrlib/plugins/po_merge [2.5.0]
  qbzr /usr/lib/python2.7/site-packages/bzrlib/plugins/qbzr [0.22.2]
  rewrite /usr/lib/python2.7/site-packages/bzrlib/plugins/rewrite [0.6.3]
  svn /usr/lib/python2.7/site-packages/bzrlib/plugins/svn [1.2.1]
  weave_fmt /usr/lib/python2.7/site-packages/bzrlib/plugins/weave_fmt [2.5.0]
========

Also, equivalence "bzr qdiff -r NNN" craches with:

========
bzr: ERROR: bzrlib.errors.ReadOnlyError: A write attempt was made in a read only transaction on LockableFiles(lock, file:///home/cetus/Projects/ksu-tests/Ksu_Ra-3.x.bzr/.bzr/branch/)

Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/bzrlib/plugins/qbzr/lib/encoding_selector.py", line 182, in triggered
    self._encodingChanged(encoding)
  File "/usr/lib/python2.7/site-packages/bzrlib/plugins/qbzr/lib/encoding_selector.py", line 73, in _encodingChanged
    self.onChanged(encoding)
  File "/usr/lib/python2.7/site-packages/bzrlib/plugins/qbzr/lib/diffwindow.py", line 313, in on_left_encoding_changed
    get_set_encoding(encoding, self.branches[0])
  File "/usr/lib/python2.7/site-packages/bzrlib/plugins/qbzr/lib/util.py", line 640, in get_set_encoding
    branch.get_config().set_user_option("encoding", encoding)
  File "/usr/lib/python2.7/site-packages/bzrlib/config.py", line 1450, in set_user_option
    self._get_branch_data_config().set_option(value, name)
  File "/usr/lib/python2.7/site-packages/bzrlib/config.py", line 1753, in set_option
    self.branch.lock_write()
  File "/usr/lib/python2.7/site-packages/bzrlib/branch.py", line 2529, in lock_write
    self.control_files.lock_write(token=token))
  File "/usr/lib/python2.7/site-packages/bzrlib/lockable_files.py", line 159, in lock_write
    raise errors.ReadOnlyError(self)
ReadOnlyError: A write attempt was made in a read only transaction on LockableFiles(lock, file:///home/cetus/Projects/ksu-tests/Ksu_Ra-3.x.bzr/.bzr/branch/)

bzr 2.5.0 on python 2.7.3 (linux2)
arguments: ['/usr/bin/bzr', 'qdiff', '-r', '440']
encoding: 'utf-8', fsenc: 'UTF-8', lang: 'ru_RU.UTF-8'
...
======

[Alexander Belchenko (bialix)]

I wonder why did you get Read Only error. Is it possible that your branch/repository is marked read-only or reside on read-only filesystem?

[Alexander Belchenko (bialix)]

It should be easy to fix by suppressing ReadOnlyError in qbzr/lib.util.py get_set_* functions in the following manner:

try:
            branch.get_config().set_user_option("encoding", encoding)
except errors.ReadOnlyError:
            pass

[Alexander Belchenko (bialix)]

I'd like to get some help from core bzr devs: why can we get ReadOnlyError when we're working with file:// URLs? Is bzr able to detect read-only storages or something like that?

[cetus (cetus-newmail)]

Ok, thank you!

I fix get_set_encoding function in qbzr/lib/util.py (using tag:release-0.22.2 of qbzr)

    else:
        if branch: # we should check boolean branch value to support 2 fake branch cases: branch is None, branch is FakeBranch
            try:
                branch.get_config().set_user_option("encoding", encoding)
            except errors.ReadOnlyError:
                pass
    return encoding

It works, but I got SIGSEGV randomly. Especially when I change encoding twice: to cp1251, then back to utf8.

What means "branch/repository is marked read-only" ?
My filesystem is writable, and branch (as branch) used to work with svn repository (copy of trunk).

[Alexander Belchenko (bialix)]

cetus пишет:
> Ok, thank you!
>
> I fix get_set_encoding function in qbzr/lib/util.py (using
> tag:release-0.22.2 of qbzr)
>
> else:
> if branch: # we should check boolean branch value to support 2 fake branch cases: branch is None, branch is FakeBranch
> try:
> branch.get_config().set_user_option("encoding", encoding)
> except errors.ReadOnlyError:
> pass
> return encoding

That's right patch.

> It works, but I got SIGSEGV randomly. Especially when I change encoding
> twice: to cp1251, then back to utf8.

I suspect it's because you did it too fast in a row. Each time you
change encoding or tab settings or different option, qdiff has to reload
diff and re-code it according to new settings. You'd better wait some
time until the full diff will be loaded, before making new change. This
is poor-man workaround :-/

> What means "branch/repository is marked read-only" ?

I meant: maybe your branch or your repository has read-only bit set in
filesystem properties?

> My filesystem is writable, and branch (as branch) used to work with svn repository (copy of trunk).

That puzzles me a lot. I don't understand why did you get this error at
all. Is it a real bzr branch or real svn workspace which you have
accessed with bzr (via bzr-svn support)? What `bzr info` says?

--
All the dude wanted was his rug back

[cetus (cetus-newmail)]

> That puzzles me a lot. I don't understand why did you get this error at
> all. Is it a real bzr branch or real svn workspace which you have
> accessed with bzr (via bzr-svn support)? What `bzr info` says?

$ bzr info
Standalone tree (format: 2a)
Location:
  branch root: .

Related branches:
    push branch: svn+http://cetus:mypasswd@ksu/svn/Ksu_Ra-3.x/trunk
  parent branch: svn+http://cetus:mypasswd@ksu/svn/Ksu_Ra-3.x/trunk

--

[cetus (cetus-newmail)]

On Fri, 27 Apr 2012 10:57:01 -0000
Alexander Belchenko wrote:

> I suspect it's because you did it too fast in a row. Each time you

I think that there is a true reason.

> change encoding or tab settings or different option, qdiff has to reload
> diff and re-code it according to new settings. You'd better wait some
> time until the full diff will be loaded, before making new change. This
> is poor-man workaround :-/

Original qbzr 0.22.2 does not crashes after complete diff loading.

[Alexander Belchenko (bialix)]

cetus пишет:
> On Fri, 27 Apr 2012 10:57:01 -0000
> Alexander Belchenko wrote:
>
>> I suspect it's because you did it too fast in a row. Each time you
>
> I think that there is a true reason.
>
>> change encoding or tab settings or different option, qdiff has to reload
>> diff and re-code it according to new settings. You'd better wait some
>> time until the full diff will be loaded, before making new change. This
>> is poor-man workaround :-/
>
> Original qbzr 0.22.2 does not crashes after complete diff loading.

Your patched version won't crash either.

--
All the dude wanted was his rug back

[cetus (cetus-newmail)]

No. My patched version dies with 'segmentation fault' while stable version continues with error window. It lets me to 'report a bug', 'Show more details', 'Ignore' and 'Close Application'. But I need only wait until full diff will be loaded.

[Alexander Belchenko (bialix)]

Your npatched version simply don't do everything that patched does. Simply because error window prevents it to work the full way.

[André Bachmann (andrebachmann-dd)]

Yesterday a coworker had the same problem with Bazar 2.5.1-1 (with qbzr trunk) and Windows 7. He was viewing a diff (qlog and qdiff) of the last two revisions of a branch which is located on a network share. Then he changed the encoding on the left side to latin-1 - this worked. Now he changed the encoding on the right side also to latin-1, and the ReadOnlyError appears.

============================================================================================
30.291 couldn't find apport bug-reporting library: No module named apport
30.374 Traceback (most recent call last):
  File "C:/Users/bachmann/AppData/Roaming/bazaar/2.0/plugins\qbzr\lib\encoding_selector.py", line 183, in triggered
  File "C:/Users/bachmann/AppData/Roaming/bazaar/2.0/plugins\qbzr\lib\encoding_selector.py", line 73, in _encodingChanged
  File "C:/Users/bachmann/AppData/Roaming/bazaar/2.0/plugins\qbzr\lib\diffwindow.py", line 326, in on_right_encoding_changed
  File "C:/Users/bachmann/AppData/Roaming/bazaar/2.0/plugins\qbzr\lib\util.py", line 676, in get_set_encoding
  File "bzrlib\config.pyo", line 1450, in set_user_option
  File "bzrlib\config.pyo", line 1753, in set_option
  File "bzrlib\branch.pyo", line 2523, in lock_write
  File "bzrlib\repofmt\pack_repo.pyo", line 1771, in lock_write
ReadOnlyError: A write attempt was made in a read only transaction on CHKInventoryRepository('file:///W:/Controller-Repository/10035_blubb/.bzr/repository/')

30.376 No Apport available to Bazaar
============================================================================================

This rises a question: If the encoding is saved per branch, why is it possible to change the encoding twice to latin-1 when I watch two diffs on the same branch (with different revisions)? A possible workaround might be: If the user changes the encoding on one side, the other side should automatically refresh the view with the new encoding. So it won't be necessary to change the encoding twice in a short time (if this is really the problem).

[Alexander Belchenko (bialix)]

André Bachmann пишет:
> This rises a question: If the encoding is saved per branch, why is it
> possible to change the encoding twice to latin-1 when I watch two diffs
> on the same branch (with different revisions)? A possible workaround
> might be: If the user changes the encoding on one side, the other side
> should automatically refresh the view with the new encoding. So it won't
> be necessary to change the encoding twice in a short time (if this is
> really the problem).

André, we've tried to support [a rare] use case when you have to compare
file in different encodings. For example, some old version of file used
latin-1 and later project switched to use utf-8. Of course in this case
those 2 versions will be seen as [almost completely] different by bzr,
but at least you'll get the idea what was the file content. The things
are much worse for Cyrillic (Russian text could be in 6 different
encodings including utf-8, 4 of them are still in use) or for CJK.

So, I think the viable workaround is to catch that Error and show a
humble notification about inability to save the encoding, but actually
change it in the qdiff window. Another (orthogonal) option is to provide
a knob in Diff-Options to switch between "one encoding for left and
right", "different encodings for left and right". But I think the first
workaround will actually solve most of the pain from this error. That
should be an easy fix.

--
All the dude wanted was his rug back

[André Bachmann (andrebachmann-dd)]

I had an idea about this: We could circumvent the problem if we don't try to save the specific encoding if it is already set for this branch.
This would lead to the following scenario: A user opens the qdiff window and changes the encoding on either the left or the right side from type0 to type1. Before we write the selected encoding to the branch's configuration, we check if it is the same. Then we only write it if the selected encoding isn't the same as the one from the file.
So this wouldn't change at all the user's experience (and also preserves the mentioned rare use case), but prevents the ReadOnlyError.
What do you think of this?