diff --git a/.github/workflows/eval.yml b/.github/workflows/eval.yml index bac9394500ac6..0582bb09da78d 100644 --- a/.github/workflows/eval.yml +++ b/.github/workflows/eval.yml @@ -132,6 +132,7 @@ jobs: uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: ref: ${{ needs.get-merge-commit.outputs.mergedSha }} + fetch-depth: 2 path: nixpkgs - name: Install Nix @@ -193,12 +194,18 @@ jobs: - name: Compare against the base branch if: steps.baseRunId.outputs.baseRunId run: | - nix-build nixpkgs/ci -A eval.compare \ + git -C nixpkgs worktree add ../base ${{ needs.attrs.outputs.baseSha }} + git -C nixpkgs diff --name-only ${{ needs.attrs.outputs.baseSha }} ${{ needs.attrs.outputs.mergedSha }} \ + | jq --raw-input . > touched-files.json + + # Use the base branch to get accurate maintainer info + nix-build base/ci -A eval.compare \ --arg beforeResultDir ./baseResult \ --arg afterResultDir ./prResult \ + --arg touchedFilesJson ./touched-files.json \ -o comparison + cat comparison/step-summary.md >> "$GITHUB_STEP_SUMMARY" - # TODO: Request reviews from maintainers for packages whose files are modified in the PR - name: Upload the combined results if: steps.baseRunId.outputs.baseRunId @@ -251,6 +258,13 @@ jobs: /repos/"$REPOSITORY"/issues/"$NUMBER"/labels \ -f "labels[]=$toAdd" done < <(comm -13 before after) + + # Request reviewers from maintainers of changed output paths + gh api \ + --method POST \ + /repos/${{ github.repository }}/pulls/${{ github.event.number }}/requested_reviewers \ + --input <(jq '{ reviewers: keys }' tmp/reviewers.json) + env: GH_TOKEN: ${{ github.token }} REPOSITORY: ${{ github.repository }} diff --git a/ci/eval/compare/default.nix b/ci/eval/compare/default.nix index 4f3b943a5a135..712a656d11add 100644 --- a/ci/eval/compare/default.nix +++ b/ci/eval/compare/default.nix @@ -5,7 +5,11 @@ writeText, ... }: -{ beforeResultDir, afterResultDir }: +{ + beforeResultDir, + afterResultDir, + touchedFilesJson, +}: let /* Derivation that computes which packages are affected (added, changed or removed) between two revisions of nixpkgs. @@ -77,11 +81,11 @@ let # - values: lists of `packagePlatformPath`s diffAttrs = diff beforeAttrs afterAttrs; + rebuilds = uniqueStrings (diffAttrs.added ++ diffAttrs.changed); + rebuildsPackagePlatformAttrs = convertToPackagePlatformAttrs rebuilds; + changed-paths = let - rebuilds = uniqueStrings (diffAttrs.added ++ diffAttrs.changed); - rebuildsPackagePlatformAttrs = convertToPackagePlatformAttrs rebuilds; - rebuildsByPlatform = groupByPlatform rebuildsPackagePlatformAttrs; rebuildsByKernel = groupByKernel rebuildsPackagePlatformAttrs; rebuildCountByKernel = lib.mapAttrs ( @@ -99,10 +103,17 @@ let labels = getLabels rebuildCountByKernel; } ); + + maintainers = import ./maintainers.nix { + changedattrs = lib.unique (map (a: a.packagePath) rebuildsPackagePlatformAttrs); + changedpathsjson = touchedFilesJson; + }; in runCommand "compare" { nativeBuildInputs = [ jq ]; + maintainers = builtins.toJSON maintainers; + passAsFile = [ "maintainers" ]; } '' mkdir $out @@ -110,5 +121,8 @@ runCommand "compare" cp ${changed-paths} $out/changed-paths.json jq -r -f ${./generate-step-summary.jq} < ${changed-paths} > $out/step-summary.md + + cp "$maintainersPath" "$out/maintainers.json" + # TODO: Compare eval stats '' diff --git a/ci/eval/compare/maintainers.nix b/ci/eval/compare/maintainers.nix new file mode 100644 index 0000000000000..0c08f85cec43b --- /dev/null +++ b/ci/eval/compare/maintainers.nix @@ -0,0 +1,123 @@ +# Almost directly vendored from https://github.com/NixOS/ofborg/blob/5a4e743f192fb151915fcbe8789922fa401ecf48/ofborg/src/maintainers.nix +{ changedattrs, changedpathsjson }: +let + pkgs = import ../../.. { + system = "x86_64-linux"; + config = { }; + overlays = [ ]; + }; + inherit (pkgs) lib; + + changedpaths = builtins.fromJSON (builtins.readFile changedpathsjson); + + anyMatchingFile = + filename: + let + matching = builtins.filter (changed: lib.strings.hasSuffix changed filename) changedpaths; + in + (builtins.length matching) > 0; + + anyMatchingFiles = files: (builtins.length (builtins.filter anyMatchingFile files)) > 0; + + enrichedAttrs = builtins.map (path: { + path = path; + name = builtins.concatStringsSep "." path; + }) changedattrs; + + validPackageAttributes = builtins.filter ( + pkg: + if (lib.attrsets.hasAttrByPath pkg.path pkgs) then + ( + if (builtins.tryEval (lib.attrsets.attrByPath pkg.path null pkgs)).success then + true + else + builtins.trace "Failed to access ${pkg.name} even though it exists" false + ) + else + builtins.trace "Failed to locate ${pkg.name}." false + ) enrichedAttrs; + + attrsWithPackages = builtins.map ( + pkg: pkg // { package = lib.attrsets.attrByPath pkg.path null pkgs; } + ) validPackageAttributes; + + attrsWithMaintainers = builtins.map ( + pkg: pkg // { maintainers = (pkg.package.meta or { }).maintainers or [ ]; } + ) attrsWithPackages; + + attrsWeCanPing = builtins.filter ( + pkg: + if (builtins.length pkg.maintainers) > 0 then + true + else + builtins.trace "Package has no maintainers: ${pkg.name}" false + ) attrsWithMaintainers; + + relevantFilenames = + drv: + (lib.lists.unique ( + builtins.map (pos: lib.strings.removePrefix (toString ../..) pos.file) ( + builtins.filter (x: x != null) [ + (builtins.unsafeGetAttrPos "maintainers" (drv.meta or { })) + (builtins.unsafeGetAttrPos "src" drv) + # broken because name is always set by stdenv: + # # A hack to make `nix-env -qa` and `nix search` ignore broken packages. + # # TODO(@oxij): remove this assert when something like NixOS/nix#1771 gets merged into nix. + # name = assert validity.handled; name + lib.optionalString + #(builtins.unsafeGetAttrPos "name" drv) + (builtins.unsafeGetAttrPos "pname" drv) + (builtins.unsafeGetAttrPos "version" drv) + + # Use ".meta.position" for cases when most of the package is + # defined in a "common" section and the only place where + # reference to the file with a derivation the "pos" + # attribute. + # + # ".meta.position" has the following form: + # "pkgs/tools/package-management/nix/default.nix:155" + # We transform it to the following: + # { file = "pkgs/tools/package-management/nix/default.nix"; } + { file = lib.head (lib.splitString ":" (drv.meta.position or "")); } + ] + ) + )); + + attrsWithFilenames = builtins.map ( + pkg: pkg // { filenames = relevantFilenames pkg.package; } + ) attrsWithMaintainers; + + attrsWithModifiedFiles = builtins.filter (pkg: anyMatchingFiles pkg.filenames) attrsWithFilenames; + + listToPing = lib.lists.flatten ( + builtins.map ( + pkg: + builtins.map (maintainer: { + handle = lib.toLower maintainer.github; + packageName = pkg.name; + dueToFiles = pkg.filenames; + }) pkg.maintainers + ) attrsWithModifiedFiles + ); + + byMaintainer = lib.lists.foldr ( + ping: collector: + collector + // { + "${ping.handle}" = [ + { inherit (ping) packageName dueToFiles; } + ] ++ (collector."${ping.handle}" or [ ]); + } + ) { } listToPing; + + textForPackages = + packages: lib.strings.concatStringsSep ", " (builtins.map (pkg: pkg.packageName) packages); + + textPerMaintainer = lib.attrsets.mapAttrs ( + maintainer: packages: "- @${maintainer} for ${textForPackages packages}" + ) byMaintainer; + + packagesPerMaintainer = lib.attrsets.mapAttrs ( + maintainer: packages: builtins.map (pkg: pkg.packageName) packages + ) byMaintainer; +in +packagesPerMaintainer diff --git a/ci/eval/compare/utils.nix b/ci/eval/compare/utils.nix index 82ba64e06a173..4bd6a45cfa7ac 100644 --- a/ci/eval/compare/utils.nix +++ b/ci/eval/compare/utils.nix @@ -11,6 +11,7 @@ rec { into { name = "hello"; + packagePath = [ "hello" ]; platform = "aarch64-linux"; } */ @@ -30,6 +31,9 @@ rec { null else { + # [ "python312Packages" "numpy" ] + inherit packagePath; + # python312Packages.numpy inherit name; @@ -52,12 +56,12 @@ rec { ] into [ - { name = "hello"; platform = "aarch64-linux"; } - { name = "hello"; platform = "x86_64-linux"; } - { name = "hello"; platform = "aarch64-darwin"; } - { name = "hello"; platform = "x86_64-darwin"; } - { name = "bye"; platform = "aarch64-darwin"; } - { name = "bye"; platform = "x86_64-darwin"; } + { name = "hello"; platform = "aarch64-linux"; packagePath = [ ... ]; } + { name = "hello"; platform = "x86_64-linux"; packagePath = [ ... ]; } + { name = "hello"; platform = "aarch64-darwin"; packagePath = [ ... ]; } + { name = "hello"; platform = "x86_64-darwin"; packagePath = [ ... ]; } + { name = "bye"; platform = "aarch64-darwin"; packagePath = [ ... ]; } + { name = "bye"; platform = "x86_64-darwin"; packagePath = [ ... ]; } ] */ convertToPackagePlatformAttrs =