Edgewall Software

Ticket #1830 (closed defect: fixed)

Opened 3 years ago

Last modified 2 years ago

Repository subsets have to be fully self contained

Reported by: jonas Owned by: cboos
Priority: normal Milestone: 0.10
Component: version control Version: devel
Severity: normal Keywords:
Cc: gkokanosky@…, ndelon@…, dserodio@…, trac.tickets@…, calvin@…, jwilliams@…, trac@…, johannes_rudolph@…

Description

When Trac is configured to use a repository subset it has to be fully self contained. It can not contain any resources copied or moved from parts of the repository that are outside of the configured subset.

See #586 for more information.

Attachments

trac-rev.diff (1.4 kB) - added by ndelon@… 3 years ago.
patch for copy/move in subset bug
trac-bug-1830.sh (0.8 kB) - added by John Williams <jwilliams@…> 2 years ago.
Minimal test case of subset repo problem to demonstrate the problem.

Change History

Changed 3 years ago by anonymous

  • cc gkokanosky@… added

Changed 3 years ago by anonymous

  • cc ndelon@… added

Changed 3 years ago by ndelon@…

Here is a patch that can solve this issue:

=== trac/versioncontrol/cache.py
==================================================================
--- trac/versioncontrol/cache.py  (revision 145)
+++ trac/versioncontrol/cache.py  (local)
@@ -46,8 +46,11 @@
         if not self.synced:
             self.sync()
             self.synced = 1
-        return CachedChangeset(self.repos.normalize_rev(rev), self.db,
-                               self.authz)
+        try:
+            return CachedChangeset(self.repos.normalize_rev(rev), self.db,
+                                   self.authz)
+        except TracError:
+            return self.repos.get_changeset(rev)

     def sync(self):
         self.log.debug("Checking whether sync with repository is needed")
=== trac/versioncontrol/svn_fs.py
==================================================================
--- trac/versioncontrol/svn_fs.py  (revision 145)
+++ trac/versioncontrol/svn_fs.py  (local)
@@ -230,7 +230,7 @@
         rev = int(rev)
         if rev == 0:
             return None
-        if self.scope == '/':
+        if self.scope == '/' or not rev in self.history:
             return rev - 1
         idx = self.history.index(rev)
         if idx + 1 < len(self.history):
@@ -241,7 +241,7 @@
         rev = int(rev)
         if rev == self.rev:
             return None
-        if self.scope == '/':
+        if self.scope == '/' or not rev in self.history:
             return rev + 1
         if rev == 0:
             return self.oldest_rev
  • trac/versioncontrol/cache.py: the CachedRepository stores all revisions contained in a given subset of a repository, but not revisions that are outside of this subset scope, that's why we try to fetch revisions directly from the repository if they can't be found in the database (but those rev are not putted back in the cache, shall they ?).
  • trac/versioncontrol/svn_fs.py: if the methods previous_rev and next_rev of the SubversionRepository class get a rev number that is not in the history (that store all the revisions of a subset if trac is handling a specific subset) we consider this revision is out of the subset

Despite this patch seems to work correctly, I'm not sure that this the good way to fix the copy/move issue.

Changed 3 years ago by ndelon@…

patch for copy/move in subset bug

Changed 3 years ago by Gunnar Wagenknecht <gunnar@…>

  • cc gunnar@… added

Changed 3 years ago by cmlenz

#1930 has been marked as duplicate of this ticket.

Changed 3 years ago by anonymous

  • cc dserodio@… added

Changed 3 years ago by anonymous

What's holding this patch from being applied to the trunk? Any pitfalls?

Changed 3 years ago by anonymous

  • cc trac.tickets@… added

Changed 3 years ago by dserodio@…

The attached patch didn't solve my problem: my project was renamed, so the whole "repository subset" was moved.

Changed 3 years ago by anonymous

  • cc calvin@… added

Changed 3 years ago by cmlenz

  • owner changed from jonas to cmlenz
  • component changed from general to version control

Changed 3 years ago by dserodio@…

r2865 finally fixed this problem for me.

Changed 3 years ago by cboos

  • owner changed from cmlenz to cboos
  • milestone set to 0.10

Perhaps, but that's far from being enough...

Our own test repos can be used to exercise this situation, when repository_dir uses the tags/v1 for its scope. The only available changeset is 7, and the browser view and the changeset view will fail (the revision log will work, though).

Changed 3 years ago by dserodio@…

Damn, I spoke too soon. To workaround this bug, I had set the problematic project's repository_dir to the repository root. Now that I've pointed it back to a subset of the repository, it's happening again, both in the Changeset view and in the Browser view too.
In the Changeset view, the error is:

Traceback (most recent call last):
  File "/usr/lib/python2.3/site-packages/trac/web/modpython_frontend.py", line 208, in handler
    dispatch_request(mpr.path_info, mpr, env)
  File "/usr/lib/python2.3/site-packages/trac/web/main.py", line 141, in dispatch_request
    dispatcher.dispatch(req)
  File "/usr/lib/python2.3/site-packages/trac/web/main.py", line 109, in dispatch
    resp = chosen_handler.process_request(req)
  File "/usr/lib/python2.3/site-packages/trac/versioncontrol/web_ui/changeset.py", line 216, in process_request
    diff_args, diff_options)
  File "/usr/lib/python2.3/site-packages/trac/versioncontrol/web_ui/changeset.py", line 425, in _render_html
    diffs = _content_changes(old_node, new_node)
  File "/usr/lib/python2.3/site-packages/trac/versioncontrol/web_ui/changeset.py", line 382, in _content_changes
    data = old_node.get_content().read()
AttributeError: 'NoneType' object has no attribute 'read'

and in the Browser view:

No changeset 1162 in the repository

I'm using r2865

Changed 3 years ago by anonymous

  • cc jwilliams@… added

Changed 3 years ago by Curtis Buys <trac@…>

  • cc trac@… added

Changed 3 years ago by anonymous

  • cc trac@… added; trac@… removed

Changed 3 years ago by cboos

  • status changed from new to assigned

The support for this feature made some progress: Please test r2992.

That changeset is on the VcRefactoring branch, but the corresponding diff should also apply cleanly on the source:trunk@2991.

Changed 2 years ago by John Williams <jwilliams@…>

I tried this, as suggested merging the diff from the VcRefactoring branch onto source:trunk@2991

Applied cleanly, testing with tracd.

Point it at the test repo that is created by my script posted to trac list in February:

http://lists.edgewall.com/archive/trac/2006-February/006655.html

(trac-bug-1830.sh script added as attachment)

If I point svn_repository to repo/subdir, I can at least browse to the (virtual) root - this is an improvement on before.

However, attempt to browse to a subdirectory within the subset repo, I get the following oops:

Traceback (most recent call last):
  File "/home/jwilliams/tmp/trac-test/trac-install/lib/python2.3/site-packages/trac/web/main.py", line 283, in dispatch_request
    dispatcher.dispatch(req)
  File "/home/jwilliams/tmp/trac-test/trac-install/lib/python2.3/site-packages/trac/web/main.py", line 170, in dispatch
    resp = chosen_handler.process_request(req)
  File "/home/jwilliams/tmp/trac-test/trac-install/lib/python2.3/site-packages/trac/versioncontrol/web_ui/browser.py", line 106, in process_request
    self._render_directory(req, repos, node, rev)
  File "/home/jwilliams/tmp/trac-test/trac-install/lib/python2.3/site-packages/trac/versioncontrol/web_ui/browser.py", line 137, in _render_directory
    changes = get_changes(self.env, repos, [i['rev'] for i in info])
  File "/home/jwilliams/tmp/trac-test/trac-install/lib/python2.3/site-packages/trac/versioncontrol/web_ui/util.py", line 36, in get_changes
    except NoSuchChangeset:
NameError: global name 'NoSuchChangeset' is not defined

Changed 2 years ago by John Williams <jwilliams@…>

Minimal test case of subset repo problem to demonstrate the problem.

Changed 2 years ago by cboos

oops, you need the small fix of r3009.

I'll try also with your repo later.

Changed 2 years ago by cboos

Browsing the test repo works fine now, but for that you'll need to upgrade your trunk to r3010, which contains an important fix for scoped repositories.

Changed 2 years ago by John Williams

Hmm, still not working for me - maybe I'm missing something. Here's what I tried

Clean test of trunk@3010:

$ svn update -r3010
$ svn revert -R *
$ rm -rf /home/jwilliams/tmp/trac-test/trac-install
$ python ./setup install --prefix=/home/jwilliams/tmp/trac-test/trac-install
$ export PATH=/home/jwilliams/tmp/trac-test/trac-install/bin:${PATH}
$ export PYTHONPATH=/home/jwilliams/tmp/trac-test/trac-install/lib/python2.3/site-packages:${PYTHONPATH}
$ svn-admin /home/jwilliams/trac-test/testrepo resync
$ tracd -p 10000 /home/jwilliams/tmp/trac-test/trac-instance

With trac.ini pointing to test-repo/subdir, when I attempt to browse 'b', I get

Internal Error

No changeset 3 in the repository

If merge the changes in the VcRefactoring branch, r2991:2992, and repeat, I get the following oops:

Traceback (most recent call last):
  File "/home/jwilliams/tmp/trac-test/trac-install/lib/python2.3/site-packages/trac/web/main.py", line 283, in dispatch_request
    dispatcher.dispatch(req)
  File "/home/jwilliams/tmp/trac-test/trac-install/lib/python2.3/site-packages/trac/web/main.py", line 170, in dispatch
    resp = chosen_handler.process_request(req)
  File "/home/jwilliams/tmp/trac-test/trac-install/lib/python2.3/site-packages/trac/versioncontrol/web_ui/browser.py", line 106, in process_request
    self._render_directory(req, repos, node, rev)
  File "/home/jwilliams/tmp/trac-test/trac-install/lib/python2.3/site-packages/trac/versioncontrol/web_ui/browser.py", line 137, in _render_directory
    changes = get_changes(self.env, repos, [i['rev'] for i in info])
  File "/home/jwilliams/tmp/trac-test/trac-install/lib/python2.3/site-packages/trac/versioncontrol/web_ui/util.py", line 36, in get_changes
    except NoSuchChangeset:
NameError: global name 'NoSuchChangeset' is not defined

Can you confirm, whether simply r3010 on trunk should work, or if it's r3010 on trunk plus r2991:2992 on VcRefactoring that's required?

Here is an excerpt of my trac.ini file, for reference:

[trac]
default_charset = iso-8859-15
ignore_auth_case = false
permission_store = DefaultPermissionStore
check_auth_ip = true
database = sqlite:db/trac.db
templates_dir = /opt/local/share/trac/templates
default_handler = WikiModule
metanav = login,logout,settings,help,about
mainnav = wiki,timeline,roadmap,browser,tickets,newticket,search
repository_type = svn
repository_dir = /home/jwilliams/trac-test/test-repo/subdir
}

Changed 2 years ago by John Williams

trac.ini excerpt, previewed this time!

[trac]
default_charset = iso-8859-15
ignore_auth_case = false
permission_store = DefaultPermissionStore
check_auth_ip = true
database = sqlite:db/trac.db
templates_dir = /opt/local/share/trac/templates
default_handler = WikiModule
metanav = login,logout,settings,help,about
mainnav = wiki,timeline,roadmap,browser,tickets,newticket,search
repository_type = svn
repository_dir = /home/jwilliams/trac-test/test-repo/subdir

Changed 2 years ago by cboos

Sorry, the instructions were not clear enough:

Note: that diff: link will work when this trac will be switched to 0.10)

Changed 2 years ago by John Williams

Fantastic! Works beautifully on the test repo and also our production repo.

Thanks for your efforts - much appreciated.

Changed 2 years ago by cboos

  • status changed from assigned to closed
  • resolution set to fixed

Merged in r3015. For those who are interested in the feature and haven't done so yet, please test!

If you discover some new issues with the above, please create a new ticket.

Changed 2 years ago by anonymous

  • cc trac@…, johannes_rudolph@… added; gunnar@…, trac@… removed

Add/Change #1830 (Repository subsets have to be fully self contained)

Author



Change Properties
<Author field>
Action
as closed
Next status will be 'reopened'
 
Note: See TracTickets for help on using tickets.