diff --git a/nixpkgs-update.cabal b/nixpkgs-update.cabal index cbc7551..40a9498 100644 --- a/nixpkgs-update.cabal +++ b/nixpkgs-update.cabal @@ -4,7 +4,7 @@ cabal-version: 2.2 -- -- see: https://github.com/sol/hpack -- --- hash: 7492d0fc2af5df377764d358317e0892c0bda2961943218f8dcdeef179700cd9 +-- hash: 0401b3234ff350b39169d38415559f3fe383a70cd04480c173c8219d5177c8ca name: nixpkgs-update version: 0.2.0 @@ -152,6 +152,7 @@ test-suite spec other-modules: DoctestSpec RewriteSpec + UpdateSpec Paths_nixpkgs_update hs-source-dirs: test diff --git a/src/Update.hs b/src/Update.hs index dd5e87e..b6ae6c1 100644 --- a/src/Update.hs +++ b/src/Update.hs @@ -6,12 +6,13 @@ {-# OPTIONS_GHC -fno-warn-type-defaults #-} module Update - ( updateAll, - updatePackage, - cveReport, + ( addPatched, cveAll, + cveReport, + prMessage, sourceGithubAll, - addPatched, + updateAll, + updatePackage, ) where @@ -236,7 +237,7 @@ updatePackageBatch log updateEnv mergeBaseOutpathsContext = -- that we actually should be touching this file. Get to work processing the -- various rewrite functions! let rwArgs = Rewrite.Args updateEnv attrPath derivationFile derivationContents - msgs <- Rewrite.runAll log rwArgs + rewriteMsgs <- Rewrite.runAll log rwArgs ---------------------------------------------------------------------------- -- -- Compute the diff and get updated values @@ -259,7 +260,7 @@ updatePackageBatch log updateEnv mergeBaseOutpathsContext = -- Publish the result lift . log $ "Successfully finished processing" result <- Nix.resultLink - publishPackage log updateEnv oldSrcUrl newSrcUrl attrPath result (Just opDiff) msgs + publishPackage log updateEnv oldSrcUrl newSrcUrl attrPath result (Just opDiff) rewriteMsgs Git.cleanAndResetTo "master" publishPackage :: @@ -272,41 +273,18 @@ publishPackage :: Maybe (Set ResultLine) -> [Text] -> ExceptT Text IO () -publishPackage log updateEnv oldSrcUrl newSrcUrl attrPath result opDiff msgs = do +publishPackage log updateEnv oldSrcUrl newSrcUrl attrPath result opDiff rewriteMsgs = do cachixTestInstructions <- doCachix log updateEnv result resultCheckReport <- case Blacklist.checkResult (packageName updateEnv) of Right () -> lift $ Check.result updateEnv (T.unpack result) Left msg -> pure msg - d <- Nix.getDescription attrPath <|> return T.empty - u <- Nix.getHomepageET attrPath <|> return T.empty + metaDescription <- Nix.getDescription attrPath <|> return T.empty + metaHomepage <- Nix.getHomepageET attrPath <|> return T.empty cveRep <- liftIO $ cveReport updateEnv - let metaDescription = - if d == T.empty - then "" - else "\n\nmeta.description for " <> attrPath <> " is: " <> d - let metaHomepage = - if u == T.empty - then "" - else "\nmeta.homepage for " <> attrPath <> " is: " <> u - let rewriteMessages = foldl (\ms m -> ms <> T.pack "\n- " <> m) "\n###### Updates performed" msgs - releaseUrlMessage <- - ( do - msg <- GH.releaseUrl updateEnv newSrcUrl - return ("\n- [Release on GitHub](" <> msg <> ")") - ) - <|> return "" - compareUrlMessage <- - ( do - msg <- GH.compareUrl oldSrcUrl newSrcUrl - return ("\n- [Compare changes on GitHub](" <> msg <> ")\n\n") - ) - <|> return "\n" + releaseUrl <- GH.releaseUrl updateEnv newSrcUrl <|> return "" + compareUrl <- GH.compareUrl oldSrcUrl newSrcUrl <|> return "" maintainers <- Nix.getMaintainers attrPath - let maintainersCc = - if not (T.null maintainers) - then "\n\ncc " <> maintainers <> " for testing." - else "" let commitMsg = commitMessage updateEnv attrPath Git.commit commitMsg commitHash <- Git.headHash @@ -328,13 +306,13 @@ publishPackage log updateEnv oldSrcUrl newSrcUrl attrPath result opDiff msgs = d isBroken metaDescription metaHomepage - rewriteMessages - releaseUrlMessage - compareUrlMessage + rewriteMsgs + releaseUrl + compareUrl resultCheckReport commitHash attrPath - maintainersCc + maintainers result (fromMaybe "" (outpathReport <$> opDiff)) cveRep @@ -364,7 +342,7 @@ prMessage :: Bool -> Text -> Text -> - Text -> + [Text] -> Text -> Text -> Text -> @@ -377,20 +355,44 @@ prMessage :: Text -> Text -> Text -prMessage updateEnv isBroken metaDescription metaHomepage rewriteMessages releaseUrlMessage compareUrlMessage resultCheckReport commitHash attrPath maintainersCc resultPath opReport cveRep cachixTestInstructions nixpkgsReviewMsg = +prMessage updateEnv isBroken metaDescription metaHomepage rewriteMsgs releaseUrl compareUrl resultCheckReport commitHash attrPath maintainers resultPath opReport cveRep cachixTestInstructions 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 + -- formatting in one place and unit test it. let brokenMsg = brokenWarning isBroken title = prTitle updateEnv attrPath - sourceLinkInfo = maybe "" pattern $ sourceURL updateEnv - where - pattern link = [interpolate|This update was made based on information from $link.|] + metaHomepageLine = + if metaHomepage == T.empty + then "" + else "\nmeta.homepage for " <> attrPath <> " is: " <> metaHomepage + metaDescriptionLine = + if metaDescription == T.empty + then "" + else "\n\nmeta.description for " <> attrPath <> " is: " <> metaDescription + rewriteMsgsLine = foldl (\ms m -> ms <> T.pack "\n- " <> m) "\n###### Updates performed" rewriteMsgs + maintainersCc = + if not (T.null maintainers) + then "\n\ncc " <> maintainers <> " for testing." + else "" + releaseUrlMessage = + if releaseUrl == T.empty + then "" + else "\n- [Release on GitHub](" <> releaseUrl <> ")" + compareUrlMessage = + if compareUrl == T.empty + then "" + else "\n- [Compare changes on GitHub](" <> compareUrl <> ")\n\n" + pat link = [interpolate|This update was made based on information from $link.|] + sourceLinkInfo = maybe "" pat $ sourceURL updateEnv in [interpolate| $title Semi-automatic update generated by [nixpkgs-update](https://github.com/ryantm/nixpkgs-update) tools. $sourceLinkInfo $brokenMsg - $metaDescription - $metaHomepage - $rewriteMessages + $metaDescriptionLine + $metaHomepageLine + $rewriteMsgsLine ###### To inspect upstream changes diff --git a/test/RewriteSpec.hs b/test/RewriteSpec.hs index b3d4a12..cea1c31 100644 --- a/test/RewriteSpec.hs +++ b/test/RewriteSpec.hs @@ -35,7 +35,7 @@ spec = do . Process.runPure ["\"http://troglobit.com/project/inadyn/\""] . Error.errorToIOFinal $ Rewrite.quotedUrls rwArgs - ) + ) T.putStrLn $ T.unlines logs head logs `shouldBe` "[quotedUrls]" result `shouldBe` Right (Just "Quoted meta.homepage for [RFC 45](https://github.com/NixOS/rfcs/pull/45)") diff --git a/test/UpdateSpec.hs b/test/UpdateSpec.hs new file mode 100644 index 0000000..76c2274 --- /dev/null +++ b/test/UpdateSpec.hs @@ -0,0 +1,36 @@ +{-# LANGUAGE OverloadedStrings #-} + +module UpdateSpec where + +import qualified Data.Text.IO as T +import Test.Hspec +import qualified Update +import qualified Utils + +main :: IO () +main = hspec spec + +spec :: Spec +spec = do + describe "PR message" do + it "matches a simple mock example" do + expected <- T.readFile "test_data/expected_pr_description_1.md" + let options = Utils.Options False False "" False False False False + let updateEnv = Utils.UpdateEnv "foobar" "1.0" "1.1" (Just "https://update-site.com") options + let isBroken = False + let metaDescription = "\"Foobar package description\"" + let metaHomepage = "\"https://foobar-homepage.com\"" + let rewriteMsgs = ["Version Update", "Other Update"] + let releaseUrl = "https://github.com/foobar/releases" + let compareUrl = "https://github.com/foobar/compare" + let resultCheckReport = "- Some other check" + let commitHash = "af39cf77a0d42a4f6771043ec54221ed" + let attrPath = "foobar" + let maintainersCc = "@maintainer1" + let resultPath = "/nix/store/some-hash-path" + let opReport = "123 total rebuild path(s)" + let cveRep = "" + let cachixTestInstructions = "" + let nixpkgsReviewMsg = "nixpkgs-review comment body" + let actual = Update.prMessage updateEnv isBroken metaDescription metaHomepage rewriteMsgs releaseUrl compareUrl resultCheckReport commitHash attrPath maintainersCc resultPath opReport cveRep cachixTestInstructions nixpkgsReviewMsg + actual `shouldBe` expected diff --git a/test_data/expected_pr_description_1.md b/test_data/expected_pr_description_1.md new file mode 100644 index 0000000..f2a51f5 --- /dev/null +++ b/test_data/expected_pr_description_1.md @@ -0,0 +1,89 @@ +foobar: 1.0 -> 1.1 + +Semi-automatic update generated by [nixpkgs-update](https://github.com/ryantm/nixpkgs-update) tools. This update was made based on information from https://update-site.com. + + + +meta.description for foobar is: "Foobar package description" + +meta.homepage for foobar is: "https://foobar-homepage.com" + +###### Updates performed +- Version Update +- Other Update + +###### To inspect upstream changes + + +- [Release on GitHub](https://github.com/foobar/releases) + +- [Compare changes on GitHub](https://github.com/foobar/compare) + + +###### Impact + +
+ +Checks done (click to expand) + + +--- + +- built on NixOS +- Some other check + +--- + +
+
+ +Rebuild report (if merged into master) (click to expand) + + +``` +123 total rebuild path(s) +``` + +
+ +
+ +Instructions to test this update (click to expand) + + +--- + + +``` +nix-build -A foobar https://github.com/r-ryantm/nixpkgs/archive/af39cf77a0d42a4f6771043ec54221ed.tar.gz +``` + +After you've downloaded or built it, look at the files and if there are any, run the binaries: +``` +ls -la /nix/store/some-hash-path +ls -la /nix/store/some-hash-path/bin +``` + +--- + +
+
+ + + +# Pre-merge build results + +We have automatically built all packages that will get rebuilt due to this change. + +This gives evidence on whether the upgrade will break dependent packages. +Note sometimes packages show up as _failed to build_ independent of the change, simply because they are already broken on the target branch. + +nixpkgs-review comment body + +--- + +###### Maintainer pings + + + +cc @maintainer1 for testing.