Can't save default version at admin panel

[anonymous]

At Page admin/ticket/versions, I can't set a default version or clear the default. After click on the button, the site connect to server but no response come back. So it don't save the move.

The Bug is in Trac 1.2 and also 1.2.1

[Jun Omae]

Cannot be reproduced it. Works for me. Please provide system information, trac.ini and trac.log after enabling TracLogging. I guess that is a configuration issue.

[Ryan J Ollos]

Can you set or clear default of milestone, component, priorities, …?

Is the Your changes have been saved message displayed after applying a change?

Seems like this is likely due to incorrect permissions on the trac.ini file. I can reproduce the behavior you describe "no response come back" by making trac.ini unwritable. We might want to improve the behavior when trac.ini cannot be written.

[Ryan J Ollos]

Configuration._write calls wait_for_file_mtime_change, which ends up in an infinite loop when trac.ini is not writable (tested on OSX). touch_file raises [Errno 13] Permission denied, which passes silently and the timestamp of the file is never changed and we remain in an infinite loop.

I considered adding a break statement when writing wait_for_file_mtime_change, but couldn't think of a scenario that would result in an infinite loop. I'm not still not sure the break is necessary, maybe we should re-raise the exception in touch_file, and then present a nice warning to the user that the file was not writable.

[Ryan J Ollos]

Proposed changes: [f88eea2be/rjollos.git].

[Jun Omae]

An IOError (not an OSError) is raised when parent directory is not writable. I think we should add unit tests for touch_file() if possible.

$ /venv/trac/1.0.14/bin/python
Python 2.5.6 (r256:88840, Oct 21 2014, 22:49:55)
[GCC 4.8.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from trac.util import touch_file
>>> touch_file('/cannot-touch-file')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/venv/trac/1.0.14/lib/python2.5/site-packages/trac/util/__init__.py", line 279, in touch_file
    with open(filename, 'ab'):
IOError: [Errno 13] Permission denied: '/cannot-touch-file'

[Ryan J Ollos]

Okay, but if we re-raise OSError when it's not errno.ENOENT, won't the behavior of touch_file be the same regardless of whether it's an IOError or OSError?

[Jun Omae]

Yeah. You're right. I'm considering whether we should document exceptions raised from touch_file and wait_for_file_mtime_change.

Also, I think we could simply trap EnvironmentError rather than OSError.

Python 2.5.6 (r256:88840, Oct 21 2014, 22:49:55)
[GCC 4.8.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> 
>>> isinstance(IOError(), EnvironmentError)
True
>>> isinstance(OSError(), EnvironmentError)
True
>>> 
>>> str(OSError(2, 'missing', '/file.txt'))
"[Errno 2] missing: '/file.txt'"
>>> OSError(2, 'missing', '/file.txt').errno
2
>>> 
>>> str(IOError(2, 'missing', '/file.txt'))
"[Errno 2] missing: '/file.txt'"
>>> IOError(2, 'missing', '/file.txt').errno
2

[Ryan J Ollos]

Replying to Jun Omae:

Yeah. You're right. I'm considering whether we should document exceptions raised from touch_file and wait_for_file_mtime_change.

Yeah, that's a good idea. Feel free to take ownership of this ticket if you want to add changes.

[Ryan J Ollos]

Committed to 1.2-stable in [16052:16053], merged to trunk in [16054:16055]. I'm all for adding more test coverage, but tests that depend on the filesystem permissions haven't worked well for me, so I'm not sure how to approach adding good tests for this. Feel free to add more changes.