diff --git a/nixpkgs-update.cabal b/nixpkgs-update.cabal index feadec84..0d99e637 100644 --- a/nixpkgs-update.cabal +++ b/nixpkgs-update.cabal @@ -175,6 +175,7 @@ test-suite spec other-modules: CheckSpec DoctestSpec + NixSpec UpdateSpec UtilsSpec hs-source-dirs: diff --git a/src/Nix.hs b/src/Nix.hs index 2c4be473..85b9c895 100644 --- a/src/Nix.hs +++ b/src/Nix.hs @@ -7,6 +7,8 @@ module Nix assertOldVersionOn, binPath, build, + buildForUpdate, + BuildOutcome (..), getAttr, getAttrString, getChangelog, @@ -20,6 +22,7 @@ module Nix getSrcUrl, hasPatchNamed, hasUpdateScript, + isUnsupportedHostPlatformFailure, lookupAttrPath, numberOfFetchers, numberOfHashes, @@ -36,9 +39,15 @@ import qualified Data.Text as T import qualified Git import Language.Haskell.TH.Env (envQ) import OurPrelude -import System.Exit () import qualified System.Process.Typed as TP -import Utils (UpdateEnv (..), nixBuildOptions, nixCommonOptions, srcOrMain) +import Utils + ( UpdateEnv (..), + nixBuildOptions, + nixBuildOptionsAllowUnsupported, + nixCommonOptions, + nixCommonOptionsAllowUnsupported, + srcOrMain, + ) import Prelude hiding (log) binPath :: String @@ -174,26 +183,58 @@ getSrcUrl = srcOrMain (nixEvalApplyRaw "p: builtins.elemAt p.drvAttrs.urls 0") -buildCmd :: Text -> ProcessConfig () () () -buildCmd attrPath = - silently $ proc (binPath <> "/nix-build") (nixBuildOptions ++ ["-A", attrPath & T.unpack]) +data BuildOutcome = BuiltNormally | BuiltWithAllowUnsupportedSystem + deriving (Eq, Show) + +buildCmdWithOptions :: [String] -> Text -> ProcessConfig () () () +buildCmdWithOptions buildOptions attrPath = + proc (binPath <> "/nix-build") (buildOptions ++ ["-A", attrPath & T.unpack]) -log :: Text -> ProcessConfig () () () -log attrPath = proc (binPath <> "/nix") (["--extra-experimental-features", "nix-command", "log", "-f", ".", attrPath & T.unpack] <> nixCommonOptions) +logWithOptions :: [String] -> Text -> ProcessConfig () () () +logWithOptions commonOptions attrPath = + proc (binPath <> "/nix") (["--extra-experimental-features", "nix-command", "log", "-f", ".", attrPath & T.unpack] <> commonOptions) build :: MonadIO m => Text -> ExceptT Text m () -build attrPath = - (buildCmd attrPath & runProcess_ & tryIOTextET) - <|> ( do - _ <- buildFailedLog - throwE "nix log failed trying to get build logs " - ) +build = buildWithOptions nixBuildOptions nixCommonOptions + +buildForUpdate :: MonadIO m => Text -> ExceptT Text m BuildOutcome +buildForUpdate attrPath = + catchE + (build attrPath >> return BuiltNormally) + ( \failure -> + if isUnsupportedHostPlatformFailure failure + then do + buildWithOptions + nixBuildOptionsAllowUnsupported + nixCommonOptionsAllowUnsupported + attrPath + return BuiltWithAllowUnsupportedSystem + else throwE failure + ) + +isUnsupportedHostPlatformFailure :: Text -> Bool +isUnsupportedHostPlatformFailure = + T.isInfixOf "not available on the requested hostPlatform" + +buildWithOptions :: MonadIO m => [String] -> [String] -> Text -> ExceptT Text m () +buildWithOptions buildOptions commonOptions attrPath = do + (exitCode, buildOutput) <- ourReadProcessInterleaved (buildCmdWithOptions buildOptions attrPath) + case exitCode of + ExitSuccess -> return () + ExitFailure _ -> buildFailed buildOutput where - buildFailedLog = do - buildLog <- - ourReadProcessInterleaved_ (log attrPath) - & fmap (T.lines >>> reverse >>> take 30 >>> reverse >>> T.unlines) - throwE ("nix build failed.\n" <> buildLog <> " ") + buildFailed buildOutput = do + buildLogResult <- lift $ runExceptT buildFailedLog + case buildLogResult of + Right buildLog -> throwE ("nix build failed.\n" <> buildLog <> " ") + Left _ -> throwE ("nix build failed.\n" <> lastBuildLines buildOutput <> " ") + + buildFailedLog = + ourReadProcessInterleaved_ (logWithOptions commonOptions attrPath) + & fmap lastBuildLines + + lastBuildLines = + T.lines >>> reverse >>> take 30 >>> reverse >>> T.unlines numberOfFetchers :: Text -> Int numberOfFetchers derivationContents = @@ -238,7 +279,7 @@ getHashFromBuild :: MonadIO m => Text -> ExceptT Text m Text getHashFromBuild = srcOrMain ( \attrPath -> do - (exitCode, _, stderr) <- buildCmd attrPath & readProcess + (exitCode, _, stderr) <- buildCmdWithOptions nixBuildOptions attrPath & readProcess when (exitCode == ExitSuccess) $ throwE "build succeeded unexpectedly" let stdErrText = bytestringToText stderr let firstSplit = T.splitOn "got: " stdErrText diff --git a/src/Update.hs b/src/Update.hs index 35c2b58c..8eba2233 100644 --- a/src/Update.hs +++ b/src/Update.hs @@ -334,7 +334,9 @@ updateAttrPath log mergeBase updateEnv@UpdateEnv {..} attrPath = do when (numPRebuilds == 0) (throwE "Update edits cause no rebuilds.") -- end outpaths section - Nix.build attrPath + buildOutcome <- Nix.buildForUpdate attrPath + let buildValidationNote = buildOutcomeNote buildOutcome + when (not $ T.null buildValidationNote) (lift . log $ buildValidationNote) -- -- Publish the result @@ -356,7 +358,7 @@ updateAttrPath log mergeBase updateEnv@UpdateEnv {..} attrPath = do then "staging-nixos" else "master" else "staging" - publishPackage log updateEnv' oldSrcUrl newSrcUrl attrPath result opReport prBase rewriteMsgs (isJust existingCommitMsg) + publishPackage log updateEnv' oldSrcUrl newSrcUrl attrPath result opReport prBase rewriteMsgs buildValidationNote (isJust existingCommitMsg) case successOrFailure of Left failure -> do @@ -374,9 +376,10 @@ publishPackage :: Text -> Text -> [Text] -> + Text -> Bool -> ExceptT Text IO () -publishPackage log updateEnv oldSrcUrl newSrcUrl attrPath result opReport prBase rewriteMsgs branchExists = do +publishPackage log updateEnv oldSrcUrl newSrcUrl attrPath result opReport prBase rewriteMsgs buildValidationNote branchExists = do cacheTestInstructions <- doCache log updateEnv result resultCheckReport <- case Skiplist.checkResult (packageName updateEnv) of @@ -418,6 +421,7 @@ publishPackage log updateEnv oldSrcUrl newSrcUrl attrPath result opReport prBase opReport cveRep cacheTestInstructions + buildValidationNote nixpkgsReviewMsg liftIO $ log prMsg if (doPR . options $ updateEnv) @@ -437,6 +441,11 @@ publishPackage log updateEnv oldSrcUrl newSrcUrl attrPath result opReport prBase commitMessage :: UpdateEnv -> Text -> Text commitMessage updateEnv attrPath = prTitle updateEnv attrPath +buildOutcomeNote :: Nix.BuildOutcome -> Text +buildOutcomeNote Nix.BuiltNormally = "" +buildOutcomeNote Nix.BuiltWithAllowUnsupportedSystem = + "Package metadata excludes the update worker host platform; validation retried with `allowUnsupportedSystem = true`." + -- See https://github.com/NixOS/nixpkgs-update/issues/514 -- -- You might think that "\@" would work, but GitHub's at-mentioning logic @@ -470,8 +479,9 @@ prMessage :: Text -> Text -> Text -> + Text -> Text -prMessage updateEnv metaDescription metaHomepage metaChangelog rewriteMsgs releaseUrl compareUrl resultCheckReport commitRev attrPath maintainers resultPath opReport cveRep cacheTestInstructions nixpkgsReviewMsg = +prMessage updateEnv metaDescription metaHomepage metaChangelog rewriteMsgs releaseUrl compareUrl resultCheckReport commitRev attrPath maintainers resultPath opReport cveRep cacheTestInstructions buildValidationNote nixpkgsReviewMsg = -- Some components of the PR description are pre-generated prior to calling -- because they require IO, but in general try to put as much as possible for -- the formatting into the pure function so that we can control the body @@ -520,6 +530,11 @@ prMessage updateEnv metaDescription metaHomepage metaChangelog rewriteMsgs relea ghUser = GH.untagName . githubUser . options $ updateEnv batch = batchUpdate . options $ updateEnv automatic = if batch then "Automatic" else "Semi-automatic" + buildValidationNoteLine = + if buildValidationNote == T.empty + then "" + else "- " <> buildValidationNote + checksDone = T.intercalate "\n" $ filter (not . T.null) ["- built on NixOS", buildValidationNoteLine, resultCheckReport] in [interpolate| $automatic update generated by [nixpkgs-update](https://github.com/NixOS/nixpkgs-update) tools. $sourceLinkInfo @@ -543,8 +558,7 @@ prMessage updateEnv metaDescription metaHomepage metaChangelog rewriteMsgs relea --- - - built on NixOS - $resultCheckReport + $checksDone --- diff --git a/src/Utils.hs b/src/Utils.hs index dc464928..1acfa9f4 100644 --- a/src/Utils.hs +++ b/src/Utils.hs @@ -16,7 +16,9 @@ module Utils getGithubToken, getGithubUser, logDir, + nixBuildOptionsAllowUnsupported, nixBuildOptions, + nixCommonOptionsAllowUnsupported, nixCommonOptions, parseUpdates, prTitle, @@ -246,9 +248,17 @@ srcOrMain et attrPath = et (attrPath <> ".src") <|> et (attrPath <> ".originalSr nixCommonOptions :: [String] nixCommonOptions = + nixCommonOptionsWithConfig "{ allowUnfree = true; allowAliases = false; }" + +nixCommonOptionsAllowUnsupported :: [String] +nixCommonOptionsAllowUnsupported = + nixCommonOptionsWithConfig "{ allowUnfree = true; allowAliases = false; allowUnsupportedSystem = true; }" + +nixCommonOptionsWithConfig :: String -> [String] +nixCommonOptionsWithConfig config = [ "--arg", "config", - "{ allowUnfree = true; allowAliases = false; }", + config, "--arg", "overlays", "[ ]" @@ -262,6 +272,14 @@ nixBuildOptions = ] <> nixCommonOptions +nixBuildOptionsAllowUnsupported :: [String] +nixBuildOptionsAllowUnsupported = + [ "--option", + "sandbox", + "true" + ] + <> nixCommonOptionsAllowUnsupported + runLog :: Member (Embed IO) r => (Text -> IO ()) -> diff --git a/test/NixSpec.hs b/test/NixSpec.hs new file mode 100644 index 00000000..949cf096 --- /dev/null +++ b/test/NixSpec.hs @@ -0,0 +1,59 @@ +{-# LANGUAGE OverloadedStrings #-} + +module NixSpec where + +import qualified Data.List as List +import qualified Data.Text as T +import qualified Nix +import Test.Hspec +import qualified Utils + +main :: IO () +main = hspec spec + +spec :: Spec +spec = do + describe "unsupported hostPlatform failures" do + it "detects Nix's metadata platform rejection" do + let failure = + T.unlines + [ "error:", + " ... while evaluating the attribute 'drvPath'", + "", + " error: Package 'tart-2.29.0' in /nix/store/source/pkgs/by-name/ta/tart/package.nix:123 is not available on the requested hostPlatform:", + " hostPlatform.config = \"x86_64-unknown-linux-gnu\";", + " package.meta.platforms = [ \"aarch64-darwin\" \"x86_64-darwin\" ];" + ] + + Nix.isUnsupportedHostPlatformFailure failure `shouldBe` True + + it "does not match ordinary build failures" do + let failure = + T.unlines + [ "nix build failed.", + "error: builder for '/nix/store/example.drv' failed with exit code 2", + "make: *** [Makefile:10: all] Error 2" + ] + + Nix.isUnsupportedHostPlatformFailure failure `shouldBe` False + + describe "allowUnsupportedSystem build options" do + it "preserves common policy while allowing unsupported systems" do + let options = Utils.nixBuildOptionsAllowUnsupported + let config = lookupArg "config" options + + options `shouldSatisfy` containsSequence ["--option", "sandbox", "true"] + config `shouldSatisfy` maybe False (List.isInfixOf "allowUnfree = true") + config `shouldSatisfy` maybe False (List.isInfixOf "allowAliases = false") + config `shouldSatisfy` maybe False (List.isInfixOf "allowUnsupportedSystem = true") + +containsSequence :: Eq a => [a] -> [a] -> Bool +containsSequence needle haystack = + any (needle `List.isPrefixOf`) (List.tails haystack) + +lookupArg :: String -> [String] -> Maybe String +lookupArg name ("--arg" : key : value : rest) + | key == name = Just value + | otherwise = lookupArg name (key : value : rest) +lookupArg name (_ : rest) = lookupArg name rest +lookupArg _ [] = Nothing diff --git a/test/UpdateSpec.hs b/test/UpdateSpec.hs index 6d045f7c..5b7d2dc0 100644 --- a/test/UpdateSpec.hs +++ b/test/UpdateSpec.hs @@ -2,7 +2,8 @@ module UpdateSpec where -import qualified Data.Text.IO as T +import qualified Data.Text as T +import qualified Data.Text.IO as TIO import Test.Hspec import qualified Update import qualified Utils @@ -34,24 +35,30 @@ spec = do let opReport = "123 total rebuild path(s)" let cveRep = "" let cacheTestInstructions = "" + let buildValidationNote = "" let nixpkgsReviewMsg = "nixpkgs-review comment body" + let unsupportedPlatformBuildNote = "Package metadata excludes the update worker host platform; validation retried with `allowUnsupportedSystem = true`." it "matches a simple mock example" do - expected <- T.readFile "test_data/expected_pr_description_1.md" - let actual = Update.prMessage updateEnv metaDescription metaHomepage metaChangelog rewriteMsgs releaseUrl compareUrl resultCheckReport commitHash attrPath maintainersCc resultPath opReport cveRep cacheTestInstructions nixpkgsReviewMsg - T.writeFile "test_data/actual_pr_description_1.md" actual + expected <- TIO.readFile "test_data/expected_pr_description_1.md" + let actual = Update.prMessage updateEnv metaDescription metaHomepage metaChangelog rewriteMsgs releaseUrl compareUrl resultCheckReport commitHash attrPath maintainersCc resultPath opReport cveRep cacheTestInstructions buildValidationNote nixpkgsReviewMsg + TIO.writeFile "test_data/actual_pr_description_1.md" actual actual `shouldBe` expected it "does not include Nixpkgs review section when no review was done" do - expected <- T.readFile "test_data/expected_pr_description_2.md" + expected <- TIO.readFile "test_data/expected_pr_description_2.md" let nixpkgsReviewMsg' = "" - let actual = Update.prMessage updateEnv metaDescription metaHomepage metaChangelog rewriteMsgs releaseUrl compareUrl resultCheckReport commitHash attrPath maintainersCc resultPath opReport cveRep cacheTestInstructions nixpkgsReviewMsg' - T.writeFile "test_data/actual_pr_description_2.md" actual + let actual = Update.prMessage updateEnv metaDescription metaHomepage metaChangelog rewriteMsgs releaseUrl compareUrl resultCheckReport commitHash attrPath maintainersCc resultPath opReport cveRep cacheTestInstructions buildValidationNote nixpkgsReviewMsg' + TIO.writeFile "test_data/actual_pr_description_2.md" actual actual `shouldBe` expected it "escapes at-mentions" do - expected <- T.readFile "test_data/expected_pr_description_3.md" + expected <- TIO.readFile "test_data/expected_pr_description_3.md" let metaDescription' = "\"Package by @foo and @bar\"" - let actual = Update.prMessage updateEnv metaDescription' metaHomepage metaChangelog rewriteMsgs releaseUrl compareUrl resultCheckReport commitHash attrPath maintainersCc resultPath opReport cveRep cacheTestInstructions nixpkgsReviewMsg - T.writeFile "test_data/actual_pr_description_3.md" actual + let actual = Update.prMessage updateEnv metaDescription' metaHomepage metaChangelog rewriteMsgs releaseUrl compareUrl resultCheckReport commitHash attrPath maintainersCc resultPath opReport cveRep cacheTestInstructions buildValidationNote nixpkgsReviewMsg + TIO.writeFile "test_data/actual_pr_description_3.md" actual actual `shouldBe` expected + + it "includes an unsupported-platform validation note when fallback build was used" do + let actual = Update.prMessage updateEnv metaDescription metaHomepage metaChangelog rewriteMsgs releaseUrl compareUrl resultCheckReport commitHash attrPath maintainersCc resultPath opReport cveRep cacheTestInstructions unsupportedPlatformBuildNote nixpkgsReviewMsg + unsupportedPlatformBuildNote `T.isInfixOf` actual `shouldBe` True