Gerrit prevents upstreaming patches from others

classic Classic list List threaded Threaded
13 messages Options
Paul Menzel Paul Menzel
Reply | Threaded
Open this post in threaded view
|

Gerrit prevents upstreaming patches from others

Dear LibreOffice folks,


I was asked to push a commit for review to the LO Gerrit instance [1].

The patch is from a colleague, who is denoted as the commit author. No
Gerrit complains, that I am not the author, and my colleagues email
address is not registered to *my* Gerrit account.

Could you please change that, and disable that limitation? I don’t see
any reason for it.

At least the committer address should also be looked at. Or some kind of
Signed-off-by procedure.


Kind regards,

Paul


[1] https://bugs.documentfoundation.org/show_bug.cgi?id=100459
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
jan iversen jan iversen
Reply | Threaded
Open this post in threaded view
|

Re: Gerrit prevents upstreaming patches from others

Hi

The gerrit allows people with committer status, to push patches for other people. In that case the email addr, from the person pushing becomes committer email, and the email from from patch owner becomes the author.

To do this please remember to use —author and —author-email in the commit statement.

No-committers cannot push patches on behalf of of others, because it would require 3 emails addresses in git (the email who merges the patch (the committer), the one who submitted the patch (the committer) and the author).

I am happy to help you push the patch, with the correct author.

rgds
jan I.

_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Paul Menzel Paul Menzel
Reply | Threaded
Open this post in threaded view
|

Re: Gerrit prevents upstreaming patches from others

Dear Jan,


On 02/06/17 10:32, Jan Iversen wrote:

> The gerrit allows people with committer status, to push patches for other people. In that case the email addr, from the person pushing becomes committer email, and the email from from patch owner becomes the author.
>
> To do this please remember to use —author and —author-email in the commit statement.

Thank you, but I knew that already.

(Please note, that your mailer mangles `--` to `—`.

> No-committers cannot push patches on behalf of of others, because it would require 3 emails addresses in git (the email who merges the patch (the committer), the one who submitted the patch (the committer) and the author).

I don’t see how that is required. In Gerrit, it would still be clear,
who pushed the patch (owner).

As written, Signed-off-by lines would be a solution, if you want to
record that information.

> I am happy to help you push the patch, with the correct author.

Please tell me how. Do you want me send the patch to the mailing list?


Kind regards,

Paul Menzel
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
jan iversen jan iversen
Reply | Threaded
Open this post in threaded view
|

Re: Gerrit prevents upstreaming patches from others


> I don’t see how that is required. In Gerrit, it would still be clear, who pushed the patch (owner).
yes but you have one step more merging to git, and if that is done by a third person information would be lost.

> Please tell me how. Do you want me send the patch to the mailing list?
yes or not to send it to hundreds of people, mail it to [hidden email] and I will submit it.

please remember to include the author details.

rgds
jan i

_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Paul Menzel Paul Menzel
Reply | Threaded
Open this post in threaded view
|

Re: Gerrit prevents upstreaming patches from others

Dear Jan,


On 02/06/17 10:43, jan iversen wrote:
>
>> I don’t see how that is required. In Gerrit, it would still be clear, who pushed the patch (owner).
 >
> yes but you have one step more merging to git, and if that is done by a third person information would be lost.

Sorry, I didn’t understand *you have one step more merging to git*.

>> Please tell me how. Do you want me send the patch to the mailing list?
 >
> yes or not to send it to hundreds of people, mail it to [hidden email] and I will submit it.
>
> please remember to include the author details.

Will do. Thank you.


Kind regards,

Paul
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Christian Lohmaier-3 Christian Lohmaier-3
Reply | Threaded
Open this post in threaded view
|

Re: Gerrit prevents upstreaming patches from others

In reply to this post by Paul Menzel
Hi  Paul, *,

On Thu, Feb 2, 2017 at 1:05 PM, Paul Menzel
<[hidden email]> wrote:
>
> I was asked to push a commit for review to the LO Gerrit instance [1].
>
> The patch is from a colleague, who is denoted as the commit author. No
> Gerrit complains, that I am not the author, and my colleagues email address
> is not registered to *my* Gerrit account.

This is intentional.

> Could you please change that, and disable that limitation? I don’t see any
> reason for it.

It is to prevent people from impersonating somebody else.

Think about someone trying your email  to introduce a backdoor ...
as well as reducing mistakes (when you're  using different email
addresses, and didn't configure your email...

> At least the committer address should also be looked at. Or some kind of
> Signed-off-by procedure.

That is already in place, as mentioned by Jan.

ciao
Christian
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
jan iversen jan iversen
Reply | Threaded
Open this post in threaded view
|

Re: Gerrit prevents upstreaming patches from others

In reply to this post by Paul Menzel


>
> Sorry, I didn’t understand *you have one step more merging to git*.

Patches in Gerrit are for review, when reviewed a committer merged them to the git repo e.g, master, and from that time the are part of the LibreOffice code base.

rgds
jan i
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Paul Menzel Paul Menzel
Reply | Threaded
Open this post in threaded view
|

Re: Gerrit prevents upstreaming patches from others

Dear Jan,


On 02/06/17 10:56, jan iversen wrote:

>> Sorry, I didn’t understand *you have one step more merging to git*.
>
> Patches in Gerrit are for review, when reviewed a committer merged them to the git repo e.g, master, and from that time the are part of the LibreOffice code base.

Sorry, I still don’t see where the additional step is?


Kind regards,

Paul


PS: Sorry, but I really can’t understand your arguments. Maybe because
of the language barrier.
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Paul Menzel Paul Menzel
Reply | Threaded
Open this post in threaded view
|

Re: Gerrit prevents upstreaming patches from others

In reply to this post by Christian Lohmaier-3
Dear Christian,


On 02/06/17 10:55, Christian Lohmaier wrote:

> On Thu, Feb 2, 2017 at 1:05 PM, Paul Menzel wrote:
>>
>> I was asked to push a commit for review to the LO Gerrit instance [1].
>>
>> The patch is from a colleague, who is denoted as the commit author. No
>> Gerrit complains, that I am not the author, and my colleagues email address
>> is not registered to *my* Gerrit account.
>
> This is intentional.
>
>> Could you please change that, and disable that limitation? I don’t see any
>> reason for it.
>
> It is to prevent people from impersonating somebody else.
>
> Think about someone trying your email to introduce a backdoor ...

In my opinion that’s highly hypothetical. And if that happens, it’ll be
figured out in no time from the Gerrit log, that it wasn’t really the
impersonated person.

> as well as reducing mistakes (when you're  using different email
> addresses, and didn't configure your email...

Then you would still get that warning, as your committer data is also
not matched.

The coreboot project doesn’t have these restrictions, and in the past
there hasn’t been any problems.

Have there been actual problems in the past?

>> At least the committer address should also be looked at. Or some kind of
>> Signed-off-by procedure.
>
> That is already in place, as mentioned by Jan.

At least it’s not documented.


Kind regards,

Paul
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
jan iversen jan iversen
Reply | Threaded
Open this post in threaded view
|

Re: Gerrit prevents upstreaming patches from others

In reply to this post by Paul Menzel

> Sorry, I still don’t see where the additional step is?
>
you submit the patch to gerrit, that does not put it into our core repo, but only to the gerrit review repo. In order to merge the patch into our core repo(typically master) a committer, needs to hit merge in gerrit, then will mark the patch as merged in gerrit, and commit it to the master repo.

the committer that does the merge becomes the commiter in git, the author in our core repo becomes:
1) gerrit author, if gerrit committer == committer who does the merge
2) else gerrit committer.

rgds
jan i

_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Paul Menzel Paul Menzel
Reply | Threaded
Open this post in threaded view
|

Re: Gerrit prevents upstreaming patches from others

Dear Jan,


On 02/06/17 12:00, jan iversen wrote:

>> Sorry, I still don’t see where the additional step is?
>
> you submit the patch to gerrit, that does not put it into our core repo, but only to the gerrit review repo. In order to merge the patch into our core repo(typically master) a committer, needs to hit merge in gerrit, then will mark the patch as merged in gerrit, and commit it to the master repo.
>
> the committer that does the merge becomes the committer in git, the author in our core repo becomes:
 >
> 1) gerrit author, if gerrit committer == committer who does the merge

Sorry, what is the “gerrit author”? Do you mean the owner?

> 2) else gerrit committer.

Do you have an example commit, where that happened? I think, then I
would understand it better.

I still don’t understand, why the author metadata should be changed in
the first place.


Kind regards,

Paul
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Christian Lohmaier-3 Christian Lohmaier-3
Reply | Threaded
Open this post in threaded view
|

Re: Gerrit prevents upstreaming patches from others

In reply to this post by Paul Menzel
Hi Paul,

On Mon, Feb 6, 2017 at 11:08 AM, Paul Menzel
<[hidden email]> wrote:
> On 02/06/17 10:55, Christian Lohmaier wrote:
>> On Thu, Feb 2, 2017 at 1:05 PM, Paul Menzel wrote:
>>
>> It is to prevent people from impersonating somebody else.
>>
>> Think about someone trying your email to introduce a backdoor ...
>
> In my opinion that’s highly hypothetical.

It is exaggerating to illustrate the point. It doesn't matter what
actual impact that change has.

> And if that happens, it’ll be
> figured out in no time from the Gerrit log, that it wasn’t really the
> impersonated person.

How would you be able to tell?
You might be able to tell that the email address is not matching what
the user has configured. But you cannot tell whether the user he was
claiming to be actually was involved.

Let's say there was no such limitation, and I'd commit something as
"Donald J Trump <[hidden email]>" and claim "I talked to him, he
did that patch" - how would you know that'd be the case? And how would
you know he'd be fine with our licencing requirements?
Again exaggerated example.

> The coreboot project doesn’t have these restrictions, and in the past there
> hasn’t been any problems.

So far nobody stole anything from my car, but I still lock it up.

If there were a way to impersonate as somebody else, then checking for
the licence agreements and other stuff just becomes too hard/you'll
run into the problem of deniability.

ciao
Christian
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Paul Menzel Paul Menzel
Reply | Threaded
Open this post in threaded view
|

Re: Gerrit prevents upstreaming patches from others

Dear Christian,


On 02/06/17 12:17, Christian Lohmaier wrote:

> On Mon, Feb 6, 2017 at 11:08 AM, Paul Menzel
> <[hidden email]> wrote:
>> On 02/06/17 10:55, Christian Lohmaier wrote:
>>> On Thu, Feb 2, 2017 at 1:05 PM, Paul Menzel wrote:
>>>
>>> It is to prevent people from impersonating somebody else.
>>>
>>> Think about someone trying your email to introduce a backdoor ...
>>
>> In my opinion that’s highly hypothetical.
>
> It is exaggerating to illustrate the point. It doesn't matter what
> actual impact that change has.
>
>> And if that happens, it’ll be
>> figured out in no time from the Gerrit log, that it wasn’t really the
>> impersonated person.
>
> How would you be able to tell?
> You might be able to tell that the email address is not matching what
> the user has configured. But you cannot tell whether the user he was
> claiming to be actually was involved.
>
> Let's say there was no such limitation, and I'd commit something as
> "Donald J Trump <[hidden email]>" and claim "I talked to him, he
> did that patch" - how would you know that'd be the case? And how would
> you know he'd be fine with our licencing requirements?
> Again exaggerated example.
>
>> The coreboot project doesn’t have these restrictions, and in the past there
>> hasn’t been any problems.
>
> So far nobody stole anything from my car, but I still lock it up.

Sorry, car theft is a reality.

Somebody could shoot me on the street, so I shoot them first? Preventive
strikes …?

> If there were a way to impersonate as somebody else, then checking for
> the licence agreements and other stuff just becomes too hard/you'll
> run into the problem of deniability.

Sorry, using the email address as verification is fundamentally flawed.
That’s why GPG exists.

I just register `chris.lohmaier` at any free provider, and send in
commits for you, without any error from Gerrit.

So, to close my participation in this thread, the current restriction
make it hard for people wanting to upstream patches from colleagues.

The LibreOffice people should really think about it again, as from the
current arguments, the restriction *cannot by design* enforce the policy
it was made for.


Kind regards,

Paul
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice