#119 coreos-release-note: fix error produced when calculating pkgdiffs
Merged 4 months ago by dustymabe. Opened 4 months ago by abai.
fedora-web/ abai/websites fix-pkg-diff  into  master

@@ -190,10 +190,10 @@ 

              } else if (strcmp(pkgDiffAccCpy[i][2].NewPackage[1], pkgInfo.PreviousPackage[1]) > 0) {

                // overall, an upgrade

                pkgDiffAccCpyp[i] = d;

-               pkgDiffAccCpy[i].NewPackage[1] = dAcc[2].NewPackage[1];

+               pkgDiffAccCpy[i][2].NewPackage[1] = dAcc[2].NewPackage[1];

              } else {

                // overall, a downgrade

-               pkgDiffAccCpy[i].PreviousPackage[1] = pkgInfo.PreviousPackage[1];

+               pkgDiffAccCpy[i][2].PreviousPackage[1] = pkgInfo.PreviousPackage[1];

              }

              break;

            }
@@ -221,11 +221,11 @@ 

                pkgDiffAccCpy[i] = [];

              } else if (strcmp(pkgDiffAccCpy[i][2].NewPackage[1], pkgInfo.PreviousPackage[1]) > 0) {

                // overall, an upgrade

-               pkgDiffAccCpy[i].PreviousPackage[1] = pkgInfo.PreviousPackage[1];

+               pkgDiffAccCpy[i][2].PreviousPackage[1] = pkgInfo.PreviousPackage[1];

              } else {

                // overall, a downgrade

                pkgDiffAccCpyp[i] = d;

-               pkgDiffAccCpy[i].NewPackage[1] = dAcc[2].NewPackage[1];

+               pkgDiffAccCpy[i][2].NewPackage[1] = dAcc[2].NewPackage[1];

              }

              break;

            }

This error was not found previously since the conditional block was
not hit when calculating pkgdiffs. Essentially the accumulated pkgdiff
is an array of arrays, and the previous code lacks a layer of indexing
to modify the individual pkgdiff info.

In the future, more error checkings need to be added to handle this kind
of situations more gracefully.

Signed-off-by: Allen Bai abai@redhat.com

I don't fully understand this code, but I trust you. :)
So LGTM!

In the future we will drop this code to something much simpler.

assuming this works for all current streams (stable, testing, next) this LGTM.

@dustymabe I triple checked this locally to make sure no more surprises..
@jlebon yes, I was thinking this should be a temporary solution when I was writing this because it's too dependent on the structure of the metadata and a lot of corner cases to consider which makes it error prone.

EDIT: typo :)

Metadata