Edgewall Software
Modify

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#12576 closed defect (cantfix)

All Versions: commit_updater.py does not update ticket comment when log message is updated

Reported by: anonymous Owned by:
Priority: normal Milestone:
Component: ticket system Version: 0.12-stable
Severity: normal Keywords: commit_updater
Cc: Branch:
Release Notes:
API Changes:
Internal Changes:

Description (last modified by anonymous)

The script fires due to my post-revprop-change hook trac-admin $TRAC_ENV changeset modified "$REPOS" "$REV" when I change a property like the commit log message, but because of the changeset_modified function on line 166, the script does not update anything when the ticket number is the same so I do not see the new commit message if I change it.

Is this intended and if so, why?

Attachments (0)

Change History (17)

comment:1 by anonymous, 5 years ago

Description: modified (diff)

comment:2 by anonymous, 5 years ago

Description: modified (diff)

comment:3 by Jun Omae, 5 years ago

Milestone: 1.2
Resolution: cantfix
Status: newclosed

Not a defect. That's an InstallationIssue.

Executing changeset modified $REPOS $REV would update message column in the revision record and CommitTicketReference processor render new message.

Support and installation questions should be asked on the mailing list or IRC channel, not filed as tickets. Also, please check whether the issue you've encountered has been reported before, either in this Trac or in past Trac-users mails.

in reply to:  3 comment:4 by anonymous, 5 years ago

Replying to Jun Omae:

Not a defect. That's an InstallationIssue.

Executing changeset modified $REPOS $REV would update message column in the revision record and CommitTicketReference processor render new message.

Support and installation questions should be asked on the mailing list or IRC channel, not filed as tickets. Also, please check whether the issue you've encountered has been reported before, either in this Trac or in past Trac-users mails.

I specifically mentioned in the ticket that I am verified executing changeset modified $REPOS $REV.

How do you figure that the CommitTicketReference will update when the tickets dict will always be empty when the ticket number is the same?

    def changeset_modified(self, repos, changeset, old_changeset):
        if self._is_duplicate(changeset):
            return
        tickets = self._parse_message(changeset.message)
        old_tickets = {}
        if old_changeset is not None:
            old_tickets = self._parse_message(old_changeset.message)
        tickets = dict(each for each in tickets.iteritems()
                       if each[0] not in old_tickets)
        comment = self.make_ticket_comment(repos, changeset)
        self._update_tickets(tickets, changeset, comment,
                             datetime.now(utc))

The above will always do nothing when the changeset messages reference the same ticket # as far as I can see

comment:5 by Jun Omae, 5 years ago

New commit message would be set to the revision record by Repository.sync(rev) at tags/trac-0.12.7/trac/versioncontrol/api.py@:678#L644 before calling CommitTicketUpdater.changeset_modified() at tags/trac-0.12.7/trac/versioncontrol/api.py@:689-690#L644.

Last edited 5 years ago by Jun Omae (previous) (diff)

comment:6 by anonymous, 5 years ago

To be clear, here is the scenerio:

  1. I make a commit referencing a ticket.
  2. The ticket gets a comment with the commit message
  3. I change the commit message with svn propedit
  4. The hook fires and executes changeset modified $REPOS $REV
  5. CommitTicketUpdater.changeset_modified() runs with the new commit message
  6. Because the code says do nothing if the ticket number in the message is the same as the old message, nothing happens

comment:7 by Jun Omae, 5 years ago

Component: generalticket system

Okay. I understand what you're saying, a little. CommitTicketUpdater updates tickets only once. This behavior is by design, not a defect. We consider no need to add comment to the ticket if updating only commit message without ticket ids (minor changes in commit message).

Last edited 5 years ago by Jun Omae (previous) (diff)

comment:8 by anonymous, 5 years ago

So I would expect that if I have a comment in ticket #123 like this:

"Here is my Commit message. refs 123"

When I edit the message to be "Here is my Commit message, and something I forgot. refs 123"…

I would expect that this would update the comment in ticket #123 and it appears the code was setup to accomplish this if I just remove the if each[0] not in old_tickets line, but my concern is this was to address some other case that could cause problems.

Perhaps it was just decided, not to update these because this scenario is rare-ish? I have not looked into the code, but I can tell you by default, the changeset page commit message does not update either.

in reply to:  8 comment:9 by Ryan J Ollos, 5 years ago

Replying to anonymous:

So I would expect that if I have a comment in ticket #123 like this:

"Here is my Commit message. refs 123"

When I edit the message to be "Here is my Commit message, and something I forgot. refs 123"…

I would expect that this would update the comment in ticket #123 and it appears the code was setup to accomplish this if I just remove the if each[0] not in old_tickets line, but my concern is this was to address some other case that could cause problems.

To reinforce what Jun said, I believe commit_updater does exactly what you want, and I think you have an InstallationIssue.

While CommitTicketUpdater does not update the ticket comment by modifying the ticket_change table when a changeset is modified (as you pointed out in comment:4 with the code snippet), the new changeset message is stored in the revision table (as Jun mentioned) and the new changeset message will be rendered in the comment by CommitTicketReferenceMacro under normal circumstances. This is because CommitTicketReferenceMacro first tries to get the changeset message from the database, and only uses the content in the WikiProcessor if an exception occurs while getting the changeset attributes from the repository: tags/trac-1.0.12/tracopt/ticket/commit_updater.py@:306,310#L299.

As a concrete example, consider th:comment:2:ticket:4618.

Let's consider a fictional scenario in which the message was edited with this change:

-Fix duplicated packages arguments of setup() (closes #4618)
+Fix duplicated `packages` arguments of `setup()` (closes #4618)

Then the content of the ticket comment would be:

In [changeset:"15741" 15741]:
{{{
#!CommitTicketReference repository="" revision="15741"
Fix duplicated packages arguments of setup() (closes #4618)
}}}

and you can see the body of the old message is contained in the WikiProcessor. However, the message would be rendered exactly as it's shown in th:comment:2:ticket:4618, since the WikiProcessor content is not used, instead the message is grabbed from the database.

I agree that this is a bit confusing, see #12485 for a proposed change that would affect this behavior.

Perhaps it was just decided, not to update these because this scenario is rare-ish? I have not looked into the code, but I can tell you by default, the changeset page commit message does not update either.

If the message on the changeset page does not update either, then it's likely that your post-revprop-change script is encountering an error. To debug the issue, considering using tags/trac-1.0.12/contrib/trac-svn-hook, which will create a nice log in the environment logs directory.

Could you confirm that you are using a cached repository for Subversion? See TracRepositoryAdmin#ExplicitSync. The type should be svn, not svnfs or direct-svnfs. I find that to be a little confusing, so I've proposed #12406.

Also, consider upgrading to Trac 1.0.12.

Last edited 5 years ago by Ryan J Ollos (previous) (diff)

comment:10 by anonymous, 5 years ago

"While CommitTicketUpdater does not update the ticket comment by modifying the ticket_change table when a changeset is modified (as you pointed out in comment:4 with the code snippet), the new changeset message is stored in the database (as Jun mentioned) "

I don't understand. If CommitTicketUpdater does not update the database, how is it supposed to get updated? For the record, my entry in the database is NOT getting updated. My post-revprop-change hook is working and does not have any errors.

in reply to:  10 comment:11 by Ryan J Ollos, 5 years ago

Replying to anonymous:

I don't understand. If CommitTicketUpdater does not update the database, how is it supposed to get updated?

I did not say the CommitTicketUpdater does not update the database, it does. It seems like you didn't even take the time to read my comment and think about it.

comment:12 by anonymous, 5 years ago

I used the trac-svn-hook to verify there is no error in the post-revprop-change hook.

I'm not sure what type of caching is in place or how to check the type, but I can tell you that we are using the ExplicitSync you mentioned. I am new to trac, but are you implying this is likely a caching problem?

So I misread your sentence. The message is getting updated in the revision table, but the CommitTicketReferenceMacro is not rendering the new message. The try block is running without error and message = changeset.message is pulling back the old message.

Another oddity is that when I click the edit button on the comment, not only does the message refresh properly, but even when I cancel the edit, the new message persists.

in reply to:  12 ; comment:13 by Ryan J Ollos, 5 years ago

Replying to anonymous:

So I misread your sentence. The message is getting updated in the revision table, but the CommitTicketReferenceMacro is not rendering the new message. The try block is running without error and message = changeset.message is pulling back the old message.

I want to make sure I understand what you are saying. Have you looked at the revision table in the database to confirm that the database contains the edited message?

If using explicit sync, make sure to set:

[trac] repository_sync_per_request =

in trac.ini. The option must be set to an empty value, it cannot be omitted from trac.ini if you wish to use explicit synchronization and disable implicit synchronization.

in reply to:  13 ; comment:14 by anonymous, 5 years ago

Replying to Ryan J Ollos:

I want to make sure I understand what you are saying. Have you looked at the revision table in the database to confirm that the database contains the edited message?

Yes, I verified the revision table gets updated every time I change the log message

Also verified the [trac] repository_sync_per_request = exists in trac.ini

in reply to:  14 comment:15 by Ryan J Ollos, 5 years ago

Replying to anonymous:

Yes, I verified the revision table gets updated every time I change the log message

If neither the CommitTicketReferenceMacro nor the /changeset page are displaying the message that is contained in the revision table following an svn propedit --revprop svn:log, then it sounds like the page is being cached in your browser. Also the comment:12 description of the message updating after entering comment edit mode points to a page caching issue. Could you try to do a hard refresh of the page after updating a commit message?

Do you have any plugins installed? Which webserver are you running?

comment:16 by anonymous, 5 years ago

I do not think it was the cache, since the first ticket I checked had modified message from another user and the first time I loaded the page it was the old comment. I am also not seeing any updates to comment even with clearing cache or changing browsers. The edit thing may have been an anomaly as it doesn't work for me in a different ticket.

Running apache2

Here is the plugin list:

authz-policy
BatchModify 0.8.1-trac0.12dev-r45075
Bitten 0.8.0
Trac 0.12.5f
TracAnnouncer 0.11.1
TracDynamicFields 1.2.4
TracIncludeMacro 3.0.0
Color
PageAuthzPolicyEditor 0.1dev-r3719
timingandestimationplugin 1.3.0b
TracAdvParseArgsPlugin 0.2.6952
TracCollapsiblePlugin 0.1
TracDateField 2.0.0dev
TracMenusPlugin 0.1
TracRandomInclude 0.1
TracReposReadMePlugin 0.1
TracTags 0.6
TracTicketCharts 0.2dev-r13335
TracTocMacro 11.0.0.3
TracWysiwyg 0.11.0.2
TracXMLRPC 1.1.2

comment:17 by Ryan J Ollos, 5 years ago

I'd focus on trying to determine why the /changeset page is not updating when you edit a commit message. You stated in comment:8 that the message isn't updating. You also stated in comment:12 that you've inspected the revision table to confirm that the message is updated in the table when you edit a message in Subversion. The edited message should certainly be displaying on the changeset page. If it is, then I expect the CommitTicketUpdater will also display the correct message.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The ticket will remain with no owner.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from (none) to the specified user.

Add Comment


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