search

The-OpenROAD-Project / megaboom

BSD 3-Clause "New" or "Revised" License
16 stars 4 forks source link

Jenkins reports SUCCESS when the build actually failed #122

Open jeffng-or opened 1 month ago

jeffng-or commented 1 month ago

Found in https://github.com/The-OpenROAD-Project/megaboom/pull/120. The bazel build failed and reported errors, but Jenkins said that the job was a success.

jeffng-or commented 1 month ago

@maliberty @oharboe - I've made updates to the script that generates the build summary and checks for errors. One quirk is related to the current state of megaboom.

In the current run, there's a message in the build output:

[ERROR GRT-0119] Routing congestion too high. Check the congestion heatmap in the GUI and load bazel-out/k8-fastbuild/bin/reports/asap7/BoomTile/base/congestion.rpt in the DRC viewer.

But, the build continues successfully and now, this would be flagged and the build would be marked as a failure. If ORFS is reporting failure, shouldn't the ORFS run stop at that point?

For the short term, I propose to update the genReport.py script to ignore this error, so that CI doesn't report failures for this particular case.

maliberty commented 1 month ago

ORFS will stop on error though bazel-orfs is a bit different. I think grt "passes" and then drt fails at its start.

jeffng-or commented 1 month ago

ORFS will stop on error though bazel-orfs is a bit different. I think grt "passes" and then drt fails at its start.

In the current flow, drt isn't run. It stops with grt and the grt ODB is written out.

oharboe commented 1 month ago

ORFS will stop on error though bazel-orfs is a bit different. I think grt "passes" and then drt fails at its start.

Indeed.

This is by intention and ref our discussion: we want bazel to generate artifacts for a failed grt, which means it has to return exit code 0, but we don't want detailed route to proceed. check out detail_route.tcl for logic.

oharboe commented 1 month ago

ORFS will stop on error though bazel-orfs is a bit different. I think grt "passes" and then drt fails at its start.

In the current flow, drt isn't run. It stops with grt and the grt ODB is written out.

Yes, but examine the error message carefully: that error message is printed out by detail_route.tcl, no?

(If not, upgrade to latest bazel-orfs)

oharboe commented 1 month ago

and if that doesn't help, upgrade ORFS too...

jeffng-or commented 1 month ago

When running bazel build BoomTile_grt with the latest bazel-orfs and ORFS, I still see:

[ERROR GRT-0119] Routing congestion too high. Check the congestion heatmap in the GUI and load bazel-out/k8-fastbuild/bin/reports/asap7/BoomTile/base/congestion.rpt in the DRC viewer.

In my local workspaces:

@oharboe, based on our conversation yesterday, the short term issue has been resolved and this issue can be closed. We can address changes to the checking flow when we feel there is a need.

It still seems worth it to bump both the ORFS version in bazel-orfs and the version of bazel-orfs in megaboom. @oharboe, in the past, you folks had bumped the ORFS version in bazel-orfs. What is your process to validate the ORFS version prior to committing a bump?

oharboe commented 1 month ago

Now that we have a CI, we simply create a PR that will build w new bazel-orfs and ORFS.

jeffng-or commented 1 month ago

Now that we have a CI, we simply create a PR that will build w new bazel-orfs and ORFS.

I guess the question was more around updating the version of ORFS in bazel-orfs. I thought you mentioned that you had your own CI for that that you run before bumping the ORFS version in bazel-orfs.

oharboe commented 1 month ago

yes: bazel-orfs has CI, just create PRs to update it w e.g. ORFS

Ideally we would have automerge enabled for bazel-orfs.

Note that you can update ORFS directly in megaboom too, or accept the ORFA from bazel-orfs through a transitive dependency

jeffng-or commented 1 month ago

OK. Will send up a PR to update ORFS and then send up a PR for the bazel-orfs bump once that's merged.

oharboe commented 1 month ago

OK. Will send up a PR to update ORFS and then send up a PR for the bazel-orfs bump once that's merged.

If, what you want, is to update ORFS in megaboom, just uncomment update this in MODULE.bazel, but if you want to update both, it might be woth updating bazel-orfs, then megaboom as well. Several ways to skin the cat...

orfs = use_extension("@bazel-orfs//:extension.bzl", "orfs_repositories")

#

orfs.default(

image = "docker.io/openroad/orfs:v3.0-1459-gd505a82b",

sha256 = "434edf30b4d0610c17cb0cd4f865ba8ce34f7b247904bd2a8ba6e17b89981c37",

)

jeffng-or commented 1 month ago

For my own experience, I'll do the two step process:

  1. https://github.com/The-OpenROAD-Project/bazel-orfs/pull/177 - updates bazel-orfs to v3.0-1496-g20cf8faa
  2. Update megaboom to use updated bazel-orfs commit tag