Links to ChangeIds from Gerrit comments are broken

Bug #1265646 reported by Zane Bitter
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Core Infrastructure
Fix Released
Undecided
Zane Bitter

Bug Description

The change https://review.openstack.org/#/c/62523/ modified the regex used to search for ChangeIds in Gerrit comments to require whitespace around the ChangeIds. The reason for this was to prevent any string of 8 hexadecimal digits (e.g. a date string in a URL) from being converted to a link. However, it has broken common and non-dangerous use of punctuation (such as commas and parentheses) around ChangeIds. It has also broken the functionality introduced in https://review.openstack.org/#/c/45464/ for bug 904298, to allow links to Git SHAs when they appear in cherry-picked patches backported to stable branches (since these are followed by a parenthesis). Ironically, it was this change to detect Git SHAs that caused the overmatching that the most recent change was attempting to fix in the first place.

Zane Bitter (zaneb)
Changed in openstack-ci:
assignee: nobody → Zane Bitter (zaneb)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to config (master)

Fix proposed to branch: master
Review: https://review.openstack.org/64738

Changed in openstack-ci:
status: New → In Progress
Revision history for this message
Jeremy Stanley (fungi) wrote :

This was fixed by https://review.openstack.org/64308 but Gerrit has to be restarted for it to take effect, and we haven't done that yet (I only just noticed it merged, so I can try to find a good window to restart it tonight maybe).

Revision history for this message
Jeremy Stanley (fungi) wrote :

Well, actually 64308 only served to address the fact that LP bug numbers started getting misinterpreted as gitshas once we reduced the minimum length for them from 8 to 7 digits.

Revision history for this message
Jeremy Stanley (fungi) wrote :

Gerrit has been restarted for the fix to the broken bug hyperlinks.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to config (master)

Reviewed: https://review.openstack.org/64738
Committed: https://git.openstack.org/cgit/openstack-infra/config/commit/?id=eeb9e111fd05fbf3042d296eef074d07eb34a362
Submitter: Jenkins
Branch: master

commit eeb9e111fd05fbf3042d296eef074d07eb34a362
Author: Zane Bitter <email address hidden>
Date: Thu Jan 2 16:32:37 2014 -0500

    Separate handling of ChangeIds and Git SHAs in Gerrit

    Since Ia337ddfe2ddc82c858072135a613786070f0dc67, ChangeIds not
    surrounded by whitespace (such as that one) are no longer turned into
    links. (It also reduces the minimum length of a hex string required to
    recognise a ChangeId from 8 to 7 for reasons that are undocumented, and
    therefore suspect.)

    The purpose of that patch was to prevent overzealous matching of strings
    in e.g. URLs as possible ChangeIds. This stems from
    I5cd74bef5510c8dbf3891dcfa3086a141e445f0d, which extended the changeid
    regex to also look for Git SHA hashes, which are far more likely to
    show up as false positives.

    In fact, Ia337ddfe2ddc82c858072135a613786070f0dc67 actually destroys the
    original rationale of I5cd74bef5510c8dbf3891dcfa3086a141e445f0d by
    refusing to match SHAs that are followed by a closing parenthesis, as
    cherry-picked commits on stable branches are.

    This patch makes the following changes:
    * Separates the changeid and gitsha commentlink handlers
    * Restores the changeid patterns to their original values
    * Matches only full Git SHAs (not just all 8+ character hex strings)
    * Allows for other common punctuation around Git SHAs (commas, colons,
      semicolons, parentheses).

    Fixes bug 1265646

    Change-Id: I0844ee2fd8d8699059eb7afa92b282cf5b890c18

Changed in openstack-ci:
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.