Packit-as-a-service does not rerun production-build when issuing /packit build and /packit rebuild-failed

[pcahyna]

Some time ago, I rebased my branch in https://github.com/pcahyna/rear/pull/7 , force pushed, and then changed the base branch, because the original one did not make sense after the rebase. Packit-as-a-service started running on the new branch tip before I changed the base branch and the runs errored out with "Merge conflicts present". (This itself seems a bug, why is Packit merging anything? I thought it is running on the code in the PR/branch, not on the merged result (unlike e.g. Zuul) . https://packit.dev/docs/packit-service/#how-to-check-packit-service-works-in-your-project says "Packit Service builds the code from the pull request in COPR repository" which does not indicate a merge is performed.)

Anyway, I tried to rerun the build after changing the base branch using /packit build, but it triggered only the COPR builds, not production-builds (those that are not "production" in any way but are in fact scratch builds in Koji). Then I tried /packit rebuild-failed and it started rebuilding the COPR builds (not sure why, they were successful already) but again not the Koji builds.

I know that https://packit.dev/docs/packit-service/#how-to-re-trigger-packit-service-actions-in-your-pull-request says that /packit build is only a shorter version of /packit copr-build and therefore one can expect that it does not trigger other types of builds. I find it non-intuitive though. The manual does not seem to document how to trigger other types of builds, and as more and more types of builds will introduced (presumably), is it really the intent that /packit build will trigger only one of them, i.e. COPR builds, and users will have to trigger every build type separately? The name is also not intuitive, because build seems to imply "any build" and not just "copr build". It is even worse for /packit rebuild-failed, because the doc says "Or it is possible to re-trigger every failed task using a pull request comment /packit rebuild-failed", and "every failed task" implies "failed production-build tasks", but in reality this does not work.

I have found that https://dashboard.packit.dev/results/srpm-builds/73627 tells me "You can retrigger the build by adding a comment (/packit production-build) into this pull request." which is better than nothing, but I find it suboptimal. The fact that one needs a separate command for this type of build is really non-intuitive, and the instruction is hidden after plenty of cryptic output. Also, is there a /packit production-rebuild-failed command, if /packit rebuild-failed does not work for production builds?

[TomasTomecek]

Some time ago, I rebased my branch in pcahyna/rear#7 , force pushed, and then changed the base branch, because the original one did not make sense after the rebase. Packit-as-a-service started running on the new branch tip before I changed the base branch and the runs errored out with "Merge conflicts present". (This itself seems a bug, why is Packit merging anything? I thought it is running on the code in the PR/branch, not on the merged result (unlike e.g. Zuul) . https://packit.dev/docs/packit-service/#how-to-check-packit-service-works-in-your-project says "Packit Service builds the code from the pull request in COPR repository" which does not indicate a merge is performed.)

Actually Packit performs the merge, this behavior can be disabled with: https://packit.dev/docs/configuration/#merge_pr_in_ci

Thanks for the pointer, we should update our documentation.

Anyway, I tried to rerun the build after changing the base branch using /packit build, but it triggered only the COPR builds, not production-builds (those that are not "production" in any way but are in fact scratch builds in Koji). Then I tried /packit rebuild-failed and it started rebuilding the COPR builds (not sure why, they were successful already) but again not the Koji builds.

That's true, the /packit build command does not interact with "production builds" right now. I agree that we should:

1. rename it
2. make /packit build interact with it

I have found that https://dashboard.packit.dev/results/srpm-builds/73627 tells me "You can retrigger the build by adding a comment (/packit production-build) into this pull request." which is better than nothing, but I find it suboptimal. The fact that one needs a separate command for this type of build is really non-intuitive, and the instruction is hidden after plenty of cryptic output. Also, is there a /packit production-rebuild-failed command, if /packit rebuild-failed does not work for production builds?

👍 for your suggestion

rebuild-failed could trigger production-builds as well actually

[lachmanfrantisek]

@pcahyna Hello!

Thanks for reaching out to us. I remember we've discussed the production_build name in person and that we should change it. So here is the fresh issue to resolve that: packit/packit#1658 I hope we can start with that one and improve the rebuild commands after that.

[pcahyna]

hello @lachmanfrantisek, renaming production_build would certainly be nice, but I feel that it it is a mostly independent issue from this one. Even if you rename it, the issue that /packit build rebuilds only one kind of builds (COPR builds) and not Koji builds will remain, same for /packit rebuild-failed. So, I don't think it is necessary to start with packit/packit#1658 . WDYT?

[pcahyna]

@TomasTomecek

Actually Packit performs the merge

Ouch! So Packit builds and tests different code than the one in the pull request, because it includes unrelated changes from the target branch? That's quite unexpected, especially if the changes in the target branch introduce an unrelated bug. For some reason I never realized that, is it mentioned anywhere else than just in the description of one configuration option?

That said, maybe my expectations are wrong. What is the usual practice in other CI systems?

[lachmanfrantisek]

Yes, it's not strictly required but very related. It's at least nice-todo-before so we can use the new name in this design of the comment commands for retries... Or do this together.

Regarding this specific issue, I am slightly inclined for having a dedicated command for each job:

• /packit <job-name> for basic retrigger
• something like /packit rerun-failed-<job-name> / /packit rebuild-failed-<job-name> /... to retrigger only the failed ones of the single job.

[lachmanfrantisek]

That said, maybe my expectations are wrong. What is the usual practice in other CI systems?

e.g. zuul uses that as well by default

[pcahyna]

Regarding this specific issue, I am slightly inclined for having a dedicated command for each job:

• /packit <job-name> for basic retrigger

• something like /packit rerun-failed-<job-name> / /packit rebuild-failed-<job-name> /... to retrigger only the failed ones of the single job.

Even with this fine-grained control, the question is what to do with /packit rebuild-failed and /packit build. The names sound generic, so IMO they should retrigger everything, not just one job type.

[lachmanfrantisek]

Even with this fine-grained control, the question is what to do with /packit rebuild-failed and /packit build. The names sound generic, so IMO they should retrigger everything, not just one job type.

I don't think retrigger everything is the only and best answer. (I am not saying it can't be like that.)

We have a build alias for the copr-build job (there are people who don't know or don't want to know about Copr) -- with this perspective, it can be seen as a rebuild alias for rebuild-failed-copr-build (or how do we want to call it)...

We can also deprecate those in favour of the explicit naming for each job so it's clear. Also, we can add rebuild-all-failed or a similar explicit command to re-run all failed jobs.

Personally, I would probably define a new naming schema and deprecate the old ones so it's really clear what will happen.

[TomasTomecek]

@TomasTomecek

Actually Packit performs the merge

Ouch! So Packit builds and tests different code than the one in the pull request, because it includes unrelated changes from the target branch? That's quite unexpected, especially if the changes in the target branch introduce an unrelated bug. For some reason I never realized that, is it mentioned anywhere else than just in the description of one configuration option?

@lachmanfrantisek recently improved our onboarding & general documentation. I submitted a PR with providing more details (including the merge) regarding the PR Copr builds:

https://github.com/packit/packit.dev/pull/508

Feedback is welcome! :)

That said, maybe my expectations are wrong. What is the usual practice in other CI systems?

CI philosophy discussion, let's go! :)

To answer you first:

1. Zuul merges the change with main and I'm not sure if you can turn this behaviour off.
2. Travis CI does both: runs with and without merging the change.
3. Related Circle CI thread.

Now back to why we think it's a good practice to merge changes with main before testing them.

I'd love to quote you:

That's quite unexpected, especially if the changes in the target branch introduce an unrelated bug.

Is this a situation you are happy about? You merged code that introduced a bug in main. So your main branch is broken and should be fixed as soon as possible right? I am not blaming you: bugs happen all the time. But this is the reason we are continuously integrating changes & code as much as possible.

I'm assuming that you want to find out about that bug that broke main before merging more code, before doing a new release, or even before opening a new PR. Therefore I see the feature of merging PRs before testing them to be the practice to discover problems sooner. We submit PRs so they can be merged as soon as possible. Once they are merged they are part of the project's code base - they shouldn't degrade the project's quality. So running tests "as if it was merged" is a method to keep those quality standards. Can it upset contributors? Yes, it can. If you dislike it, you are welcome to turn it off, but you may be sacrificing some quality for convenience. (this has even recently happened to us: I merged my PR, all was green, but the change broke another repo - I went on to fix the breakage to unblock all other PRs)

I agree that being hampered by unrelated bugs is frustrating. Although merging with main lets you discover issues quickly. You can then resolve it sooner before it can cause more damage.

[stale[bot]]

This issue has been marked as stale because it hasn't seen any activity for the last 60 days.

Stale issues are closed after 14 days, unless the label is removed by a maintainer or someone comments on it.

This is done in order to ensure that open issues are still relevant.

Thank you for your contribution! :unicorn: :rocket: :robot:

(Note: issues labeled with pinned or EPIC are never marked as stale.)

[TomasTomecek]

TODO:

• [ ] propose a schema for our "rebuild" comments based on discussion in this issue
  • keep in mind that we have multiple ways of building and there will be even more in the future
• [ ] implement it 😁
• [ ] clearly document