Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

git squash by author does not squash if multiple authors touching the same file #13828

Closed
2 tasks done
hefee opened this issue Feb 11, 2025 · 2 comments · Fixed by #13832
Closed
2 tasks done

git squash by author does not squash if multiple authors touching the same file #13828

hefee opened this issue Feb 11, 2025 · 2 comments · Fixed by #13832
Assignees
Labels
bug Something is broken.
Milestone

Comments

@hefee
Copy link

hefee commented Feb 11, 2025

Describe the issue

When use squash by author and a component is touched by two authors squash fails and all commits are not handled at all. From the code it already looks like it should handle those case, but it does not.

I saw that the test does not test this case. I have a suggestion for a test case, but I do not get the tests running on my machine, so I cannot test if this testcase matches the issue already. I expect that the additional commit needs to happen on a different unit:

From 049d96eb6714433763f487d0ac0589736d222d27 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sandro=20Knau=C3=9F?= <[email protected]>
Date: Tue, 11 Feb 2025 21:14:08 +0100
Subject: [PATCH] Add test for multiple users edit same component.

---
 weblate/addons/tests.py | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/weblate/addons/tests.py b/weblate/addons/tests.py
index 03999c9039..f7bd926343 100644
--- a/weblate/addons/tests.py
+++ b/weblate/addons/tests.py
@@ -1234,7 +1234,8 @@ class GitSquashAddonTest(ViewTestCase):
             self.component.commit_pending("test", None)

     def test_squash(
-        self, mode: str = "all", expected: int = 1, sitewide: bool = False
+        self, mode: str = "all", expected: int = 1, sitewide: bool = False,
+        edit_method: None
     ) -> None:
         addon = self.create(mode=mode, sitewide=sitewide)
         repo = self.component.repository
@@ -1242,7 +1243,10 @@ class GitSquashAddonTest(ViewTestCase):
         # Test no-op behavior
         addon.post_commit(self.component, True)
         # Make some changes
-        self.edit()
+        if not edit_method:
+            self.edit()
+        else:
+            edit_method()
         self.assertEqual(repo.count_outgoing(), expected)

     def test_squash_sitewide(self) -> None:
@@ -1260,10 +1264,16 @@ class GitSquashAddonTest(ViewTestCase):

     def test_author(self) -> None:
         self.test_squash("author", 2)
-        # Add commit which can not be squashed
-        self.change_unit("Diky za pouzivani Weblate.", "Thank you for using Weblate.")
-        self.component.commit_pending("test", None)
-        self.assertEqual(self.component.repository.count_outgoing(), 3)
+
+    def test_multiple_authors_on_same_file(self) -> None:
+        def edit() -> None:
+            self.edit()
+
+            # Add commit on the same unit, that is touched by anotheruser
+            self.change_unit("Danke für die Benutzung von Weblate.", "Thank you for using Weblate.", lang="de")
+            self.component.commit_pending("test", None)
+
+        self.test_squash("author", 3, edit_method=edit)

     def test_commit_message(self) -> None:
         commit_message = "Squashed commit message"
--
2.47.2

I already tried

  • I've read and searched the documentation.
  • I've searched for similar filed issues in this repository.

Steps to reproduce the behavior

The problem is something like:

  • user1 edits comp1
  • user1 edits comp1
  • user1 edits comp2
  • user2 edits comp2
  • user1 edits comp2

In a way, that squash aurthor can merge user1 commits but fails to cherry-pick user2 with a "Merge conflict".

The behaviour currently is "Weblate do not squash at all and fails".

run git squash_author with the commits for the https://gitlab.tails.boum.org/tails/tails.git:
master = 8ecd43e9e99d4ebc0a829bbdd91a584b6908ebdd
origin/master = f2215b5e878d0c2a86fb0b9d3a4cf9eaa93696db

Expected behavior

Weblate squashes at much as possible aka:
3 commits:
user1 edits comp1&comp2
user2 edits comp2
user1 edits comp2

Exception traceback

I added some print statement to squash_author to get some insight information. 
* "handled" - this commit is added to the handled list
* "skipped" - this commit is skipped, because of wrong author
* "failed" - RepositoryError is triggered

start with commit='709dbc3a1ce8bdcd0c41f9e8a06321f36f2e73bd' author='victordargallo'
['4610ebefc920fd5e8265a2ec12647bcbb1af66f3', 'victordargallo']
 handled
['3c7fdd34ac39d8444406381c3721d3e8ffb6691b', 'victordargallo']
 handled
['6897b84b20307b48fc21527fa9ab511515c5d4c4', 'victordargallo']
 handled
['7a991ed21442782da0312a4c863b7e66802fb8ae', 'victordargallo']
 handled
['144a99bcc9a68c9ba1289074f324b3b712fe850c', 'victordargallo']
 handled
['ef3c0fb20f9750246f95945c1f58e613bedc3f0d', 'victordargallo']
 handled
['1424706bf1be9a6ef09c35f3b1b43f42ac581046', 'victordargallo']
 handled
['49c3f8bef0f187e14e0d97d4ca0bc6816f8260a0', 'victordargallo']
 handled
['9a81c0ced8733535759065ece51125ecb2994407', 'victordargallo']
 handled
['cb9c785a156641fb1959b75564c1cd2f496db4b5', 'victordargallo']
 handled
['67e792cd247836a90356605c88848e790e813cf7', 'cacukin']
 skipped
['ec2150498e425a48416e31b1271476c7eab405e0', 'victordargallo']
 handled
['e2b46680972b5dedc50630241c0477144453b6a3', 'victordargallo']
 handled
['1ffe6350d4a8ef67bd8d812de78f931f0fa69bcc', 'victordargallo']
 handled
['bdc17241dff8a5a9d4dd45ebebd7e478b3f17d24', 'cacukin']
 skipped
['bcf161a49f8dedc19dd003d4dee46db564133822', 'cacukin']
 skipped
['66596ed22bb8dd3fef040d87a2f9841f5c3dd8d4', 'victordargallo']
 handled
['a76cdce82bfd821da2eba6fccd1e5c2d755669cb', 'victordargallo']
 handled
['a47013a826551851e367742203202afafb818491', 'cacukin']
 skipped
['752ec9182f42cba174bcf5c6d5764b3ed5d60a29', 'victordargallo']
 handled
['0dbd0ae9996ec2c23a8e2909a2a3e13193e19a1e', 'victordargallo']
 failed
start with commit='67e792cd247836a90356605c88848e790e813cf7' author='cacukin'
Failed squash :(
[2025-02-11 21:04:22,448: WARNING/18118] Failed squash: RepositoryError: Auto-merging wiki/src/doc/persistent_storage/additional_software/dangerzone.es.po
CONFLICT (content): Merge conflict in wiki/src/doc/persistent_storage/additional_software/dangerzone.es.po
error: could not apply 67e792cd247... Translated using Weblate (Spanish)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

How do you run Weblate?

Docker container

Weblate versions

Docker version v5.3.0.1

Weblate deploy checks

Additional context

You can find the testcase in our repo. The test sets the git commits as expected and calls gitsquash.py:
https://gitlab.torproject.org/tpo/tpa/puppet-weblate/-/blob/master/files/scripts/tests/test_gitsquash.py#L43

gitshquash.py is simplify calling squash_author on the main component (first component):
https://gitlab.torproject.org/tpo/tpa/puppet-weblate/-/blob/master/files/scripts/gitsquash.py

@nijel
Copy link
Member

nijel commented Feb 12, 2025

I've added the test #13830, and it works just fine locally.

I think the problem in your case is that Weblate squashes too much. It picks all commits from victordargallo, but these then block cherry-picking 67e792cd247836a90356605c88848e790e813cf7. Most likely because 1ffe6350d4a8ef67bd8d812de78f931f0fa69bcc has been already cherry-picked.

This might be caused by the custom merge driver Weblate which might end up doing weird things here. We should really figure out a way to use it only in merge and not in cherry-picking or rebasing...

nijel added a commit to nijel/weblate that referenced this issue Feb 12, 2025
In regular merge it it useful to merge timestamps in the PO file, but
this often causes issues in rebase (because the commits become
different) or squash (because it can apply changes before they should be
applied).

Fixes WeblateOrg#13828
@nijel nijel added the bug Something is broken. label Feb 12, 2025
@nijel nijel added this to the 5.10 milestone Feb 12, 2025
@nijel nijel self-assigned this Feb 12, 2025
nijel added a commit that referenced this issue Feb 12, 2025
In regular merge it it useful to merge timestamps in the PO file, but
this often causes issues in rebase (because the commits become
different) or squash (because it can apply changes before they should be
applied).

Fixes #13828
Copy link

Thank you for your report; the issue you have reported has just been fixed.

  • In case you see a problem with the fix, please comment on this issue.
  • In case you see a similar problem, please open a separate issue.
  • If you are happy with the outcome, don’t hesitate to support Weblate by making a donation.

nijel added a commit that referenced this issue Feb 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is broken.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants