packit / packit-service

Packit provided as a service
https://packit.dev
MIT License
37 stars 48 forks source link

Koji builds not triggered in branches where a merge has been fast forward if there are multiple commits in the merge. #2537

Open majamassarini opened 1 month ago

majamassarini commented 1 month ago

This is a Pagure bug and probably we should fix it in Pagure (I don't figure out a way to workaround it in Packit).

I had two commits in rawhide:

The above commits were fast forwarded in f39, f40 and f41:

The pagure.git.receive messages generated by the PRs merges are shown below. For rawhide the packit.spec file change contained in commit fe01a99843805bf2eed16146ae91e3e87333f52d is listed (since it is the only commit contained in the PR). For all the branched branches is listed only the packit.yaml file change contained in commit 44742d731eb4a74e5abf0c3a113b0609ccd0b745 even though the PRs contain two commits. The changes for commit fe01a99843805bf2eed16146ae91e3e87333f52d are not listed in the attribute changed_files.

We filter the events out for all branched branches because Pagure tell us there are no changes in the specfile.

It works retriggering it with a comment.

    {
      "body": {
        "total_commits": 2,
        "start_commit": "44742d731eb4a74e5abf0c3a113b0609ccd0b745",
        "end_commit": "fe01a99843805bf2eed16146ae91e3e87333f52d",
        "old_commit": "37bd1f0639e8f4c1140dd1d3060bfdb1e38c23ec",
        "branch": "f41",
        "forced": false,
        "authors": [
          {
            "name": "mmassari",
            "fullname": "Maja Massarini",
            "url_path": "user/mmassari",
            "full_url": "https://src.fedoraproject.org/user/mmassari"
          },
          {
            "fullname": "Packit",
            "name": null,
            "url_path": null
          }
        ],
        "changed_files": {
          ".packit.yaml": "M"
        },
        "agent": "mmassari",
        "repo": {
          "id": 34882,
          "name": "packit",
          "fullname": "rpms/packit",
          "url_path": "rpms/packit",
          "full_url": "https://src.fedoraproject.org/rpms/packit",
          "description": "A set of tools to integrate upstream open source projects into Fedora operating system",
          "namespace": "rpms",
          "parent": null,
          "date_created": "1552485797",
          "date_modified": "1672657173",
          "user": {
            "name": "lachmanfrantisek",
            "fullname": "František Lachman",
            "url_path": "user/lachmanfrantisek",
            "full_url": "https://src.fedoraproject.org/user/lachmanfrantisek"
          },
          "access_users": {
            "owner": [
              "lachmanfrantisek"
            ],
            "admin": [
              "lbarczio",
              "mfocko",
              "ttomecek"
            ],
            "commit": [
              "mmassari",
              "nforro",
              "nikromen"
            ],
            "collaborator": [],
            "ticket": []
          },
          "access_groups": {
            "admin": [],
            "commit": [],
            "collaborator": [],
            "ticket": []
          },
          "tags": [
            ""
          ],
          "priorities": {},
          "custom_keys": [],
          "close_status": [],
          "milestones": {}
        },
        "pull_request_id": 1287
      },
      "headers": {
        "sent-at": "2024-09-18T13:32:33+00:00",
        "fedora_messaging_schema": "pagure.git.receive",
        "fedora_messaging_severity": 20,
        "fedora_messaging_user_mmassari": true
      },
      "id": "c14bd6d4-8def-4795-8a20-ccd2cab41132",
      "priority": 0,
      "queue": null,
      "topic": "org.fedoraproject.prod.pagure.git.receive"
    },
    {
      "body": {
        "total_commits": 2,
        "start_commit": "44742d731eb4a74e5abf0c3a113b0609ccd0b745",
        "end_commit": "fe01a99843805bf2eed16146ae91e3e87333f52d",
        "old_commit": "37bd1f0639e8f4c1140dd1d3060bfdb1e38c23ec",
        "branch": "f39",
        "forced": false,
        "authors": [
          {
            "name": "mmassari",
            "fullname": "Maja Massarini",
            "url_path": "user/mmassari",
            "full_url": "https://src.fedoraproject.org/user/mmassari"
          },
          {
            "fullname": "Packit",
            "name": null,
            "url_path": null
          }
        ],
        "changed_files": {
          ".packit.yaml": "M"
        },
        "agent": "mmassari",
        "repo": {
          "id": 34882,
          "name": "packit",
          "fullname": "rpms/packit",
          "url_path": "rpms/packit",
          "full_url": "https://src.fedoraproject.org/rpms/packit",
          "description": "A set of tools to integrate upstream open source projects into Fedora operating system",
          "namespace": "rpms",
          "parent": null,
          "date_created": "1552485797",
          "date_modified": "1672657173",
          "user": {
            "name": "lachmanfrantisek",
            "fullname": "František Lachman",
            "url_path": "user/lachmanfrantisek",
            "full_url": "https://src.fedoraproject.org/user/lachmanfrantisek"
          },
          "access_users": {
            "owner": [
              "lachmanfrantisek"
            ],
            "admin": [
              "lbarczio",
              "mfocko",
              "ttomecek"
            ],
            "commit": [
              "mmassari",
              "nforro",
              "nikromen"
            ],
            "collaborator": [],
            "ticket": []
          },
          "access_groups": {
            "admin": [],
            "commit": [],
            "collaborator": [],
            "ticket": []
          },
          "tags": [
            ""
          ],
          "priorities": {},
          "custom_keys": [],
          "close_status": [],
          "milestones": {}
        },
        "pull_request_id": 1286
      },
      "headers": {
        "sent-at": "2024-09-18T13:31:56+00:00",
        "fedora_messaging_schema": "pagure.git.receive",
        "fedora_messaging_severity": 20,
        "fedora_messaging_user_mmassari": true
      },
      "id": "5b3d9e08-5e14-4bf2-83df-9f8fa8323f01",
      "priority": 0,
      "queue": null,
      "topic": "org.fedoraproject.prod.pagure.git.receive"
    },
    {
      "body": {
        "total_commits": 2,
        "start_commit": "44742d731eb4a74e5abf0c3a113b0609ccd0b745",
        "end_commit": "fe01a99843805bf2eed16146ae91e3e87333f52d",
        "old_commit": "37bd1f0639e8f4c1140dd1d3060bfdb1e38c23ec",
        "branch": "f40",
        "forced": false,
        "authors": [
          {
            "name": "mmassari",
            "fullname": "Maja Massarini",
            "url_path": "user/mmassari",
            "full_url": "https://src.fedoraproject.org/user/mmassari"
          },
          {
            "fullname": "Packit",
            "name": null,
            "url_path": null
          }
        ],
        "changed_files": {
          ".packit.yaml": "M"
        },
        "agent": "mmassari",
        "repo": {
          "id": 34882,
          "name": "packit",
          "fullname": "rpms/packit",
          "url_path": "rpms/packit",
          "full_url": "https://src.fedoraproject.org/rpms/packit",
          "description": "A set of tools to integrate upstream open source projects into Fedora operating system",
          "namespace": "rpms",
          "parent": null,
          "date_created": "1552485797",
          "date_modified": "1672657173",
          "user": {
            "name": "lachmanfrantisek",
            "fullname": "František Lachman",
            "url_path": "user/lachmanfrantisek",
            "full_url": "https://src.fedoraproject.org/user/lachmanfrantisek"
          },
          "access_users": {
            "owner": [
              "lachmanfrantisek"
            ],
            "admin": [
              "lbarczio",
              "mfocko",
              "ttomecek"
            ],
            "commit": [
              "mmassari",
              "nforro",
              "nikromen"
            ],
            "collaborator": [],
            "ticket": []
          },
          "access_groups": {
            "admin": [],
            "commit": [],
            "collaborator": [],
            "ticket": []
          },
          "tags": [
            ""
          ],
          "priorities": {},
          "custom_keys": [],
          "close_status": [],
          "milestones": {}
        },
        "pull_request_id": 1282
      },
      "headers": {
        "sent-at": "2024-09-18T13:31:39+00:00",
        "fedora_messaging_schema": "pagure.git.receive",
        "fedora_messaging_severity": 20,
        "fedora_messaging_user_mmassari": true
      },
      "id": "23f2c309-81e8-44ef-899c-c9a33a992b91",
      "priority": 0,
      "queue": null,
      "topic": "org.fedoraproject.prod.pagure.git.receive"
    },
    {
      "body": {
        "total_commits": 1,
        "start_commit": "fe01a99843805bf2eed16146ae91e3e87333f52d",
        "end_commit": "fe01a99843805bf2eed16146ae91e3e87333f52d",
        "old_commit": "44742d731eb4a74e5abf0c3a113b0609ccd0b745",
        "branch": "rawhide",
        "forced": false,
        "authors": [
          {
            "fullname": "Packit",
            "name": null,
            "url_path": null
          }
        ],
        "changed_files": {
          ".gitignore": "M",
          "README.packit": "M",
          "packit.spec": "M",
          "plans/main.fmf": "M",
          "sources": "M"
        },
        "agent": "mmassari",
        "repo": {
          "id": 34882,
          "name": "packit",
          "fullname": "rpms/packit",
          "url_path": "rpms/packit",
          "full_url": "https://src.fedoraproject.org/rpms/packit",
          "description": "A set of tools to integrate upstream open source projects into Fedora operating system",
          "namespace": "rpms",
          "parent": null,
          "date_created": "1552485797",
          "date_modified": "1672657173",
          "user": {
            "name": "lachmanfrantisek",
            "fullname": "František Lachman",
            "url_path": "user/lachmanfrantisek",
            "full_url": "https://src.fedoraproject.org/user/lachmanfrantisek"
          },
          "access_users": {
            "owner": [
              "lachmanfrantisek"
            ],
            "admin": [
              "lbarczio",
              "mfocko",
              "ttomecek"
            ],
            "commit": [
              "mmassari",
              "nforro",
              "nikromen"
            ],
            "collaborator": [],
            "ticket": []
          },
          "access_groups": {
            "admin": [],
            "commit": [],
            "collaborator": [],
            "ticket": []
          },
          "tags": [
            ""
          ],
          "priorities": {},
          "custom_keys": [],
          "close_status": [],
          "milestones": {}
        },
        "pull_request_id": 1275
      },
      "headers": {
        "sent-at": "2024-09-18T13:30:29+00:00",
        "fedora_messaging_schema": "pagure.git.receive",
        "fedora_messaging_severity": 20,
        "fedora_messaging_user_mmassari": true
      },
      "id": "6575adfb-93f4-4e4c-bb28-f8f66bea1e14",
      "priority": 0,
      "queue": null,
      "topic": "org.fedoraproject.prod.pagure.git.receive"
    },

datagrepper link

nforro commented 1 month ago

I don't think this has anything to do with multiple commits being pushed. That's not something that rarely happens, quite the opposite. The code in Pagure does nothing else but git diff --name-status $old_commit $end_commit, so I don't understand how this is possible.

nforro commented 1 month ago

I must be blind :man_facepalming: It finally hit me:

        ...
        changed_files = pagure.lib.git.get_changed_files(
            revs[-1],
            oldrev,
            repodir,
        )

        revs.reverse()
        print("* Publishing information for %i commits" % len(revs))

        topic = "git.receive"
        msg = dict(
            total_commits=len(revs),
            start_commit=revs[0],
            end_commit=revs[-1],
            old_commit=oldrev,
        ...

The list of pushed revs is :grey_exclamation:reversed:grey_exclamation: before sending the event, so revs[-1] passed to get_changed_files() becomes revs[0] and the command called is effectively git diff --name-status $old_commit $start_commit.

nforro commented 1 month ago

The fix is trivial, but I want to adjust the unit tests to cover this case, but unfortunately running the Pagure test suite eats all memory on my laptop and dies :sweat_smile: I have to find a way to work that around.

lachmanfrantisek commented 1 month ago

@nforro thanks a lot for fixing that!

We've just discussed with @majamassarini that we might want to do something until this gets to Pagure production (what can take some time) and agreed on the following:

nforro commented 1 month ago

Pagure PR is here (the tests might still need some work).

nforro commented 1 month ago

Marking as blocked as this issue is waiting on the Pagure PR to be merged and the fix deployed in production.