svn:log editing

[mortonda@…]

Would it be possible to use trac to edit svn:log properties? It would be handy to be able to edit a change log to add more information if needed.

[daniel]

This would be seriously cool.

[cboos@…]

See also TracObjectModelProposal / Trac Objects / Toward an Unification / Changeset

(In a nutshell:

• Trac objects are things having an unique id, an author, a creation time and a Wiki content
• Changeset can be considered to be Trac objects as well, the commit log being the Wiki content.
  • this content for now is read-only
  • if editable, this could/should be implemented using a svnadmin setlog equivalent

)

[cmlenz]

A couple of comments:

• Log message editing would have to be optional, as it requires the revprop-change hook to be installed for the svn repository.
• We could provide a hook script that would write the old log message into the Trac DB, and make the history of log messages available through the web interface.
• Permissions: should we add a CHANGESET_MODIFY permission for this? Or only allow TRAC_ADMIN and the original author of the changeset to edit the log message?

[otavio]

I think a CHANGESET_MODIFY permission is better because when the project have a team sometimes you need to fix a mistake of another person in log or something like.

[cboos]

Argh, this is really needed.

It's very easy to get a messed up formatting because of

'message': wiki_to_html(util.wiki_escape_newline(chgset.message or '--'),

in trac/Changeset.py: what would otherwise be valid WikiFormatting (like block quotes) is completely ignored because of the escaping of newlines...

See [1535] for an example of a real disaster.

I know that this escaping was done because of #48, but we really should do something better.

[anonymous]

Why don't we take advantage of svnadmin setlog REPOS_PATH -r N FILE instead of using the revprop-change hook? Using the hook would mean that the user has to install it in every repo and it would also lead to problems with windows again (e.g. see #897).

[cboos]

The real problem with changing the log message is not really how to do it (there are several options), but rather how to protect the information, which is obviously sensitive.

The log messages themselves are not versioned within Subversion, and this would need to be done in Trac, somehow similar to what is done for a Wiki page (hence the reference above to the TracObjectModelProposal).

[anonymous]

In case of saving the log history (inside Trac) you have to enable Subversion's revision property modifications, i.e. to put the revprop-change hook into the repository.
Will this be automated in some way? IMHO you shouldn't provide functionality inside Trac when you're not sure about what permissions are enabled in Subversion. That's why I wanted to add attention to the two potential problems above.
OTOH you could also save the current log message before editing it and then use svnadmin (which would lead to fewer problems IMHO).
However, I agree that editing log messages should be protected by some of Trac's permissions!

[anonymous]

Using fs.change_rev_prop() with svn:log as PROPNAME seems to be the most adequate way. Note that this change isn't versioned by Subversion.

[anonymous]

This would be a really nice feature...

[markus]

I tried to implement such a svn:log editing feature into Trac and it turned out that editing the log message is indeed quite easy. However, saving old log messages that were replaced within Trac seems to be the hardest task.

My concept was to save old log messages within the Trac database (stored in a table named log_change) in order to be able to show a complete history of log message changes (and their dates and authors). Unfortunately I'm lost when it comes to adapt the resync command that currently just deletes all cached log messages stored in the db. What should happen to the message history in this case? Furthermore, my approach heavily depends on constant revision information inside Subversion. That means that if any other application beside Trac (e.g. svnadmin) changes a revision's log property (say commit date) the log history will get messed up and useless.

I'm looking forward to your ideas and suggestions.

[cboos]

(changing the status, as I've not actually started to work on that yet)

[moisei]

You may considering to use the svn properties mechanism. That's it, instead of keeping the messages history in the trac tables, keep it in some revision property reserved by trac (trac:log-history).

[cboos]

Just adding a quick note that for the purpose of add more information if needed, there's also the option to have comments for changesets, like we have for tickets. See #2035.

[markus]

Well, why commenting on an incorrect log message if there's the ability to change/correct it? I think these two tickets aim for different things.

I already thought about using revision properties like suggested by moisei. However, since most Trac users don't want Trac do write access their repositories (which Trac obviously needs to do in this case), I fear that most of them also don't want to have automated and program-specific revision properties (containing e.g. hashes). Anyway, I'll get back to this topic when #1271 has been integrated.

[moschny]

Note that the proposed patch doesn't touch versioncontrol/api.py. Trac now supports multiple version control backends, so any change should first be made there.

Currently, the backends provide read-only access to a repository. API changes that introduce write operations should be introduced carefully. Other VC systems might have their own access rules and/or requirements for such operations. The Monotone backend for example would need a private key for adding new certificates to a revision.

[markus]

Yeah, my patch is outdated. I'm also aware of the fact that Trac supports multiple vc backends now and so I'm going to do this as a Subversion-only plugin first. The pros and cons and how it should be integrated into trunk can be discussed at a later point once there are a few users working with the plugin. However, I'll wait for #2348 before starting to write the plugin (cf. comment:ticket:1271:22).

[trachacks@…]

While I see this plugin as very handy there will be situations where Trac only has readonly access to the repo.

I would really very much like for trac-admin /path/to/repo resync to accept a -r argument for revision ranges. We've got over twenty thousand revisions in our SVN and even this is not stupid (Mandriva have over a hundred thousand and I'm sure that's not as high as some people). The full resync I had to do after changing a log took well over 5 mins on a fairly powerful machine all the while locking users out of the Trac web interface. Not very nice. If I could create a post-revprop-change hook that ran trac-admin /path/to/repo resync -r thisrev then it would presumably only take a second or two. This would presumably be useful for other backends too, but I guess the whole revision number thing is not universal and thus complicates issues.

[Prasand J. <cyrano423@…>]

The last comment was addressed in revision 5140 and 5141 (see ticket:4797).

---

Storing the changes in a trac property may address TracMultipleProjects concerns with a single repository, and multiple enviroments. Since the varying environments would all read the trac:property instead of it being stored in a single database. However, I agree with markus that many users may not want additional data added to their repositories. I believe that trac should remain separate from the repository.

I also agree that the functionality should not be available for servers that do not have the pre-revprop-change hook enabled. The problem with making it easier for people to use, is making it easier for people make a mess of things. Even users with the correct elevated permissions will still make mistakes. Also, there's a problem with "not" using a svn hook. If hooks aren't enabled on the server, while trac can continue to accept items and keep it solely in the database ... what happens if hooks are enabled later (so changes can be made via the svn clients)? How does trac differentiate between a front-end and svn change? Strings can be compared, but what about the dates of the edit (to see which has a higher priority or was more recent)?

I'll try to address such concerns with my proposition. First, I disagree with the use of svnadmin. With using svn propedit one has the ability of editing things other than the log (for example, admins can address username changes if the webmaster deems to enable that in the hook).

Determining what's enabled and what is not can be addressed by the svn propedit output which would be directed to the user upon a successful change, or denied change along with the message. If the user has not setup the hook, then they'll get the message indicating that they have not (or it would be disabled all together). For now it can be restricted to editing the log, but other items can be eventually added due to the use of svn propedit giving the editing aspects of trac room to grow.

If the result is successful the previous prop item and value is stored in the database table revision_change. The table would contain two columns rev# and data (not those names just an idea of what should go into the table). When trac reads the info from the database it would grab / select all items pertaining to that revision (column one) and compare the table row modified dates and display the most recent change. However, the issue with resync still exists. To address that, on a resync if the old database item is not equal to the previous one ... store the previous item in revision_change table as editing via trac's interface would.

This presents a final problem. That is ... what happens when the svn users edit from an svn client yesterday, but someone edits it on the trac interface today, and it gets resynced later today? The sync would update the database, and the order of the edits would be out of sequence even if everything is stored and kept trac of correctly (hence the non-hook date issue previously presented). Since the update would point to today (after today's true edit), although it was made yesterday.

Additionally, what happens if a resync wasn't done and the person who is unaware of the change made in the svn ... tries to correct something already corrected? Who's correction is more accurate and which should take precedence? The person who came last?

I guess this can be remedied in the ability to resync the database per revision (ticket:4797 again). In the post-revprop-change hook it can be called (trac-admin <repos> resync <rev>). Either that or a function to add the item to the database can be created. For now let's say: trac-admin <repos> change <rev> <olddata> <newdata> ... is called in the pre-revprop-change. In either case, waiting for a full resync and things being out of sequence wouldn't be an issue.

An alternative might be. If a user attempts to edit an item from the trac interface, before the item is submitted it is retrieved from the svn. Making sure that the data being edited is the same as the data the user was working on. A conflict checker & diff display of sorts. The user would get the update unsuccessful and it would show the conflicts.

Though maybe that proves to be too complicated. I dunno, just wanted to extend some additional thoughts. Maybe it'll invoke some ideas. IMHO while the latter implementation would be a nice feature, it presents the problem of trac being outdated until someone tries to edit a previously created entry. I think trac-admin <repos> change would be a nice compliment. However, it can be handled with trac-admin <repos> resync <rev> and may be deemed redundant. I guess it's just an issue of "pre" versus "post" processing.

If it's done in a pre hook, and trac errors it can prevent the item from being changed ... thus saving the data from being lost (if successful return 0 (zero) and allow the commit). Wheras in a post operation if trac errors, the data was already changed and thus lost. In general this tracking may appeal to users who have hooks to email property modifications.

Please excuse the length, and thank you for your time.

- Prasand

[Prasand J. <cyrano423@…>]

didn't mean to delete the 'helpwanted' keyword.

[cboos]

... you also cleared the milestone ;-)

Thanks for sharing your thoughts on the bi-directional sync problem.

As you mentioned, the unidirectional sync (from svn to Trac) should now be possible thanks to a post-revprop-change hook making use of the trac-admin <repos> resync <rev> command. We should probably document that somewhere...

[Prasand J. <cyrano423@…>]

Referencing cboos:

if editable, this could/should be implemented using a svnadmin setlog equivalent

Referencing anonymous:

Why don't we take advantage of svnadmin setlog REPOS_PATH -r N FILE

Replying to cboos:

After re-visiting a similar concern, I began to wonder if we all weren't just complicating matters. While I can see the concern to trac bi-directional changes, a base svnadmin front-end is truly what this ticket addressed.

Considering you're the one who first proposed it (in your first comment) do you think the core functionality / front-end to svnadmin can be implemented, and keeping trac of the changes / history added later? Do you think it should be, and why?

[mortonda@…]

Replying to Prasand J. <cyrano423@hotmail.com>:

After re-visiting a similar concern, I began to wonder if we all weren't just complicating matters. While I can see the concern to trac bi-directional changes, a base svnadmin front-end is truly what this ticket addressed.

This is all I really want, IMHO... just to correct dumb errors or to add a little information that was missing. For example, when dealing with multiple branches, I like to specify the branch in the svn log, just for easier identification. Sometime I forget, or another person commits without it. It would be nice to be able to fix that from within trac.

I'm not all that worried about the history of the log files. As log as this operation is protected by the proper ACL and maybe have a warning about the lack of history.... that's all I need.

[cboos]

There are many ways to approach the functionality requested here...

For example, in the original request:

It would be handy to be able to edit a change log to add more information if needed.

That could be alternatively fulfilled by the possibility to add comments on changesets, the way we're doing it for tickets (#2035). This way we avoid the synchronization problem altogether.

That's also an approach that would work well when the VersioningSystemBackend has immutable changeset message (e.g. Mercurial).

But if you're really willing to edit the changeset message itself and want to allow that even for backends like Mercurial, then you realize that you have no other choice than version the changeset message in Trac. That would work pretty well, as you'd have the possibility to see that a message has been edited, see the edits, go back to the original version, etc. like you can for a wiki page (or a ticket description in 0.11).

The above gets indeed a bit more complicated for backends like Subversion, where you actually have the option to edit the changeset message by other means (1. svnadmin setlog, 2. svn propedit, 3. direct use of the SVN Python bindings in Trac).

Having the possibility to integrate those external changes (1. and 2.) as new versions of the changeset message in Trac would actually be an advantage, as otherwise those changes would be unversioned. OTOH, changes made within Trac should be mirrored in Subversion, so that the notion of the "current" changeset log message is the same in both worlds. For this, we could use 3. or if not possible for some reason, use an external program (1 or 2). Of course, when that change is "coming back" by the way of the revprop hook, Trac should notice that it already has this "version" of the changeset message, and should not loop. That should be easy to do, as the corresponding message content, author and timestamp should already exist in Trac's db.

In case of a conflict, or a svn propedit overwriting someone's edit within Trac, the fact that all the changes are versioned within Trac make it possible to correct those problems by making a new edit.

[cboos]

Replying to mortonda@dgrmm.net:

This is all I really want, IMHO... just to correct dumb errors or to add a little information that was missing... It would be nice to be able to fix that from within trac.

Understood. That could be an alternative option, or rather, something to get started with.

[Prasand J. <cyrano423@…>]

Replying to cboos:

But if you're really willing to edit the changeset message itself and want to allow that even for backends like Mercurial, then you realize that you have no other choice than version the changeset message in Trac.

---

direct use of the SVN Python bindings in Trac

---

I imagine that addressing the issue with the varying VCS that trac supports in a unified manner is of the utmost priority. As simply creating varying frontends may lead to bloat, and require many tweaks. As such forward-thinking is important. Additionally addressing the varying db concerns it can be downright annoying to implement in such a manner, lol.

At first, after reading your very last comment I thought the plugin for now would be beneficial to users. While this addresses something supplemental or something else entirely. Though after examining the priorities, I'm kinda against the plugin idea now. As I believe it is primary to come up an effective system that works irrespective of the VCS type, something that should work even if there isn't a VCS. Then address the backends and external changes secondarily.

Whereas the plugin would invert those priorities. Then again, that's the purpose of a plugin isn't it? To address users immediate concerns, whereas the core developers would think from a bigger picture or from the general client base perspective. I divest.

--

The commenting system may appeal to trac users, but it might not 'generally' with the admins. Where a user would comment on what 'should' be changed, but the admin or manager may want to actually apply the change. I think in such a case the user might as well just submit a ticket, for the item to be corrected. On the other hand, commenting on changesets may be beneficial discussion wise (or for code review purposes as the other ticket indicates). Also, comments can be used as the "detailed" description, and the svn:log (or equivalent) can be perceived as the summary. Regardless I think that's another subject entirely.

After looking at the revision table structure, I see that there's column for a unix timestamp. That table's structure and content can be duplicated to a revision_history or revision_change, and all revision changes are simply appended to the additional table. When the revision change is applied or made active, it would replace the original table's values. The revision_change table would contain an untouched history of every change made.

When things such as the timeline, or changeset display it will only read from the revision table. If a user would like to see the previous messages or changes, they can do so by clicking on the message itself or a diff link to the right of it (or at the end). However, that diff link should not contain the number of changes.

Forward-thinking ... appending to the revision_change table will lead to very large tables on some systems. Counting those values each time the changeset page is loaded, may take awhile (especially if re-indexing is needed). So the pulling and counting of revision_change numbers, should only happen if specifically requested. It's also beneficial as the trac codebase won't require many changes to implement it. Also, in the case with large databases there can be an option to only retain a arbitrary number of changes. Defaulting to retain all of them, and at the minimum retain only the last two or three changes.

From a VCS backend perspective this would work as well. As the resync command would only touch the revision table, but even if it over writes something the item it overwrote would still be in the history. If revision history are read & counted only on the property history page load, then the revision_change table 'can' be updated at that moment. Therefore not needing to adapt the resync command. If someone applied a resync between the last time that page was loaded and now ... then the new message is appended to the history.

The qualm might be "how does the date of that revprop change get added?" Some perfectionists may dislike it being dated with the last revision history page load's date. I'm a perfectionist, but I think I could live with that, lol.

[Prasand J. <cyrano423@…>]

A follow up of previous comment:

The previous VCS backend perspective doesn't factor in multiple / repeated edits to the VCS. Even if a revprop hook is triggered, if the revision_change table is only updated when someone attempts to view the revision's changes. It will only add the last item synced. As such, the resync command may require adjusting afterall (unless someone comes up with an alternative). However, I guess that can be addressed after the core trac editing concept has been solidified (as an additional task). Just wanted to make note of it.