From 0feb0e70514f4318598000296ea6204316953d32 Mon Sep 17 00:00:00 2001 From: Benjamin Hipple Date: Sun, 3 May 2020 00:13:46 -0400 Subject: [PATCH] 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 | 11 +++---- src/Git.hs | 41 ++++++++++++++++++++------ src/Update.hs | 35 +++++++++++++++++----- src/Utils.hs | 30 ++----------------- test_data/expected_pr_description_1.md | 2 -- 5 files changed, 67 insertions(+), 52 deletions(-) diff --git a/app/Main.hs b/app/Main.hs index 05ee7e1..6aa1f73 100644 --- a/app/Main.hs +++ b/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." diff --git a/src/Git.hs b/src/Git.hs index 2f6857d..2809192 100644 --- a/src/Git.hs +++ b/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 <- diff --git a/src/Update.hs b/src/Update.hs index b6ae6c1..c5a6707 100644 --- a/src/Update.hs +++ b/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 diff --git a/src/Utils.hs b/src/Utils.hs index fdca1d7..ad6f73a 100644 --- a/src/Utils.hs +++ b/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) diff --git a/test_data/expected_pr_description_1.md b/test_data/expected_pr_description_1.md index f2a51f5..538fc8b 100644 --- a/test_data/expected_pr_description_1.md +++ b/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.