Merge pull request NixOS#186 from ryantm/nixpkgsreview

optionally run nixpkgs-review

README.md

@@ -107,6 +107,10 @@ nixpkgs-update supports interactive, single package updates via the
 * `--cve`—adds CVE vulnerability reporting to the PR message. On
   first invocation with this option, a CVE database is
   built. Subsequent invocations will be much faster.
+* `--nixpkgs-review`—runs
+  [nixpkgs-review](https://github.com/Mic92/nixpkgs-review), which
+  tries to build all the packages that depend on the one being updated
+  and adds a report.
 
 # Batch updates
 

app/Main.hs

@@ -26,6 +26,7 @@ data UpdateOptions
       { pr :: Bool,
         cve :: Bool,
         cachix :: Bool,
+        nixpkgsReview :: Bool,
         outpaths :: Bool
       }
 
@@ -46,6 +47,7 @@ updateOptionsParser =
     <$> O.flag False True (O.long "pr" <> O.help "Make a pull request using Hub.")
     <*> O.flag False True (O.long "cve" <> O.help "Make a CVE vulnerability report.")
     <*> O.flag False True (O.long "cachix" <> O.help "Push changes to Cachix")
+    <*> O.flag False True (O.long "nixpkgs-review" <> O.help "Runs nixpkgs-review on update commit rev")
     <*> O.flag False True (O.long "outpaths" <> O.help "Calculate outpaths to determine the branch to target")
 
 updateParser :: O.Parser Command
@@ -126,19 +128,19 @@ main = do
       setupNixpkgs token
       P.setEnv "GITHUB_TOKEN" (T.unpack token) True
       deleteDone token
-    UpdateList UpdateOptions {pr, cachix, cve, outpaths} -> do
+    UpdateList UpdateOptions {pr, cachix, cve, nixpkgsReview, outpaths} -> do
       token <- getGithubToken
       updates <- T.readFile "packages-to-update.txt"
       setupNixpkgs token
       P.setEnv "PAGER" "" True
       P.setEnv "GITHUB_TOKEN" (T.unpack token) True
-      updateAll (Options pr True token cve cachix outpaths) updates
-    Update UpdateOptions {pr, cve, cachix} update -> do
+      updateAll (Options pr True token cve cachix nixpkgsReview outpaths) updates
+    Update UpdateOptions {pr, cve, cachix, nixpkgsReview} update -> do
       token <- getGithubToken
       setupNixpkgs token
       P.setEnv "PAGER" "" True
       P.setEnv "GITHUB_TOKEN" (T.unpack token) True
-      result <- updatePackage (Options pr False token cve cachix False) update
+      result <- updatePackage (Options pr False token cve cachix nixpkgsReview False) update
       case result of
         Left e -> T.putStrLn e
         Right () -> T.putStrLn "Done."
@@ -156,12 +158,12 @@ main = do
       setupNixpkgs undefined
       report <-
         cveReport
-          (UpdateEnv productID oldVersion newVersion Nothing (Options False False undefined False False False))
+          (UpdateEnv productID oldVersion newVersion Nothing (Options False False undefined False False False False))
       T.putStrLn report
     SourceGithub -> do
       token <- getGithubToken
       updates <- T.readFile "packages-to-update.txt"
       setupNixpkgs token
       P.setEnv "GITHUB_TOKEN" (T.unpack token) True
-      sourceGithubAll (Options False False token False False False) updates
+      sourceGithubAll (Options False False token False False False False) updates
     FetchRepology -> Repology.fetch

default.nix

@@ -24,5 +24,7 @@ in pkg.overrideAttrs (attrs: {
     jq
     tree
     gist
+    (import sources.nixpkgs-review { inherit pkgs; })
+    cabal-install # just for develpoment
   ];
 })

nix/sources.json

@@ -34,5 +34,17 @@
         "type": "tarball",
         "url": "https://github.com/nixos/nixpkgs/archive/78bfdbb291fd20df0f0f65061ee3081610b0a48f.tar.gz",
         "url_template": "https://github.com/<owner>/<repo>/archive/<rev>.tar.gz"
+    },
+    "nixpkgs-review": {
+        "branch": "master",
+        "description": "Review pull-requests on https://github.com/NixOS/nixpkgs",
+        "homepage": "",
+        "owner": "mic92",
+        "repo": "nixpkgs-review",
+        "rev": "370e90a8d20640cc8924dacb4f55a86dadcec57f",
+        "sha256": "026lmwbvqdp7a3nkd08rd0nfyb9yiic36w6s7mh2rpp0ihp7qsd6",
+        "type": "tarball",
+        "url": "https://github.com/mic92/nixpkgs-review/archive/370e90a8d20640cc8924dacb4f55a86dadcec57f.tar.gz",
+        "url_template": "https://github.com/<owner>/<repo>/archive/<rev>.tar.gz"
     }
 }

nixpkgs-update.cabal

@@ -4,7 +4,7 @@ cabal-version: 2.2
 --
 -- see: https://github.com/sol/hpack
 --
--- hash: 09aa8671b0f8077c9c194e15f72bc2aec790275993b5122ab17c192622236836
+-- hash: 7492d0fc2af5df377764d358317e0892c0bda2961943218f8dcdeef179700cd9
 
 name:           nixpkgs-update
 version:        0.2.0
@@ -37,6 +37,7 @@ library
       GH
       Git
       Nix
+      NixpkgsReview
       NVD
       NVDRules
       OurPrelude

src/File.hs

@@ -53,6 +53,6 @@ replaceIO :: MonadIO m => Text -> Text -> FilePath -> m Bool
 replaceIO find replacement file =
   liftIO
     $ runFinal
-    $ embedToFinal @IO
+    $ embedToFinal
     $ runIO
     $ (replace find replacement file)

src/Git.hs

@@ -124,7 +124,7 @@ commit ref =
   runProcessNoIndexIssue_ (proc "git" ["commit", "-am", T.unpack ref])
 
 headHash :: MonadIO m => ExceptT Text m Text
-headHash = readProcessInterleavedNoIndexIssue_ "git rev-parse HEAD"
+headHash = T.strip <$> readProcessInterleavedNoIndexIssue_ "git rev-parse HEAD"
 
 deleteBranchesEverywhere :: Vector Text -> IO ()
 deleteBranchesEverywhere branches = do

src/Nix.hs

@@ -224,7 +224,7 @@ getHomepageET attrPath =
   ExceptT
     . liftIO
     . runFinal
-    . embedToFinal @IO
+    . embedToFinal
     . Error.runError
     . Process.runIO
     $ getHomepage attrPath

src/NixpkgsReview.hs

@@ -0,0 +1,48 @@
+{-# LANGUAGE OverloadedStrings #-}
+
+module NixpkgsReview
+  ( cacheDir,
+    runReport,
+  )
+where
+
+import Data.Text as T
+import qualified File as F
+import OurPrelude
+import qualified Process as P
+import System.Environment.XDG.BaseDir (getUserCacheDir)
+import Prelude hiding (log)
+
+cacheDir :: IO FilePath
+cacheDir = getUserCacheDir "nixpkgs-review"
+
+revDir :: FilePath -> Text -> FilePath
+revDir cache commit = cache <> "/rev-" <> T.unpack commit
+
+run ::
+  Members '[F.File, P.Process] r =>
+  FilePath ->
+  Text ->
+  Sem r Text
+run cache commit = do
+  -- TODO: probably just skip running nixpkgs-review if the directory
+  -- already exists
+  void $ ourReadProcessInterleavedSem $
+    proc "rm" ["-rf", revDir cache commit]
+  void $ ourReadProcessInterleavedSem $
+    proc "nixpkgs-review" ["rev", T.unpack commit, "--no-shell"]
+  F.read $ (revDir cache commit) <> "/report.md"
+
+-- Assumes we are already in nixpkgs dir
+runReport :: (Text -> IO ()) -> Text -> IO Text
+runReport log commit = do
+  log "[check][nixpkgs-review]"
+  c <- cacheDir
+  msg <-
+    runFinal
+      . embedToFinal
+      . F.runIO
+      . P.runIO
+      $ NixpkgsReview.run c commit
+  log msg
+  return msg

src/OurPrelude.hs

@@ -25,6 +25,7 @@ module OurPrelude
     ourReadProcessInterleavedBS_,
     ourReadProcessInterleaved,
     ourReadProcessInterleaved_Sem,
+    ourReadProcessInterleavedSem,
     silently,
     bytestringToText,
   )
@@ -81,11 +82,11 @@ ourReadProcessInterleaved_ =
   readProcessInterleaved_ >>> tryIOTextET >>> fmapRT bytestringToText
 
 ourReadProcessInterleaved_Sem ::
-  Members '[P.Process, Error Text] r =>
+  Members '[P.Process] r =>
   ProcessConfig stdin stdoutIgnored stderrIgnored ->
   Sem r Text
 ourReadProcessInterleaved_Sem =
-  P.readInterleaved >>> fmap bytestringToText
+  P.readInterleaved_ >>> fmap bytestringToText
 
 ourReadProcessInterleaved ::
   MonadIO m =>
@@ -96,5 +97,13 @@ ourReadProcessInterleaved =
     >>> tryIOTextET
     >>> fmapRT (\(a, b) -> (a, bytestringToText b))
 
+ourReadProcessInterleavedSem ::
+  Members '[P.Process] r =>
+  ProcessConfig stdin stdoutIgnored stderrIgnored ->
+  Sem r (ExitCode, Text)
+ourReadProcessInterleavedSem =
+  P.readInterleaved
+    >>> fmap (\(a, b) -> (a, bytestringToText b))
+
 silently :: ProcessConfig stdin stdout stderr -> ProcessConfig () () ()
 silently = setStderr closed >>> setStdin closed >>> setStdout closed

src/Process.hs

@@ -7,9 +7,11 @@ import qualified Data.ByteString.Lazy as BSL
 import Polysemy
 import Polysemy.Input
 import qualified System.Process.Typed as TP
+import System.Exit (ExitCode(..))
 
 data Process m a where
-  ReadInterleaved :: TP.ProcessConfig stdin stdout stderr -> Process m BSL.ByteString
+  ReadInterleaved_ :: TP.ProcessConfig stdin stdout stderr -> Process m BSL.ByteString
+  ReadInterleaved :: TP.ProcessConfig stdin stdout stderr -> Process m (ExitCode, BSL.ByteString)
 
 makeSem ''Process
 
@@ -19,7 +21,8 @@ runIO ::
   Sem r a
 runIO =
   interpret $ \case
-    ReadInterleaved config -> embed $ (TP.readProcessInterleaved_ config :: IO BSL.ByteString)
+    ReadInterleaved_ config -> embed $ (TP.readProcessInterleaved_ config)
+    ReadInterleaved config -> embed $ (TP.readProcessInterleaved config)
 
 runPure ::
   [BSL.ByteString] ->
@@ -28,4 +31,7 @@ runPure ::
 runPure outputList =
   runInputList outputList
     . reinterpret \case
-      ReadInterleaved _config -> maybe "" id <$> input
+      ReadInterleaved_ _config -> maybe "" id <$> input
+      ReadInterleaved _config -> do
+        r <- maybe "" id <$> input
+        return (ExitSuccess, r)

src/Rewrite.hs

@@ -101,7 +101,7 @@ quotedUrlsET log rwArgs =
   ExceptT
     $ liftIO
       . runFinal
-      . embedToFinal @IO
+      . embedToFinal
       . Error.runError
       . Process.runIO
       . File.runIO

src/Update.hs

@@ -31,6 +31,7 @@ import qualified GH
 import qualified Git
 import NVD (getCVEs, withVulnDB)
 import qualified Nix
+import qualified NixpkgsReview
 import OurPrelude
 import Outpaths
 import qualified Rewrite
@@ -79,13 +80,19 @@ getLog o = do
       return log
     else return T.putStrLn
 
+notifyOptions :: (Text -> IO ()) -> Options -> IO ()
+notifyOptions log o = do
+  when (doPR o) $ log "Will do push to origin and do PR on success."
+  when (pushToCachix o) $ log "Will push to cachix."
+  when (calculateOutpaths o) $ log "Will calculate outpaths."
+  when (makeCVEReport o) $ log "Will make a CVE security report."
+  when (runNixpkgsReview o) $ log "Will run nixpkgs-review."
+
 updateAll :: Options -> Text -> IO ()
 updateAll o updates = do
   log <- getLog o
   log "New run of nixpkgs-update"
-  when (doPR o) $ log "Will do push to origin and do PR on success."
-  when (pushToCachix o) $ log "Will push to cachix."
-  when (calculateOutpaths o) $ log "Will calculate outpaths."
+  notifyOptions log o
   twoHoursAgo <- runM $ Time.runIO Time.twoHoursAgo
   mergeBaseOutpathSet <-
     liftIO $ newIORef (MergeBaseOutpathsInfo twoHoursAgo S.empty)
@@ -256,16 +263,15 @@ updatePackageBatch log updateEnv mergeBaseOutpathsContext =
     Git.cleanAndResetTo "master"
 
 publishPackage ::
-  MonadIO m =>
-  (Text -> m ()) ->
+  (Text -> IO ()) ->
   UpdateEnv ->
   Text ->
   Text ->
   Text ->
   Text ->
   Maybe (Set ResultLine) ->
   [Text] ->
-  ExceptT Text m ()
+  ExceptT Text IO ()
 publishPackage log updateEnv oldSrcUrl newSrcUrl attrPath result opDiff msgs = do
   cachixTestInstructions <- doCachix log updateEnv result
   resultCheckReport <-
@@ -304,6 +310,10 @@ publishPackage log updateEnv oldSrcUrl newSrcUrl attrPath result opDiff msgs = d
   let commitMsg = commitMessage updateEnv attrPath
   Git.commit commitMsg
   commitHash <- Git.headHash
+  nixpkgsReviewMsg <-
+    if runNixpkgsReview . options $ updateEnv
+      then liftIO $ NixpkgsReview.runReport log commitHash
+      else return ""
   -- Try to push it three times
   when
     (doPR . options $ updateEnv)
@@ -329,6 +339,7 @@ publishPackage log updateEnv oldSrcUrl newSrcUrl attrPath result opDiff msgs = d
           (fromMaybe "" (outpathReport <$> opDiff))
           cveRep
           cachixTestInstructions
+          nixpkgsReviewMsg
   if (doPR . options $ updateEnv)
     then do
       let base =
@@ -362,8 +373,9 @@ prMessage ::
   Text ->
   Text ->
   Text ->
+  Text ->
   Text
-prMessage updateEnv isBroken metaDescription metaHomepage rewriteMessages releaseUrlMessage compareUrlMessage resultCheckReport commitHash attrPath maintainersCc resultPath opReport cveRep cachixTestInstructions =
+prMessage updateEnv isBroken metaDescription metaHomepage rewriteMessages releaseUrlMessage compareUrlMessage resultCheckReport commitHash attrPath maintainersCc resultPath opReport cveRep cachixTestInstructions nixpkgsReviewMsg =
   let brokenMsg = brokenWarning isBroken
       title = prTitle updateEnv attrPath
       sourceLinkInfo = maybe "" pattern $ sourceURL updateEnv
@@ -417,6 +429,7 @@ prMessage updateEnv isBroken metaDescription metaHomepage rewriteMessages releas
        </details>
        <br/>
        $cveRep
+       $nixpkgsReviewMsg
 
        $maintainersCc
     |]
@@ -540,6 +553,7 @@ updatePackage o updateInfo = do
     let (p, oldV, newV, url) = head (rights (parseUpdates updateInfo))
     let updateEnv = UpdateEnv p oldV newV url o
     let log = T.putStrLn
+    liftIO $ notifyOptions log o
     Nix.assertNewerVersion updateEnv
     attrPath <- Nix.lookupAttrPath updateEnv
     Version.assertCompatibleWithPathPin updateEnv attrPath

src/Utils.hs

@@ -109,6 +109,7 @@ data Options
         githubToken :: Text,
         makeCVEReport :: Bool,
         pushToCachix :: Bool,
+        runNixpkgsReview :: Bool,
         calculateOutpaths :: Bool
       }
   deriving (Show)
@@ -204,10 +205,10 @@ setupNixpkgs githubt = do
           & System.Process.Typed.setEnv -- requires that user has forked nixpkgs
           [("GITHUB_TOKEN" :: String, githubt & T.unpack)]
           & runProcess_
-      setCurrentDirectory fp
-      shell "git remote add upstream https://github.com/NixOS/nixpkgs"
-        & runProcess_
-      shell "git fetch upstream" & runProcess_
+        setCurrentDirectory fp
+        shell "git remote add upstream https://github.com/NixOS/nixpkgs"
+          & runProcess_
+        shell "git fetch upstream" & runProcess_
       setCurrentDirectory fp
       System.Posix.Env.setEnv "NIX_PATH" ("nixpkgs=" <> fp) True
 

test/RewriteSpec.hs

@@ -23,13 +23,13 @@ spec = do
     it "quotes an unquoted meta.homepage URL" do
       nixQuotedHomepageBad <- T.readFile "test_data/quoted_homepage_bad.nix"
       nixQuotedHomepageGood <- T.readFile "test_data/quoted_homepage_good.nix"
-      let options = Utils.Options False False "" False False False
+      let options = Utils.Options False False "" False False False False
       let updateEnv = Utils.UpdateEnv "inadyn" "2.5" "2.6" Nothing options
       -- TODO test correct file is being read
       let rwArgs = Rewrite.Args updateEnv "inadyn" undefined undefined
       (logs, (newContents, result)) <-
         ( runFinal
-            . embedToFinal @IO
+            . embedToFinal
             . Output.runOutputList
             . File.runPure [nixQuotedHomepageBad]
             . Process.runPure ["\"http://troglobit.com/project/inadyn/\""]