From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.ffmpeg.org (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTPS id 79A5E4C829 for ; Tue, 5 Aug 2025 22:18:13 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTP id 4607F68C985; Wed, 6 Aug 2025 01:18:09 +0300 (EEST) Received: from mout.gmx.net (mout.gmx.net [212.227.17.20]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTPS id 989F668C7C7 for ; Wed, 6 Aug 2025 01:18:02 +0300 (EEST) X-UI-Sender-Class: 724b4f7f-cbec-4199-ad4e-598c01a50d3a Received: from metallschleim.local ([91.62.6.9]) by mail.gmx.net (mrgmx104 [212.227.17.168]) with ESMTPSA (Nemesis) id 1N2mBQ-1udtjB3LJM-00rSWd for ; Wed, 06 Aug 2025 00:18:01 +0200 Date: Wed, 6 Aug 2025 00:18:00 +0200 To: FFmpeg development discussions and patches Message-ID: References: <20250803153139.GC29660@pb2> <20250803190234.GE29660@pb2> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-Provags-ID: V03:K1:5XmZc7ImYv4RoM1otnGKtsmOKeoeAoRmV6q04Vsd51iL1f+tDLf iBeCr/4dKv15947c2nzB8DjhhDQPG1jM3L2YeEJCuhOaqQkZ1NJxhXUTMjOBNK1JSwhfcCf L/X2ZZrXvgcS34DfyxsPzZ0nFtGzrAaMRLKwLx7V6M1u0qk1YVry+S8+uOLP12qLDvBZlta 21tIaH5n+zaTKALH8wFkw== X-Spam-Flag: NO UI-OutboundReport: notjunk:1;M01:P0:c1CqTx9Jg5Q=;Vg6qHx1QSAiEeuX1Oqc6hfJ9A2p xit45czqr3LybEYp22ZRBPNZiLysh+E9nYyj62RM9SFVQzjTQij2xheRlPsYgF+gNiGmR90OK MrhzlAU6XL9L04E6ny3gnpz0ZA32OQOEYx/2kGs+S/BykgagOOwCisLmdjfcAsrS6xOdL4e9y LAf0AamN/+uLBqLYUwaw9W+XJKn1bKHhttPOjYknQEqyOvo09I5rdYsZJTRRa8e6RuvKR12CB QJmVtkoS1/zO5BpUZq2NymkPXV5q7pSqBDjlT6VM5FokqGuI2C582bxDTD9JmhsL2QN4JnCtm sfWsmrTZJsWqssRzroFDjI0OXXoXFqDny40/mKu99C58CqOtYhvvAEab5QgoNvVmTIRDJOR6v XjwClArt5XyDmUlvhH5JLG9c8sZDzjiZgdBIrhmub+QGr+dPiQOAlm/8kLcN1su5bctS0oWmQ zdML9F73oAalhdyv2uOy64i2LxRqz7kPy08bK0SDFVkQu4xuy61PikMH0lsGmtXU4R6S7pOQd k3lMuzqolg0NUmPB0fFA4s2BfIF60XbHD7g/7HS63++p1/LIFPfZlKF9DR6RAM8qPEOE4Te0y huxz87yot8J1MryuBEfvapf2QgCL64WRAuUru6j1PwHSwF2mxeDX0jWmiCz/uPgJVB06d9J6b vnxlGBfMdkbgxioaq7YQCtG/8wpU5J+godfZVSIMWqIdxA4a9p/Pi+t8Fi0YawK7qE3R8HjuX fIsT0kwyvwQAy+JBFnLH1JRYk+LdazUzZZRz1ZrU84DnhqB1pxwR+kG11k3HX6bDfCgE5kee5 4rhpzG0p/dQjzdazAZJTP+QF/jtseZHbv4glN6dzPMGZJXu3nTj1heWDec1zsSpFNFWWpqDXN m3B2L1Nb0R/AIIsETfiGyQz14am8vNmg4lPaadzHQNkqbIrGk+cm/i/t4o7OJ6u1WGvYz7c/U GzZQM2vKVHWz7OC4EiEXZOj2cQ9+L5A6jd9fQXt2S/hPaeLe614uot392Fh4+qFPydB1ZNq9K h1bqDukIliE8e8+cHR3fjb9UGX3H+coTfX6cd9YeuVKPmBKjL6Gro01ATbu+pXmHQgvTNJ4we 2fBlKBjCDHcQRmdmJk+F5ucx4FwctZqI0vfkat+HG/V8TTEPI0r2BKu3wwRpLgcFW7pc7U1Z0 KJsi0Sa+wDCoisf3stNU/HJJypoU2I1M/2DpFqiCRTTRO7Tt2U+ZHoFXM6hMQyhx9bC/mhjZt 5Wo0FoIYlwgVeShz5wqnZlGMpBgtI14F5mvo434g2FSiDxSVu2xeBdj6Q2FiN34p/pyEhF+L1 iH0H1LD/9kGbBbkwjP7K2HBr0T1h90MXr2Bs46YsE2GEZJsGtOvHiB+3wdjpwXiJi3zkrAcZU rv7TyFATuhKAOxwt55AEV34Csig3H71coOKJnB9+SAb6bU/a8LjtpXCLRLTMgGyMJTrpxZVZw Cy41ghfOUwlOnYR4b5XeCV2F3uOj5XiZhbq1/2Pztw25rB05cRGjsbhoAhV2dc3xJoYSEqaI/ Em0gbIva0RRLzmcK6duqo5L8p3PrB3D30g1NwBJ6COUWL671D3CLy3zzVSJJU22mCRrM5Td+w huOOxVlQRxPEMOj7Gz2jKfrXrGxLLOsEuxbSZEcfEf4yu5WiezqAIpJJaDlTU9jMz8eQiyaln 3oP70YsOuHx+T1WF7vWh1fBOWubDIfmDg1pfyY4Xg5kBPnUh2RYdwnjyPOqLCPVPNIuMFFRKu AsV/Za4gpYkqTKnkw8aowV0uwPMnuwlHni/vBIx1MW5fo6h+S8FMEXswQmi4Zn+DX964sHLFl Dqb74ngmieWtZ8kBiBCeAeIqQq5WCN4f72Z1c5khfYYfPm6umHH1rrIjrd3TJalGEstWf65YW /i0RKR4Wm4pgkeaGbYbrA06EuEx52Iu1kdGrfuTdik+43y6LtMjUlNPTyY4dz1mWAVrUP2hVe ap4RbEOr5nCi6LpmSXUvwvVhXo1GQZ5mBVdPtZTR4tixA6Yma6rC7BOg9ClHYCfem/m1A9pCK JKLsXIc5zjGjgHVf+tvU/WO9jbHDCAPcI3wsIaG6sagGsq5AKqLZKQaUWt8DZ4oXe+F/4cH48 ytk7IP2QhgeogH7tyRtjz+vvQ3AzT9B1k9J3x5NMp0aBcNo8at7kapjidT3AQItBx8ieb8Pls 52cIzzS2Sdp59PW8+QbDruRmq0i3+ykYgpgTaAwPHoiygp+PkHozTVsCrD20zM9b6fGVgL1JF 2aIb0nN0t8cIhWoQcA7zzcxLxRtS0r8We3xzbKeAGsPaCqa/Bu4HrjC4Viy5FbP7S6KR3yo4k J9R+n9TPDfSfJiuAaSflAu2vOgP7FdzsMqSGhom8jrVi9cQy8uawlkeEuHK1kw15LhMC7dDXQ QnzjhraD0+om7YQ1VpaRdFMFp1xatn84Fu2PxLGcTSyQgjwr1FAeyMbiKW/BPsGnKsqJbi/a0 6f0Ms5uoAtd1nSgZPjGVPQtQp1bMGJC6qfN1mT+7Lv/XBKEU0Aj126s9bE/S7L/ZcWKHBnu0n GL4FDaLo+s52cDyFoiG0cGxkqBX1ASW6W3/LcT5dIBIm7qcGKs8aynWkto8RxOth1YoDEEGV3 mhpaHETDFuEeop5ckEl+4mrI+4uTBAUNe0tnA0Wd3NIwsF4LBNF/WpMFBL1R2WqiAXSqqJ2Tp XcpBfsHvCmMOTawW8aoHSChjaa+uqfyB+41VQZAQlwkWipXOjJTFreCfKgvzo+zaSrCvpenA7 CiXR5o1RHS9195huSjvDi7RzF1SsXrKMdz8ENxdfMvFGX0eClvFEywyA3OTvMzQCHagVDTm8Z Lb1HVjooLHUoM5Isj7GYIFd1f7Onv00FKq1mQ9ur0TtCAWG43wYlnvY7+5P7rgZLd/C6xUOkT jLC4lD5uyZCErJYvs1QJxM72riitc/bhDstZpIfW7Az5aOdnZfx1UEKv/+v1okQo8/xyOXgg4 C8AXdViBG/7g0t9X+93e9GPnxOLDrOnKT9y2CIn/1ns5cslKVxbVAB+6+FOONB7LNdhAbuWUH FTgR9V3iccgxj6i+3CD5/Ic9cOG2O7XqL+jCOD8OATD8r+ONaqdpc3BaJZ0bXVRnmHlOrhRLu JLYimu3998vdWiuOCX+5IgL9XiGdwilObQM/e8dlEl+hjKfAzfQommEfC64HQhG9tzcyWYFk3 Yx8Lkyn6E1QQzrCSwXhzeb4q1+k5CsQV20Pd4bcx8xc0oYKBygFM7AXpVoEIW6xuKHyWhd2ck xuBBJNRt/Vhm5qa/OlH0uNlHv+/h1giF5ns95dVYG5eOqbHFBlSNYyou8/QYBoH/Em9Q9GbSC HPsMif1fNzx+LlpX6UizUg2YC08qEQzWVrInhXaYR03cOgyxYPptOhvJgfyrKC9TznhKGX/ Subject: Re: [FFmpeg-devel] rebasing security X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Alexander Strasser via ffmpeg-devel Reply-To: FFmpeg development discussions and patches Cc: Alexander Strasser Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On 2025-08-05 05:06 +0200, Kacper Michajlow wrote: > On Mon, 4 Aug 2025 at 23:38, Marton Balint wrote: > > > > On Mon, 4 Aug 2025, Alexander Strasser via ffmpeg-devel wrote: [...] > > > > > > If I understand the original point you wanted to discuss correctly, > > > than this is not a question of rebase or merge but one of letting > > > **commits happen on the forge**. If it happens it bears the > > > possibility of modification on the server the forge is running on. > > > > > > TL;DR: I think it's fine the way it's setup now. > > > > > > I'm not against letting rebase/merges happen on the server because > > > otherwise we would lose a lot of advantages and comfort we get by > > > using a forge for PRs. > > > > > > Only alternative I see is to do PRs on the forge and doing merging > > > manually by the same person that ensures reviewed PR is not changed > > > and pushes (after rebase or with a clean merge commit) from their > > > machine. > > > > Two things came to my mind about the current forgejo workflow. > > > > - Previously it was pretty clear from git history who actually committed > > a change from the comitter field. With using forgejo the comitter > > field no longer shows the person who actually *committed* the change to > > the main repo, but it is inherited from the original pull request commit > > instead, so it simply shows the original author of the patch. > > I don't think this is accurate. Committer field is set to the person > who clicks the "merge" button. Same as they would manually git push > the patches. > > Slightly related, I don't like how simple the web ui commit log of > forgejo is, it doesn't show commiter at all. For me this information > is as important as the author. I'm keeping notes on forgejo usage and > will share it when the time comes, it has some annoying limitations > compared to other forges. > > > - A pull request is writable both by the reviewer and the author up to > > the point when it is actually committed to the main repo. So force > > pushes from an author can happen anytime during this timeline: > > - reviewer reads changes > > - approves the changes > > - rebases the branch > > - sets it up to auto merge > > - CI actually runs > > - forgejo auto-merge > > A reviewer may not realize the new force push from the author. Maybe > > forgejo handle some force pushes in this timeline gracefully and aborts, > > or ignores them, I am not sure. It still looks a bit fragile, my > > expectation as a reviewer would be that what I saw when I finished the > > review and clicked on the Approve button will get comitted, when I later > > click on the merge button. > > There is a timeline of events. If PR is approved, you can see if there > were new events (comments/pushes) after that. This is close to the > "merge" button and you should see "approve" as the last event in the > timeline to be sure nothing changes. > > Also if there were rebases after approval, the approved tick mark (in > Reviewers list) becomes yellow instead of green. > > It is also possible to configure to completely discard previous > approval if any push happened. Right, that is what should be enforced by our forgejo. I think other ways are not good for our constellation of collaboration. Anyway while this is an important point, the original danger I mentioned is orthogonal to that and still there. Something could go wrong inside forgejo and get pushed into our main git repo. So if anybody is deeming this to be to risky, now would be the moment to speak up. Alexander _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".