Edgewall Software
Modify

Opened 7 years ago

Closed 6 years ago

#11351 closed enhancement (fixed)

Repository admin panel should indicate when repository path is not valid

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Milestone: 1.0.3
Component: admin/web Version:
Severity: normal Keywords:
Cc: Branch:
Release Notes:

Repository admin page indicates when a repository path is invalid by displaying the path in grey with the error in the title.

API Changes:
Internal Changes:

Description

The repository admin page should indicate when repository paths are invalid by graying out the entry. A warning can also be added when a repository path is saved that is not valid.

Attachments (1)

20131119T193453.png (33.1 KB ) - added by Ryan J Ollos 7 years ago.

Download all attachments as: .zip

Change History (18)

by Ryan J Ollos, 7 years ago

Attachment: 20131119T193453.png added

comment:1 by Ryan J Ollos, 7 years ago

At first I had in mind something simple like this (log:rjollos.git:t11351):

However, we might be able to provide more detailed information. In this example the first 3 grayed-out links are due to (1), whereas the fourth grayed-out link is due to (2) (the error messages are shown as tooltips). If we added unique exceptions for the two cases, then we could gray-out the Type when the version control system is not properly enabled, and gray-out the Directory when it doesn't appear to be a version control repository. The exceptions could be something like: UnsupportedConnectorError and InvalidPathError.

  1. Unsupported version control system "svn": Can't find an appropriate component, maybe the corresponding plugin was not enabled?
  2. home/user/Workspace/11351 does not appear to be a Git repository

comment:2 by anonymous, 7 years ago

This would be great enhancement! two remarks, why this would be great:

  1. To me, corresponding to finally found documentation, git repositories don't look like listed above, but have to have the '/.git' subdir given, which is not quite clear from the UI and unusual for git users, isn't it?
  1. Repositories (Repositorys) UI has some strange effects in displaying resp. copy/paste behavior with hidden/ special characters. Explanation about 2.:

I started with some bad file permissions on git repositories leading to "Trac[PyGIT] ERROR: GIT control files missing" erros resp. "TracError: /home/auser/gitrepos/someproject/.git does not appear to be a Git repository" error message on trac-admin repository resync command.

After fixing this I then used the already given repo listings from UI, copied them, deleted the entries and readded them. Thus including some unvisible blanks, where in one case trailing blank was visible, when path was marked for copy and in one case I finally found it just in trac log file. One other effect, immediately visible after pasting path as copy from old entry has been, slahes got converted to <slash><dot> "/." or "/?" when copy it to some editor. This should have alerted me, but I failed with the blanks.

This leads to the misty situation, that only one of 3 repos worked, although all of them had correct permissions and have been created same way.

Finally let me say, I like trac and hope this comment is usefull to somebody!

My setup: Debian jessie(!), german localization, access via FF 26 using incorporated postrges and git packages

Systeminformationen

Paket Version
Trac 1.0.1
Babel 0.9.6
Docutils 0.11
Genshi 0.6 (with speedups)
GIT 1.8.4.3
mod_python 3.3.1
psycopg2 2.4.5
Pygments 1.6
Python 2.7.5+ (default, Sep 17 2013, 15:33:59) [GCC 4.8.1]
pytz 2012c
setuptools 0.6
jQuery 1.7.2

Installierte Plugins

TracAccountManager 0.4.3
Last edited 7 years ago by Jun Omae (previous) (diff)

in reply to:  2 comment:3 by Ryan J Ollos, 7 years ago

Milestone: next-stable-1.0.x1.0.3

Replying to anonymous:

  1. To me, corresponding to finally found documentation, git repositories don't look like listed above, but have to have the '/.git' subdir given, which is not quite clear from the UI and unusual for git users, isn't it?

This issue was addressed recently in #11297.

  1. Repositories (Repositorys) UI has some strange effects in displaying resp. copy/paste behavior with hidden/ special characters.

This will be handled in #11371.

Last edited 7 years ago by Ryan J Ollos (previous) (diff)

in reply to:  2 comment:4 by Ryan J Ollos, 7 years ago

Replying to anonymous:

  1. Repositories (Repositorys) UI has some strange effects in displaying resp. copy/paste behavior with hidden/ special characters.

I experienced today, when copying a path from the table on the /admin/versioncontrol/repository page into trac.ini, the result is: /home/<200b>user/<200b>Workspace/<200b>th10834/<200b>teo-rjollos.git.

comment:5 by Remy Blank, 7 years ago

FTR, these are zero-width spaces that I put there intentionally so that long paths are broken properly. I don't remember the exact details, but removing them either made the table larger than the page, or possibly the breaks were in unfortunate locations.

We already work around the copy-paste problem by removing zero-width spaces from certain input fields on the server side, maybe we should just remove them from all inputs. Or maybe there's a better way to solve the path-breaking problem that doesn't need adding special characters to the text.

comment:6 by Ryan J Ollos, 7 years ago

Stripping the zero-width spaces from input fields could be done as part of #11371, and then possibly extended to other realms.

comment:7 by Ryan J Ollos, 6 years ago

Status: newassigned

comment:8 by Ryan J Ollos, 6 years ago

Proposed changes in log:rjollos.git:t11351.2.

in reply to:  1 comment:9 by Ryan J Ollos, 6 years ago

Replying to rjollos:

However, we might be able to provide more detailed information. In this example the first 3 grayed-out links are due to (1), whereas the fourth grayed-out link is due to (2) (the error messages are shown as tooltips). If we added unique exceptions for the two cases, then we could gray-out the Type when the version control system is not properly enabled, and gray-out the Directory when it doesn't appear to be a version control repository. The exceptions could be something like: UnsupportedConnectorError and InvalidPathError.

  1. Unsupported version control system "svn": Can't find an appropriate component, maybe the corresponding plugin was not enabled?
  2. home/user/Workspace/11351 does not appear to be a Git repository

Providing detailed information on the repository admin page for the error described in (1) will probably be addressed in #11713.

Last edited 6 years ago by Ryan J Ollos (previous) (diff)

comment:10 by Ryan J Ollos, 6 years ago

Release Notes: modified (diff)

Committed to 1.0-stable in [13605] and merged to trunk in [13606]. Documentation updated in TracDev/Exceptions@12.

Next I'll prepare a patch for the MercurialPlugin.

comment:11 by Jun Omae, 6 years ago

Test failures on autobuild after [13605]. See https://travis-ci.org/edgewall/trac/jobs/45758513. If Subversion binding is not installed, the tests fail.

$ TMP=/dev/shm/tmp make python=25-1.0 clean Trac.egg-info all test=trac/versioncontrol/tests/functional.py
Python: /home/jun66j5/venv/py25-1.0/bin/python
find -name \*.py[co] -exec rm {} \;
rm -f .figleaf* *.figleaf
rm -fr figleaf
coverage erase
rm -fr build/doc

Python: /home/jun66j5/venv/py25-1.0/bin/python

  Package        Version
  ---------------------------------------------------------------------
  Python       : 2.5.6 (r256:88840, Jan 25 2014, 17:35:34)
  [GCC 4.6.3]
  Setuptools   : 0.6
  Genshi       : 0.7
  Babel        : 0.9.6
  sqlite3      : 2.3.2
  PySqlite     : not installed
  MySQLdb      : 1.2.5
  Psycopg2     : 2.5.2 (dt dec pq3 ext)
  SVN bindings : not installed
  Mercurial    : 3.0.1
  Pygments     : 1.6
  Pytz         : 2014.4
  ConfigObj    : 4.7.2
  Docutils     : 0.11
  Twill        : 0.9
  LXML         : 3.3.5
  coverage     : not installed
  figleaf      : not installed

Variables:
  PATH=/home/jun66j5/venv/py25-1.0/bin::$PATH
  PYTHONPATH=.::$PYTHONPATH
  TRAC_TEST_DB_URI=
  server-options= -p 3000 -a '*,/home/jun66j5/src/trac-htdigest.txt,auth'  -e

External dependencies:
  Git version: git version 1.7.9.5

...
python trac/versioncontrol/tests/functional.py
SKIP: versioncontrol/tests/functional.py (no svn bindings)
FFF
...
----------------------------------------------------------------------
Ran 3 tests in 15.111s

FAILED (failures=3)
make: *** [all] Error 1

comment:12 by Jun Omae, 6 years ago

If repository type is not supported, a TracError would be raised. I think we should raise an InvalidRepository for also that case.

Another thing, there are raw U+200B characters in literal of test cases. I think we should prevent using such characters in literal.

  • trac/versioncontrol/api.py

    diff --git a/trac/versioncontrol/api.py b/trac/versioncontrol/api.py
    index ce11e26..20e0572 100644
    a b class RepositoryManager(Component):  
    760760            if prio >= 0: # no error condition
    761761                return connector
    762762            else:
    763                 raise TracError(
     763                raise InvalidRepository(
    764764                    _('Unsupported version control system "%(name)s"'
    765765                      ': %(error)s', name=rtype,
    766766                      error=to_unicode(connector.error)))
    767767        else:
    768             raise TracError(
     768            raise InvalidRepository(
    769769                _('Unsupported version control system "%(name)s": '
    770770                  'Can\'t find an appropriate component, maybe the '
    771771                  'corresponding plugin was not enabled? ', name=rtype))
  • trac/versioncontrol/tests/functional.py

    diff --git a/trac/versioncontrol/tests/functional.py b/trac/versioncontrol/tests/functional.py
    index fb67fec..50d96dc 100755
    a b class TestAdminInvalidRepository(FunctionalTwillTestCaseSetup):  
    3535        tc.formvalue('trac-addrepos', 'name', 'InvalidRepos')
    3636        tc.formvalue('trac-addrepos', 'dir', '/the/invalid/path')
    3737        tc.submit()
    38         tc.find('<span class="missing" title="/the/invalid/path does not '
    39                 'appear to be a Subversion repository.">/the/​invalid/​path'
    40                 '</span>')
     38        tc.find((u'<span class="missing" title="[^"]*">'
     39                 u'/the/\u200binvalid/\u200bpath</span>').encode('utf-8'))
    4140
    4241
    4342class TestEmptySvnRepo(FunctionalTwillTestCaseSetup):

in reply to:  12 comment:13 by Ryan J Ollos, 6 years ago

Replying to jomae:

If repository type is not supported, a TracError would be raised. I think we should raise an InvalidRepository for also that case.

I described some points to consider with that change in comment:9.

comment:14 by Ryan J Ollos, 6 years ago

With a failing test I was seeing the following:

======================================================================
ERROR: Repository with an invalid path is rendered with an error
----------------------------------------------------------------------
Traceback (most recent call last):
  File "trac/versioncontrol/tests/functional.py", line 38, in runTest
    tc.find('<span class="missing" title="/the/invalid/path does not '
  File "/home/user/Workspace/t11944/teo-rjollos.git/trac/tests/functional/better_twill.py", line 227, in better_find
    (unicode(e), filename))
UnicodeDecodeError: 'ascii' codec can't decode byte 0xe2 in position 113: ordinal not in range(128)

After this patch, the traceback prints correctly.

  • trac/tests/functional/better_twill.py

    diff --git a/trac/tests/functional/better_twill.py b/trac/tests/functional/bette
    index f9314a6..a630e24 100755
    a b try:  
    2727except ImportError:
    2828    from StringIO import StringIO
    2929
     30from trac.util.text import to_unicode
     31
    3032# On OSX lxml needs to be imported before twill to avoid Resolver issues
    3133# somehow caused by the mac specific 'ic' module
    3234try:
    if twill:  
    224226        except twill.errors.TwillAssertionError, e:
    225227            filename = twill_write_html()
    226228            raise twill.errors.TwillAssertionError('%s at %s' %
    227                                                    (unicode(e), filename))
     229                                                   (to_unicode(e), filename))
    228230    tc.find = better_find
    229231
    230232    def better_notfind(what, flags='', tcnotfind=tc.notfind):
    if twill:  
    233235        except twill.errors.TwillAssertionError, e:
    234236            filename = twill_write_html()
    235237            raise twill.errors.TwillAssertionError('%s at %s' %
    236                                                    (unicode(e), filename))
     238                                                   (to_unicode(e), filename))
    237239    tc.notfind = better_notfind
    238240
    239241    # Same for tc.url - no hint about what went wrong!
    if twill:  
    243245        except twill.errors.TwillAssertionError, e:
    244246            filename = twill_write_html()
    245247            raise twill.errors.TwillAssertionError('%s at %s' %
    246                                                    (unicode(e), filename))
     248                                                   (to_unicode(e), filename))
    247249    tc.url = better_url
    248250else:
    249251    b = tc = None
======================================================================
FAIL: Repository with an invalid path is rendered with an error
----------------------------------------------------------------------
Traceback (most recent call last):
  File "trac/versioncontrol/tests/functional.py", line 39, in runTest
    u'/the/\u200binvalid/\u200bpath</span>').encode('utf-8'))
  File "/home/user/Workspace/t11944/teo-rjollos.git/trac/tests/functional/better_twill.py", line 230, in better_find
    (to_unicode(e), filename))
TwillAssertionError: no match to '<span class="missing" title="[^"]*">/the/\u200binvalid/\u200bpath</span>' at file:///home/user/Workspace/t11944/teo-rjollos.git/testenv/trac/log/TestAdminInvalidRepository.html

Does the change make sense?

comment:15 by Ryan J Ollos, 6 years ago

Changes committed to 1.0-stable in [13613], merged to trunk in [13614]. I'll prepare additional changes in #11713.

comment:16 by Ryan J Ollos, 6 years ago

Patch from comment:14 committed to 1.0-stable in [13641], merged to trunk in [13642].

in reply to:  10 comment:17 by Ryan J Ollos, 6 years ago

Resolution: fixed
Status: assignedclosed

Replying to rjollos:

Next I'll prepare a patch for the MercurialPlugin.

Fixed in [41/mercurial-plugin].

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Ryan J Ollos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.