Fix several bugs with single-package updates

1. We should always use `~/.cache/nixpkgs`, since if we do it in PWD the hard
resets risk blowing away user work!

2. Previously, we weren't fetching and resetting to up-to-date master, which
   meant cmds would fail if your nixpkgs checkout was stale.

3. Previously, we were ignoring the `--pr` option entirely. This fixes that bug
by passing the bool into the UpdateEnv, and also cleans up the selected options
in the logger so it's easier to see.

4. We were including the title PR twice, like this:
   https://github.com/NixOS/nixpkgs/pull/86624

There are still some issues and things to improve, but with this PR I've managed
to get a working usage here:
https://github.com/NixOS/nixpkgs/pull/86625

app/Main.hs

@@ -17,7 +17,8 @@ import qualified Repology
 import System.IO (BufferMode (..), hSetBuffering, stderr, stdout)
 import qualified System.Posix.Env as P
 import Update (cveAll, cveReport, sourceGithubAll, updateAll, updatePackage)
-import Utils (Options (..), UpdateEnv (..), getGithubToken, setupNixpkgs)
+import Utils (Options (..), UpdateEnv (..), getGithubToken)
+import Git
 
 default (T.Text)
 
@@ -125,22 +126,22 @@ main = do
   case command of
     DeleteDone -> do
       token <- getGithubToken
-      setupNixpkgs token
+      Git.setupNixpkgs token
       P.setEnv "GITHUB_TOKEN" (T.unpack token) True
       deleteDone token
     UpdateList UpdateOptions {pr, cachix, cve, nixpkgsReview, outpaths} -> do
       token <- getGithubToken
       updates <- T.readFile "packages-to-update.txt"
-      setupNixpkgs token
+      Git.setupNixpkgs token
       P.setEnv "PAGER" "" True
       P.setEnv "GITHUB_TOKEN" (T.unpack token) True
       updateAll (Options pr True token cve cachix nixpkgsReview outpaths) updates
     Update UpdateOptions {pr, cve, cachix, nixpkgsReview} update -> do
       token <- getGithubToken
-      setupNixpkgs token
+      Git.setupNixpkgs token
       P.setEnv "PAGER" "" True
       P.setEnv "GITHUB_TOKEN" (T.unpack token) True
-      result <- updatePackage (Options False False token cve cachix nixpkgsReview 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."

src/Git.hs

@@ -1,17 +1,18 @@
 {-# LANGUAGE OverloadedStrings #-}
 
 module Git
-  ( cleanAndResetTo,
-    cleanup,
-    diff,
-    fetchIfStale,
-    fetch,
-    push,
+  ( checkAutoUpdateBranchDoesntExist,
     checkoutAtMergeBase,
-    checkAutoUpdateBranchDoesntExist,
+    cleanAndResetTo,
+    cleanup,
     commit,
-    headHash,
     deleteBranchesEverywhere,
+    diff,
+    fetch,
+    fetchIfStale,
+    headHash,
+    push,
+    setupNixpkgs,
   )
 where
 
@@ -20,12 +21,14 @@ import Control.Exception
 import qualified Data.ByteString as BS
 import qualified Data.ByteString.Lazy as BSL
 import qualified Data.Text as T
+import qualified System.Process.Typed
 import qualified Data.Text.Encoding as T
+import System.Posix.Env (setEnv)
 import qualified Data.Text.IO as T
 import Data.Time.Clock (addUTCTime, getCurrentTime)
 import qualified Data.Vector as V
 import OurPrelude hiding (throw)
-import System.Directory (getModificationTime)
+import System.Directory (doesDirectoryExist, getModificationTime, setCurrentDirectory)
 import System.Environment.XDG.BaseDir (getUserCacheDir)
 import System.Exit
 import Utils (Options (..), UpdateEnv (..), branchName, branchPrefix)
@@ -102,6 +105,26 @@ push updateEnv =
         )
     )
 
+-- Setup a NixPkgs clone in $XDG_CACHE_DIR/nixpkgs
+-- Since we are going to have to fetch, git reset, clean, and commit, we setup a
+-- cache dir to avoid destroying any uncommitted work the user may have in PWD.
+setupNixpkgs :: Text -> IO ()
+setupNixpkgs githubt = do
+  fp <- getUserCacheDir "nixpkgs"
+  exists <- doesDirectoryExist fp
+  unless exists $ do
+    proc "hub" ["clone", "nixpkgs", fp]
+      & 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_
+  setCurrentDirectory fp
+  _ <- runExceptT fetchIfStale
+  _ <- runExceptT $ cleanAndResetTo "master"
+  System.Posix.Env.setEnv "NIX_PATH" ("nixpkgs=" <> fp) True
+
 checkoutAtMergeBase :: MonadIO m => Text -> ExceptT Text m ()
 checkoutAtMergeBase bName = do
   base <-

src/Update.hs

@@ -83,11 +83,21 @@ getLog o = do
 
 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."
+  let repr f = if f o then "YES" else " NO"
+  let pr = repr doPR
+  let cachix = repr pushToCachix
+  let outpaths = repr calculateOutpaths
+  let cve = repr makeCVEReport
+  let review = repr runNixpkgsReview
+  log $ [interpolate|
+    Configured Nixpkgs-Update Options:
+    ----------------------------------
+    Send pull request on success:  $pr
+    Push to cachix:                $cachix
+    Calculate Outpaths:            $outpaths
+    CVE Security Report:           $cve
+    Run nixpkgs-review:            $review
+    ----------------------------------|]
 
 updateAll :: Options -> Text -> IO ()
 updateAll o updates = do
@@ -325,7 +335,9 @@ publishPackage log updateEnv oldSrcUrl newSrcUrl attrPath result opDiff rewriteM
             if (isNothing opDiff || numPackageRebuilds (fromJust opDiff) < 100)
               then "master"
               else "staging"
-      pullRequestUrl <- GH.pr updateEnv (prTitle updateEnv attrPath) prMsg ("r-ryantm:" <> (branchName updateEnv)) base
+      -- FIXME: #192 This needs to be detected dynamically. Use the hub token or GH library?
+      let ghUser = "r-ryantm"
+      pullRequestUrl <- GH.pr updateEnv (prTitle updateEnv attrPath) prMsg (ghUser <> ":" <> (branchName updateEnv)) base
       liftIO $ log pullRequestUrl
     else liftIO $ T.putStrLn prMsg
 
@@ -386,8 +398,6 @@ prMessage updateEnv isBroken metaDescription metaHomepage rewriteMsgs releaseUrl
       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
        $metaDescriptionLine
@@ -578,6 +588,9 @@ doCachix log updateEnv resultPath =
       lift $ log "skipping cachix"
       return "Build yourself:"
 
+-- FIXME: We should delete updatePackageBatch, and instead have the updateLoop
+-- just call updatePackage, so we aren't maintaining two parallel
+-- implementations!
 updatePackage ::
   Options ->
   Text ->
@@ -588,6 +601,12 @@ updatePackage o updateInfo = do
     let updateEnv = UpdateEnv p oldV newV url o
     let log = T.putStrLn
     liftIO $ notifyOptions log o
+    --
+    -- Update our git checkout and swap onto the update branch
+    Git.fetchIfStale <|> liftIO (T.putStrLn "Failed to fetch.")
+    Git.cleanAndResetTo "master"
+    Git.checkoutAtMergeBase (branchName updateEnv)
+    -- Gather some basic information
     Nix.assertNewerVersion updateEnv
     attrPath <- Nix.lookupAttrPath updateEnv
     Version.assertCompatibleWithPathPin updateEnv attrPath

src/Utils.hs

@@ -11,7 +11,6 @@ module Utils
     VersionMatcher (..),
     UpdateEnv (..),
     URL,
-    setupNixpkgs,
     tRead,
     parseUpdates,
     overwriteErrorT,
@@ -44,10 +43,9 @@ import Database.SQLite.Simple.Ok (Ok (..))
 import Database.SQLite.Simple.ToField (ToField, toField)
 import OurPrelude
 import Polysemy.Output
-import System.Directory (doesDirectoryExist, getCurrentDirectory, setCurrentDirectory)
-import System.Environment.XDG.BaseDir (getUserCacheDir)
+import System.Directory (doesDirectoryExist)
 import System.Posix.Directory (createDirectory)
-import System.Posix.Env (getEnv, setEnv)
+import System.Posix.Env (getEnv)
 import System.Posix.Files
   ( directoryMode,
     fileExist,
@@ -58,7 +56,6 @@ import System.Posix.Files
   )
 import System.Posix.Temp (mkdtemp)
 import System.Posix.Types (FileMode)
-import qualified System.Process.Typed
 import Text.Read (readEither)
 import Type.Reflection (Typeable)
 
@@ -189,29 +186,6 @@ logDir = do
     Right dir -> return dir
     Left e -> error $ T.unpack e
 
-setupNixpkgs :: Text -> IO ()
-setupNixpkgs githubt = do
-  currentDir <- getCurrentDirectory
-  if "nixpkgs" `isSuffixOf` currentDir
-    then do
-      T.putStrLn "Looks like current directory is a nixpkgs checkout, so using that."
-      System.Posix.Env.setEnv "NIX_PATH" ("nixpkgs=" <> currentDir) True
-    else do
-      T.putStrLn "Looks like current directory is not a nixpkgs checkout, so trying to use .cache/nixpkgs."
-      fp <- getUserCacheDir "nixpkgs"
-      exists <- doesDirectoryExist fp
-      unless exists $ do
-        proc "hub" ["clone", "nixpkgs", fp]
-          & 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
-      System.Posix.Env.setEnv "NIX_PATH" ("nixpkgs=" <> fp) True
-
 overwriteErrorT :: MonadIO m => Text -> ExceptT Text m a -> ExceptT Text m a
 overwriteErrorT t = fmapLT (const t)
 

test_data/expected_pr_description_1.md

@@ -1,5 +1,3 @@
-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.