From 81b7269112529d3a306da8b666d4bc5031d964d3 Mon Sep 17 00:00:00 2001 From: Ryan Mulligan Date: Mon, 29 Aug 2022 07:40:24 -0700 Subject: [PATCH] feature: check for existing updates with same src url fixes #315 --- src/GH.hs | 29 ++++++++++++++++++++--------- src/Update.hs | 22 +++++++++++++++------- 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/src/GH.hs b/src/GH.hs index 805fc10..f283d4c 100644 --- a/src/GH.hs +++ b/src/GH.hs @@ -201,23 +201,34 @@ authFromToken = GH.OAuth . T.encodeUtf8 authFrom :: UpdateEnv -> GH.Auth authFrom = authFromToken . U.githubToken . options -checkExistingUpdatePR :: MonadIO m => UpdateEnv -> Text -> ExceptT Text m () -checkExistingUpdatePR env attrPath = do +checkExistingUpdatePR :: MonadIO m => UpdateEnv -> Text -> Text -> ExceptT Text m () +checkExistingUpdatePR env attrPath srcUrl = do searchResult <- ExceptT $ liftIO $ GH.github (authFrom env) (GH.searchIssuesR search) & fmap (first (T.pack . show)) - if T.length (openPRReport searchResult) == 0 - then return () - else - throwE - ( "There might already be an open PR for this update:\n" - <> openPRReport searchResult - ) + when (T.length (openPRReport searchResult) /= 0) + (throwE + ( "There might already be an open PR for this update:\n" + <> openPRReport searchResult)) + + srcUrlSearchResult <- + ExceptT $ + liftIO $ + GH.github (authFrom env) (GH.searchIssuesR srcUrlSearch) + & fmap (first (T.pack . show)) + + when (srcUrl /= T.empty && T.length (openPRReport srcUrlSearchResult) /= 0) + (throwE + ( "There might already be an open PR for this update:\n" + <> openPRReport searchResult)) + + return () where title = U.prTitle env attrPath search = [interpolate|repo:nixos/nixpkgs $title |] + srcUrlSearch = [interpolate|new src url: $srcUrl |] openPRReport searchResult = GH.searchResultResults searchResult & V.filter (GH.issueClosedAt >>> isNothing) diff --git a/src/Update.hs b/src/Update.hs index aa06dbd..95c8aee 100644 --- a/src/Update.hs +++ b/src/Update.hs @@ -247,8 +247,9 @@ checkExistingUpdate :: UpdateEnv -> Maybe Text -> Text -> + Text -> ExceptT Text IO () -checkExistingUpdate log updateEnv existingCommitMsg attrPath = do +checkExistingUpdate log updateEnv existingCommitMsg srcUrl attrPath = do case existingCommitMsg of Nothing -> lift $ log "No auto update branch exists" Just msg -> do @@ -263,7 +264,7 @@ checkExistingUpdate log updateEnv existingCommitMsg attrPath = do -- Note that this check looks for PRs with the same old and new -- version numbers, so it won't stop us from updating an existing PR -- if this run updates the package to a newer version. - GH.checkExistingUpdatePR updateEnv attrPath + GH.checkExistingUpdatePR updateEnv attrPath srcUrl updateAttrPath :: (Text -> IO ()) -> @@ -277,6 +278,7 @@ updateAttrPath log mergeBase updateEnv@UpdateEnv {..} attrPath = do successOrFailure <- runExceptT $ do hasUpdateScript <- Nix.hasUpdateScript attrPath + oldSrcUrl <- Nix.getSrcUrl attrPath <|> pure "" existingCommitMsg <- fmap getAlt . execWriterT $ whenBatch updateEnv do @@ -286,7 +288,7 @@ updateAttrPath log mergeBase updateEnv@UpdateEnv {..} attrPath = do mbLastCommitMsg <- lift $ Git.findAutoUpdateBranchMessage packageName tell $ Alt mbLastCommitMsg unless hasUpdateScript do - lift $ checkExistingUpdate log updateEnv mbLastCommitMsg attrPath + lift $ checkExistingUpdate log updateEnv mbLastCommitMsg attrPath oldSrcUrl unless hasUpdateScript do Nix.assertNewerVersion updateEnv @@ -310,7 +312,6 @@ updateAttrPath log mergeBase updateEnv@UpdateEnv {..} attrPath = do -- Get the original values for diffing purposes derivationContents <- liftIO $ T.readFile $ T.unpack derivationFile oldHash <- Nix.getOldHash attrPath <|> pure "" - oldSrcUrl <- Nix.getSrcUrl attrPath <|> pure "" oldRev <- Nix.getAttr Nix.Raw "rev" attrPath <|> pure "" oldVerMay <- rightMay `fmapRT` (lift $ runExceptT $ Nix.getAttr Nix.Raw "version" attrPath) @@ -379,17 +380,18 @@ updateAttrPath log mergeBase updateEnv@UpdateEnv {..} attrPath = do assertNotUpdatedOn updateEnv' derivationFile "master" assertNotUpdatedOn updateEnv' derivationFile "staging" assertNotUpdatedOn updateEnv' derivationFile "staging-next" + whenBatch updateEnv do when pr do when hasUpdateScript do - checkExistingUpdate log updateEnv' existingCommitMsg attrPath + checkExistingUpdate log updateEnv' existingCommitMsg attrPath newSrcUrl Nix.build attrPath -- -- Publish the result lift . log $ "Successfully finished processing" - result <- Nix.resultLink + result <- Nix.resultLink let opReport = if isJust skipOutpathBase then "Outpath calculations were skipped for this package; total number of rebuilds unknown." @@ -455,6 +457,7 @@ publishPackage log updateEnv oldSrcUrl newSrcUrl attrPath result opReport prBase metaDescription metaHomepage metaChangelog + newSrcUrl rewriteMsgs releaseUrl compareUrl @@ -494,6 +497,7 @@ prMessage :: Text -> Text -> Text -> + Text -> [Text] -> Text -> Text -> @@ -507,7 +511,7 @@ prMessage :: Text -> Text -> Text -prMessage updateEnv isBroken metaDescription metaHomepage metaChangelog rewriteMsgs releaseUrl compareUrl resultCheckReport commitRev attrPath maintainers resultPath opReport cveRep cachixTestInstructions nixpkgsReviewMsg = +prMessage updateEnv isBroken metaDescription metaHomepage metaChangelog newSrcUrl rewriteMsgs releaseUrl compareUrl resultCheckReport commitRev 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 @@ -538,6 +542,8 @@ prMessage updateEnv isBroken metaDescription metaHomepage metaChangelog rewriteM if compareUrl == T.empty then "" else "- [Compare changes on GitHub](" <> compareUrl <> ")" + -- Needed for search of existing PRs + newSrcUrlLine = "- new src url: " <> newSrcUrl nixpkgsReviewSection = if nixpkgsReviewMsg == T.empty then "NixPkgs review skipped" @@ -571,6 +577,8 @@ prMessage updateEnv isBroken metaDescription metaHomepage metaChangelog rewriteM ###### To inspect upstream changes + $newSrcUrlLine + $releaseUrlMessage $compareUrlMessage