Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Wikipedia: Wikitech

Caveat: git merge and git cherry-pick don't add "Change-Id" by default

 

 

Wikipedia wikitech RSS feed   Index | Next | Previous | View Threaded


saper at saper

Mar 24, 2012, 9:21 PM

Post #1 of 4 (1796 views)
Permalink
Caveat: git merge and git cherry-pick don't add "Change-Id" by default

When playing with moving (cherry picking) commits from master to one
of the release branches (REL1_19) I noticed that "git cherry-pick"
and "git merge" do not invoke "commit-msg" hook and therefore don't
add Change-Id to the commits.

This is normally not a problem for people who are allowed changes
without "Change-Id" to the repository (i.e. trunk gatekeepers),
but this may add some problems for committers at large.

1) you can't push result of such merge or cherry-pick for review

2) you can't directly cherry-pick a commit imported from SVN
into git master (those have no change-ID's) to one of
the branches.

3) merge commits done by gatekeepers are not visible to gerrit
(you can't find them for example when searching by ID).
Therefore we loose ability to comment on them if necessary.

Here is an example how it failed on me today:

https://www.mediawiki.org/w/index.php?title=Git/Workflow&diff=prev&oldid=515259

Sometimes a "git commit -c <original commit>" or "git commit --amend"
are able to fix the issue, because "git commit" DOES invoke the hook.

This will be especially important for developers wanting to propose
their changes into release branches or deployment branches.

This may also bite you if you are using some private branches,
all commits there have Commit-IDs but merges will not have
and you may have hard time push it all together for review.

It is also interesting to see how Change-IDs represent
multiple review items (I52150208654fa14e02b6d80fb2cff4108089ef6c
is https://gerrit.wikimedia.org/r/3713 and
https://gerrit.wikimedia.org/r/3714).

I think the right workaround right now is to make sure
all commits (even merges or things going directly into git master
for some reason) have their gerrit "Change-ID".

With cherry pick it can be done by doing it in two stages:
git cherry-pick -n <revision>
git commit -c <revision>
(but you might want to improve the commit message).

Something similar can be done for merges probably
(there is --no-commit option to git merge).

There are probably some other git commands causing automatic
commit that may have this problem. Fast forwards are
fine, as they don't produce commits.

I did some of the testing today:

https://gerrit.wikimedia.org/r/#q,project:test/mediawiki/core+owner:saper%2540saper.info,n,z

//Saper


_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


roan.kattouw at gmail

Mar 25, 2012, 2:50 AM

Post #2 of 4 (1765 views)
Permalink
Re: Caveat: git merge and git cherry-pick don't add "Change-Id" by default [In reply to]

On Sat, Mar 24, 2012 at 9:21 PM, Marcin Cieslak <saper [at] saper> wrote:
> When playing with moving (cherry picking) commits from master to one
> of the release branches (REL1_19) I noticed that "git cherry-pick"
> and "git merge" do not invoke "commit-msg" hook and therefore don't
> add Change-Id to the commits.
>
> This is normally not a problem for people who are allowed changes
> without "Change-Id" to the repository (i.e. trunk gatekeepers),
> but this may add some problems for committers at large.
>
This is an issue that has been known to us for some time, but I guess
it was never documented properly. Running git commit --amend after a
merge or cherry-pick has always added a Change-Id for me.

Roan

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


roan.kattouw at gmail

Mar 26, 2012, 11:30 AM

Post #3 of 4 (1755 views)
Permalink
Re: Caveat: git merge and git cherry-pick don't add "Change-Id" by default [In reply to]

On Sat, Mar 24, 2012 at 9:21 PM, Marcin Cieslak <saper [at] saper> wrote:
> This is normally not a problem for people who are allowed changes
> without "Change-Id" to the repository (i.e. trunk gatekeepers),
> but this may add some problems for committers at large.
>
Let me debunk that myth right there: NO ONE is allowed to submit
changes for review that don't have Change-IDs, not even core
reviewers. AFAIK mediawiki/core is not configured to allow direct
pushes bypassing review by anyone, but even people that have this
ability can't magically submit things into Gerrit without Change-IDs.

Roan

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


tstarling at wikimedia

Mar 26, 2012, 9:09 PM

Post #4 of 4 (1755 views)
Permalink
Re: Caveat: git merge and git cherry-pick don't add "Change-Id" by default [In reply to]

On 25/03/12 15:21, Marcin Cieslak wrote:
> When playing with moving (cherry picking) commits from master to one
> of the release branches (REL1_19) I noticed that "git cherry-pick"
> and "git merge" do not invoke "commit-msg" hook and therefore don't
> add Change-Id to the commits.
>

Also "git revert" suffers the same problem.

-- Tim Starling


_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Wikipedia wikitech RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.