Edgewall Software
Modify

Opened 8 years ago

Last modified 5 years ago

#10640 new enhancement

Mercurial user aliases

Reported by: Julian Brost <julian@…> Owned by:
Priority: normal Milestone: undecided
Component: plugin/mercurial Version: 0.12-stable
Severity: normal Keywords: mercurial patch review patch
Cc: ethan.jucovy@…, Ryan J Ollos Branch:
Release Notes:
API Changes:

Description

With Mercurial you usually use "Full Name <email@address>" as author for your changesets while with Trac you use just "somename" so it would be nice to have the ability to map changesets to the right Trac user.

I've made a patch that implements this but I guess someone should take a look at it as this is the first time I'm working on a Trac plugin.

Attachments (2)

repos_user_aliases.diff (2.2 KB ) - added by Ethan Jucovy <ethan.jucovy@…> 7 years ago.
changeset_user_mapper_component.diff (4.3 KB ) - added by ethan.jucovy@… 6 years ago.
New patch using component architecture

Download all attachments as: .zip

Change History (13)

comment:1 by Julian Brost <julian@…>, 8 years ago

Sorry, I can't upload the patch here because it triggers some spam filters and the captcha stuff does not work.

Uploaded it on pastebin.com instead: http://pastebin.com/8dTz1Pes

comment:2 by Oleg Frantsuzov <franoleg@…>, 8 years ago

Keywords: patch review added

comment:3 by ethan.jucovy@…, 7 years ago

Cc: ethan.jucovy@… added

I've been using Trac with Git repositories lately and would love to be able to define author→user aliases for my Git repos. I wonder if there's a way to do this in the core versioncontrol component so that it's agnostic of the versioncontrol backend(s) used?

by Ethan Jucovy <ethan.jucovy@…>, 7 years ago

Attachment: repos_user_aliases.diff added

comment:4 by Ethan Jucovy <ethan.jucovy@…>, 7 years ago

For the sake of argument I've attached a patch to the core trac.versioncontrol.api which provides this feature in a different way. There are two main points of departure:

  1. This patch should work across all versioncontrol backends. (Though I've only tested it — and briefly — against the Git backend.) No changes are necessary to any of the particular versioncontrol backends.
  1. This patch looks up the user aliases from a trac.ini configuration Option instead of a database query. I haven't done any benchmarks, but I expect this performs better in most circumstances. It also seems more in line with Trac's philosophy for what to put in configuration and what to put in the database (though admittedly that's just a gut feeling)

Configuration looks like:

[trac]
repository_user_aliases = Ethan Jucovy <ejucovy@example.com> = ethan, Full Name <email@address.com> = joe

This configuration gets loaded into memory and passed in to every repository object's params dict. The Changeset constructor then uses those aliases to convert the passed-in author before storing it as self.author.

In theory, the code allows individual versioncontrol backends, or individual repositories, to override the default user_aliases with a custom one, or with none at all. I haven't implemented any demonstration of that though, since it seems likely to be unnecessary.

The patch lacks tests, documentation, a web UI, and robust error checking :-) But I wanted to propose it in a raw form first to see what people think of the approach.

in reply to:  3 comment:5 by Ethan Jucovy <ethan.jucovy@…>, 7 years ago

Replying to ethan.jucovy@…:

I've been using Trac with Git repositories lately and would love to be able to define author→user aliases for my Git repos.

I should note that the Git backend does provide its own user-mapping feature:

## (0.12.0.3+) enable reverse mapping of git email addresses to trac user ids; default: false
trac_user_rlookup = true

This isn't very useful to me though, because it just reuses Trac's own user↔email mappings. I frequently use a different email address for my git commits than the one that's configured for my Trac instances; and I also use multiple email addresses for my git commits (e.g. depending on what machine I'm committing from)

Meanwhile looking over the Mercurial backend I don't see any comparable feature.

And this feature was recently requested for the Bazaar backend: https://answers.launchpad.net/trac-bzr/+question/177099

So I think there's value in a unified approach, and one which is independent of Trac's get_known_users function.

comment:6 by Christian Boos, 7 years ago

Milestone: undecided

All the tickets for {20} from last year have probably been seen multiple times by now, yet are still to be triaged…

by ethan.jucovy@…, 6 years ago

New patch using component architecture

comment:7 by ethan.jucovy@…, 6 years ago

I've uploaded a new patch attachment:changeset_user_mapper_component.diff which implements a unified approach to mapping changeset authors to Trac users. Unlike my patch last year, this one defines a new interface IChangesetUserMapper and provides a default implementation which just uses a trac.ini section [repository-changeset-user-mapper]. So, by default, configuration looks like

[repository-changeset-user-mapper]
Ethan Jucovy <ejucovy@example.com> = ethan
Full Name <email@address.com> = joe

But, the default component can be swapped out. This might be useful for implementing a database-backed alias map for sites with a very large number of users (e.g. github-style sites). The swappable implementation could also be used to provide a more intelligent mapper (e.g. trying to parse the author string, extract a raw email address, and look up that email address in trac's session table) or a mapper that maintains a separate map for each repository.

A few questions about this approach:

  • The Changeset object doesn't have access to the Environment, but we need the instantiated component in Changeset.__init__ — that's the only way I've found to make a single approach work across all repository backends. So, I had to pass in the component instance in the repos.params dict. Is that safe? Does that params dict ever expect to be pickleable for example?
  • The IChangesetUserMapper defines a method get_user(author, repos) which receives the literal author string, and the actual Repository object. Should it only receive repos.params instead? Or even just reponame?
  • Should the default ConfigSection-backed implementation cache the section contents in memory on __init__, or is it fast enough to do the lookup each time during the get_user call? (I assume that Trac loads trac.ini into memory only once per request, but I'm not familiar with the internals there.) Since this get_user method is called during initialization of every Changeset, it can happen many times in a single request, e.g. during a Timeline view or a Browser view.
  • Is there a better way to do this? I notice that CachedChangeset does have access to the env — should this feature only be available to objects that inherit from CachedChangeset perhaps? Or should Changeset itself be given access to env in its init? (Changing the Changeset signature would mean that third-party repository backends, like the bzr backend, need to update their code.)

comment:8 by Ryan J Ollos, 6 years ago

Cc: Ryan J Ollos added

in reply to:  7 comment:9 by Peter Suter, 5 years ago

Replying to ethan.jucovy@…:

The IChangesetUserMapper defines a method get_user(author, repos)

Bikeshedding: I'd have a slight preference for IRepositoryUserMapper (and maybe map_user(...)). The trac.ini section could be reduced to [repository-users].

It might be worth thinking about generalizing the interface. (What else could we do with something similar? Could a IChangesetManipulator be useful to tweak some other properties?)

But, the default component can be swapped out. This might be useful for implementing a database-backed alias map for sites with a very large number of users (e.g. github-style sites). The swappable implementation could also be used to provide a more intelligent mapper (e.g. trying to parse the author string, extract a raw email address, and look up that email address in trac's session table) or a mapper that maintains a separate map for each repository.

I think the default component should support per-repository maps somehow if easily possible. Maybe:

[repository-users]
reponame.username = comma_separated_authorstrings
trac.psuter = Peter Suter <petsuter@example.com>, peter

It seems like TracGit's trac_user_rlookup option should be extracted into an alternative implementation.

A few questions about this approach:

I think your design choices look acceptable.

One alternative would be to let each repository backend define and use its own extension point (based on the same interface). No change to Changeset and repo.params is needed. (And if desired one could configure Git to use rlookup, Mercurial to use config sections, and SVN to do nothing.)

comment:10 by Ryan J Ollos, 5 years ago

For reference, recent discussion on the mailing list about this ticket took place in:

comment:11 by Ryan J Ollos, 5 years ago

Keywords: mercurial patch review → mercurial patch review patch

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The ticket will remain with no owner.
The ticket will be disowned. Next status will be 'new'.
as The resolution will be set. Next status will be 'closed'.
The owner will be changed from (none) to anonymous. Next status will be 'assigned'.

Add Comment


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