Issue 1708: --branch option for set-reviewers
Status:  New
Owner: ----
Reported by saxo...@gmail.com, Dec 6, 2012
Gerrit permits the reuse of Change-Id across branches and projects, and a commit SHA can appear on multiple branches within a project. This makes it impossible to uniquely identify a change using either of these identifiers. 

I've filed a separate bug about the use of the internal change number for this purpose, but we'd prefer not to use the internal change number. A --branch option would solve this issue better, since the branch/sha and branch/project/change-id option sets would permit unique identification without exposing the internal change number.  
Feb 8, 2013
Project Member #1 david.pu...@sonymobile.com
> Gerrit permits the reuse of Change-Id across branches and projects

I don't think Change-Id is supposed to be used to identify a specific change/commit.  Instead I think the sha1 should be used.


> a commit SHA can appear on multiple branches within a project

As far as I can tell it's not possible to push the exact same commit for review on more than one branch.  On the second push it will fail with "remote rejected" and "no new changes".

The only way I found to get it to accept was to edit the commit before pushing the second time.  But then it's no longer the "same" commit; the sha1 is different.

> A --branch option would solve this issue better

As far as I can see in the implementation, it is not trivial to add this.  And other ssh command also don't have this.  See the review command for example.


Feb 8, 2013
#2 saxo...@gmail.com
I'm not sure how we managed to do it, but in our database right now we have 4 pairs of changes each referencing one or more identical patch sets:

mysql> select c.change_id,c.change_key,p.revision,p.patch_set_id,c.dest_branch_name from changes c join patch_sets p on c.change_id=p.change_id where p.revision in (select revision from patch_sets group by revision having count(revision) > 1 );
+-----------+-------------------------------------------+------------------------------------------+--------------+--------------------+
| change_id | change_key                                | revision                                 | patch_set_id | dest_branch_name   |
+-----------+-------------------------------------------+------------------------------------------+--------------+--------------------+
|        91 | Ib2d62eacd677fb519ea5500ca3e7b6395294b3d6 | 7c3a7687c596d8dadf046d94244d7887c9a1f548 |            2 | refs/heads/rel8.10 |
|        92 | I7c3a7687c596d8dadf046d94244d7887c9a1f548 | 7c3a7687c596d8dadf046d94244d7887c9a1f548 |            1 | refs/heads/rel8.10 |
|       460 | I2e1dfc3629d191a080d32dfb4db41e3543d61f0a | e8a61d218b71006716c8f522976f938b01accc06 |            2 | refs/heads/rel1.14 |
|       461 | Ie8a61d218b71006716c8f522976f938b01accc06 | e8a61d218b71006716c8f522976f938b01accc06 |            1 | refs/heads/rel1.14 |
|      1482 | I487d7eca53aa8b6fe9df568a348e3fe22c458500 | cfa6a6657fe17a9222822bef1808a3337a2d30c5 |            3 | refs/heads/rel8.10 |
|      1509 | Icfa6a6657fe17a9222822bef1808a3337a2d30c5 | cfa6a6657fe17a9222822bef1808a3337a2d30c5 |            1 | refs/heads/rel8.10 |
|      2035 | I14b4df1adc7fcf329aca3cad404d671451cd2cc8 | 2e3c97e1da925a0d321a54ce1680bf376ff10a2f |            2 | refs/heads/rel8.10 |
|      2038 | I14b4df1adc7fcf329aca3cad404d671451cd2cc8 | 2e3c97e1da925a0d321a54ce1680bf376ff10a2f |            2 | refs/heads/rel9.00 |
+-----------+-------------------------------------------+------------------------------------------+--------------+--------------------+
8 rows in set (38.67 sec)

The last two changes are the ones that gave us problems with set-reviewer, since the only differentiating factor is the branch and we can't restrict by that. Here's the commit history for those:

mysql> select * from changes where change_key='I14b4df1adc7fcf329aca3cad404d671451cd2cc8' \G
*************************** 1. row ***************************
            change_key: I14b4df1adc7fcf329aca3cad404d671451cd2cc8
            created_on: 2012-12-04 09:33:56
       last_updated_on: 2012-12-06 09:45:38
              sort_key: 002190d5000007f3
      owner_account_id: 43
     dest_project_name: mmrd
      dest_branch_name: refs/heads/rel8.10
                  open: N
                status: M
        nbr_patch_sets: 2
  current_patch_set_id: 2
               subject: D-13518 Clicking back and forth within sections in patient info skinny list makes correct/refreshed display in chart browser.
                 topic: D-13518
last_sha1_merge_tested: NULL
             mergeable: Y
           row_version: 8
             change_id: 2035
*************************** 2. row ***************************
            change_key: I14b4df1adc7fcf329aca3cad404d671451cd2cc8
            created_on: 2012-12-04 09:58:08
       last_updated_on: 2012-12-06 09:10:48
              sort_key: 002190b2000007f6
      owner_account_id: 43
     dest_project_name: mmrd
      dest_branch_name: refs/heads/rel9.00
                  open: N
                status: M
        nbr_patch_sets: 3
  current_patch_set_id: 3
               subject: D-13518 Clicking back and forth within sections in patient info skinny list makes correct/refreshed display in chart browser.
                 topic: D-13518
last_sha1_merge_tested: NULL
             mergeable: Y
           row_version: 10
             change_id: 2038
2 rows in set (0.00 sec)

mysql> select * from patch_sets where change_id=2035 or change_id=2038 \G
*************************** 1. row ***************************
           revision: 496bd9f7f53212621f9ef39e8cddf2c504e50921
uploader_account_id: 43
         created_on: 2012-12-04 09:33:56
              draft: N
          change_id: 2035
       patch_set_id: 1
*************************** 2. row ***************************
           revision: 2e3c97e1da925a0d321a54ce1680bf376ff10a2f
uploader_account_id: 43
         created_on: 2012-12-05 13:24:00
              draft: N
          change_id: 2035
       patch_set_id: 2
*************************** 3. row ***************************
           revision: 3f7ab6b9237307c3eed773e4a37789eae3f8e511
uploader_account_id: 43
         created_on: 2012-12-04 09:58:08
              draft: N
          change_id: 2038
       patch_set_id: 1
*************************** 4. row ***************************
           revision: 2e3c97e1da925a0d321a54ce1680bf376ff10a2f
uploader_account_id: 43
         created_on: 2012-12-05 13:18:41
              draft: N
          change_id: 2038
       patch_set_id: 2
*************************** 5. row ***************************
           revision: 2565be7ea8ba62c6c04a22de8fce78b3105cc69b
uploader_account_id: 43
         created_on: 2012-12-05 13:28:26
              draft: N
          change_id: 2038
       patch_set_id: 3
5 rows in set (0.00 sec)


So, we think the best thing would be to prevent this from ever happening. Barring that, it would help us if we could set the reviewer via ssh somehow. The documentation says you can set via "numeric change identifiers," but that doesn't seem to work either. 
 
Dec 2, 2013
#3 zu.bruce.china@gmail.com
2012 year records? 

you can use  older style "change,patchset" instead. 
Dec 3, 2013
#4 saxo...@gmail.com
Yes, we filed the ticket in 2012.

Using "change,patchset" doesn't solve the problem. The "change,patchset" pair is not unique, because a change id can be reused.

This has happened once more since we filed the ticket. It doesn't happen very often, but it does happen.