Opened 8 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: chrisheller
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@… 8 years ago.
adapt includesource plugin to Trac 0.12 multirepos branch

Download all attachments as: .zip

Change History (6)

Changed 8 years ago by gwk.rko@…

adapt includesource plugin to Trac 0.12 multirepos branch

comment:1 Changed 8 years ago by chrisheller

  • Resolution set to fixed
  • Status changed from new to closed

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_s

  • Priority changed from normal to high
  • Resolution fixed deleted
  • Severity changed from normal to blocker
  • Status changed from closed to reopened

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 chrisheller

  • Resolution set to fixed
  • Status changed from reopened to closed

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 chrisheller

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.

Add Comment

Modify Ticket

as closed The owner will remain chrisheller.
The resolution will be deleted. Next status will be 'reopened'.

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

Note: See TracTickets for help on using tickets.