Opened 9 years ago

Closed 8 years ago

Last modified 8 years ago

#3911 closed defect (fixed)

PATCH: adapt includesourcepartial plugin to the Trac 0.12multirepos branch

Reported by: gwk.rko@… Owned by: Chris Heller
Priority: high Component: IncludeSourcePartialPlugin
Severity: blocker Keywords:
Cc: Trac Release: 0.11


Please find attached my patch that makes the includesourcepartial plugin usable for the Trac 0.12multirepos branch.

Attachments (1)

includesource-trac-multirepos.patch (1.7 KB) - added by gwk.rko@… 9 years ago.
adapt includesource plugin to Trac 0.12 multirepos branch

Download all attachments as: .zip

Change History (6)

Changed 9 years ago by gwk.rko@…

adapt includesource plugin to Trac 0.12 multirepos branch

comment:1 Changed 9 years ago by Chris Heller

Resolution: fixed
Status: newclosed

Applied in changeset:4519.

Note that I only tested this patch to ensure that it still works properly in 0.11 as I don't have an 0.12multirepos environment to test against. I'm assuming that the submitter already checked that :-)

comment:2 Changed 8 years ago by Martin Scharrer

Priority: normalhigh
Resolution: fixed
Severity: normalblocker
Status: closedreopened

This patch introduces a heavy bug on my trac 0.11.1 installation. All URL schemes given in the example section stopped working because the 'trunk' is taken as a repository name!

So e.g. [[IncludeSource(trunk/proj/]] gives me an 'proj/ not found in revision 123' error.

Providing 'trunk/' twice fixes the problem partially: the source code is displayed but the link to the full source file is incorrect because of the second 'trunk/'.

Downgrading to the previous revision fixes the problem.

This patches completely break backwards compatibility (at least on my installation) and isn't needed for 0.11 anyway, so I suggest to remove it from the 0.11 branch and only have in the 0.12multirepos branch.

I also like to suggest to update the usage section to explain how to use the new multirepos syntax before applying such a patch.

comment:3 Changed 8 years ago by Chris Heller

Resolution: fixed
Status: reopenedclosed

OK, I think that I must have run the tests in an environment that still had the old version of the macro installed, which explains why I didn't see any regressions.

After realizing that, I have now made the test for the multirepos code more explicit by looking for the new IRepositoryProvider interface in the multirepos patch. If this interface does not exist, then the original code runs.

@gwk: I have slightly modified your existing patch logic to now raise a TracError if multirepos support is available and no repository is returned.

However, I'm still not 100% sure about the inner workings of the multirepos patch, so I don't know if this is the correct behavior. Is it possible to have the multirepos patch installed, but only be using one repository? If so, then what is the proper test to discover whether multiple repositories are being used?

comment:4 Changed 8 years ago by Chris Heller

Nevermind the previous question. I took a closer look at the multirepos patch and see that the original line is the way to go. Restored in changeset:4590.

There is a comment in source:sandbox/multirepos/trac/versioncontrol/ that indicates that at some point the get_repositories method will return a default repository so this may need re-visiting in the future.

comment:5 Changed 8 years ago by gwk.rko@…

Hey, just wanted to say I'm sorry I caused you quite some grief. I should have been more explicit when I posted, should have said that I created this code for the multirepos branch only and did not consider the case of running this same code in 0.11. I'm sorry.

Modify Ticket

Change Properties
Set your email in Preferences
as closed The owner will remain Chris Heller.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment

E-mail address and name can be saved in the Preferences.

Note: See TracTickets for help on using tickets.