From ed2f4dd97b4c3bf62c25c7fc268227cb7d48d4e1 Mon Sep 17 00:00:00 2001 From: patrick brisbin Date: Thu, 10 Oct 2024 07:56:53 -0400 Subject: [PATCH 1/7] Only commit paths we restyled This avoids unintentionally including other files we may have created, such as downloaded files or backup files produced by a restyler. --- src/Restyler/Monad/Git.hs | 8 ++++---- src/Restyler/Restyler/Run.hs | 2 +- src/Restyler/RestylerResult.hs | 7 ++++--- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/Restyler/Monad/Git.hs b/src/Restyler/Monad/Git.hs index 54b43a7a..8713bb9e 100644 --- a/src/Restyler/Monad/Git.hs +++ b/src/Restyler/Monad/Git.hs @@ -25,7 +25,7 @@ import System.Process.Typed class Monad m => MonadGit m where isGitRepository :: HasCallStack => m Bool gitDiffNameOnly :: HasCallStack => Maybe String -> m [FilePath] - gitCommitAll :: HasCallStack => String -> m String + gitCommit :: HasCallStack => String -> NonEmpty FilePath -> m String -- | An instance that invokes the real @git@ newtype ActualGit m a = ActualGit @@ -47,8 +47,8 @@ instance where isGitRepository = (== ExitSuccess) <$> runGitExitCode ["rev-parse"] gitDiffNameOnly mRef = readGitLines $ ["diff", "--name-only"] <> maybeToList mRef - gitCommitAll msg = do - runGit_ ["commit", "-a", "--message", msg] + gitCommit msg paths = do + runGit_ $ ["commit", "--message", msg, "--"] <> toList paths readGitChomp ["rev-parse", "HEAD"] runGit_ @@ -121,4 +121,4 @@ newtype NullGit m a = NullGit instance Monad m => MonadGit (NullGit m) where isGitRepository = pure False gitDiffNameOnly _ = pure [] - gitCommitAll _ = pure "" + gitCommit _ _ = pure "" diff --git a/src/Restyler/Restyler/Run.hs b/src/Restyler/Restyler/Run.hs index d614ab64..b5c13dc3 100644 --- a/src/Restyler/Restyler/Run.hs +++ b/src/Restyler/Restyler/Run.hs @@ -233,7 +233,7 @@ runRestyler config r = \case isGit <- isGitRepository if isGit - then getRestylerResult config r + then getRestylerResult config paths r else do Nothing <$ logWarn "Unable to determine Restyler result (not a git repository)" diff --git a/src/Restyler/RestylerResult.hs b/src/Restyler/RestylerResult.hs index d752bd8a..8c9a892c 100644 --- a/src/Restyler/RestylerResult.hs +++ b/src/Restyler/RestylerResult.hs @@ -33,17 +33,18 @@ data RestylerResult = RestylerResult getRestylerResult :: (MonadGit m, MonadReader env m, HasNoCommitOption env) => Config + -> [FilePath] -> Restyler -> m (Maybe RestylerResult) -getRestylerResult config restyler = do +getRestylerResult config paths restyler = do noCommit <- getNoCommit - mRestyled <- nonEmpty <$> gitDiffNameOnly Nothing + mRestyled <- nonEmpty . filter (`elem` paths) <$> gitDiffNameOnly Nothing for mRestyled $ \restyled -> do sha <- if noCommit then pure Nothing - else Just <$> gitCommitAll commitMessage + else Just <$> gitCommit commitMessage restyled pure $ RestylerResult From f25f0364ec5f25ed84aaf7b8138d23b2ed6e5d49 Mon Sep 17 00:00:00 2001 From: patrick brisbin Date: Thu, 10 Oct 2024 08:06:02 -0400 Subject: [PATCH 2/7] Run git-clean at the end, by default This ensures any files created by the restyling process are cleaned up, which is important to prevent any create-pull-request action running after us from including them in its own commit. Behavior can be disabled by `NO_CLEAN` / `--no-clean`. --- README.md | 7 ++++-- restyler.cabal | 1 + src/Restyler/Local.hs | 7 +++++- src/Restyler/Local/App.hs | 2 ++ src/Restyler/Local/Options.hs | 10 ++++++++ src/Restyler/Monad/Git.hs | 3 +++ src/Restyler/Options/NoClean.hs | 42 +++++++++++++++++++++++++++++++++ test/SpecHelper.hs | 2 ++ 8 files changed, 71 insertions(+), 3 deletions(-) create mode 100644 src/Restyler/Options/NoClean.hs diff --git a/README.md b/README.md index a3d44b50..1f760202 100644 --- a/README.md +++ b/README.md @@ -14,8 +14,8 @@ curl --proto '=https' --tlsv1.2 -sSf \ ```console Usage: restyle [--debug] [--trace] [--color WHEN] [--dry-run] [--fail-on-differences] [--host-directory DIRECTORY] - [--image-cleanup] [--manifest FILE] [--no-commit] [--no-pull] - PATH [PATH] + [--image-cleanup] [--manifest FILE] [--no-commit] [--no-clean] + [--no-pull] PATH [PATH] Restyle local files @@ -30,6 +30,7 @@ Available options: --image-cleanup Remove pulled restyler images after restyling --manifest FILE Restylers manifest to use --no-commit Don't make commits for restyle changes + --no-clean Don't run git-clean after restyling --no-pull Don't docker-pull images before docker-run -h,--help Show this help text @@ -50,6 +51,8 @@ Available environment variables: LOG_FORMAT LOG_LEVEL MANIFEST Restylers manifest to use + NO_CLEAN Don't run git-clean after + restyling NO_COLOR NO_COMMIT Don't make commits for restyle changes diff --git a/restyler.cabal b/restyler.cabal index 27db4c3e..1fd59291 100644 --- a/restyler.cabal +++ b/restyler.cabal @@ -47,6 +47,7 @@ library Restyler.Options.ImageCleanup Restyler.Options.LogSettings Restyler.Options.Manifest + Restyler.Options.NoClean Restyler.Options.NoCommit Restyler.Options.NoPull Restyler.Options.Repository diff --git a/src/Restyler/Local.hs b/src/Restyler/Local.hs index 6f06713a..fca7bc6c 100644 --- a/src/Restyler/Local.hs +++ b/src/Restyler/Local.hs @@ -19,13 +19,14 @@ import Restyler.Ignore import Restyler.Monad.Directory import Restyler.Monad.Docker (MonadDocker) import Restyler.Monad.DownloadFile -import Restyler.Monad.Git (MonadGit) +import Restyler.Monad.Git (MonadGit (..)) import Restyler.Monad.ReadFile import Restyler.Monad.WriteFile import Restyler.Options.DryRun import Restyler.Options.HostDirectory import Restyler.Options.ImageCleanup import Restyler.Options.Manifest +import Restyler.Options.NoClean import Restyler.Options.NoCommit import Restyler.Options.NoPull import Restyler.Restrictions @@ -49,6 +50,7 @@ run , HasImageCleanupOption env , HasManifestOption env , HasNoCommitOption env + , HasNoCleanOption env , HasNoPullOption env , HasRestrictions env , HasPullRequestState pr @@ -90,4 +92,7 @@ run pr paths = do , "sha" .= result.sha ] + noClean <- getNoClean + unless noClean gitClean + pure $ maybe RestyleNoDifference (const RestyleDifference) mresults diff --git a/src/Restyler/Local/App.hs b/src/Restyler/Local/App.hs index 8ecba112..5227e5e4 100644 --- a/src/Restyler/Local/App.hs +++ b/src/Restyler/Local/App.hs @@ -23,6 +23,7 @@ import Restyler.Options.HostDirectory import Restyler.Options.ImageCleanup import Restyler.Options.LogSettings import Restyler.Options.Manifest +import Restyler.Options.NoClean import Restyler.Options.NoCommit import Restyler.Options.NoPull import Restyler.Restrictions @@ -39,6 +40,7 @@ data App = App deriving (HasImageCleanupOption) via (ThroughOptions App) deriving (HasManifestOption) via (ThroughOptions App) deriving (HasNoCommitOption) via (ThroughOptions App) + deriving (HasNoCleanOption) via (ThroughOptions App) deriving (HasNoPullOption) via (ThroughOptions App) deriving (HasRestrictions) via (ThroughOptions App) diff --git a/src/Restyler/Local/Options.hs b/src/Restyler/Local/Options.hs index bc92068d..d92025c1 100644 --- a/src/Restyler/Local/Options.hs +++ b/src/Restyler/Local/Options.hs @@ -27,6 +27,7 @@ import Restyler.Options.HostDirectory import Restyler.Options.ImageCleanup import Restyler.Options.LogSettings import Restyler.Options.Manifest +import Restyler.Options.NoClean import Restyler.Options.NoCommit import Restyler.Options.NoPull import Restyler.Restrictions @@ -39,6 +40,7 @@ data Options = Options , imageCleanup :: ImageCleanupOption , manifest :: ManifestOption , noCommit :: NoCommitOption + , noClean :: NoCleanOption , noPull :: NoPullOption , restrictions :: Restrictions } @@ -63,6 +65,9 @@ instance HasManifestOption Options where instance HasNoCommitOption Options where getNoCommitOption = (.noCommit) +instance HasNoCleanOption Options where + getNoCleanOption = (.noClean) + instance HasNoPullOption Options where getNoPullOption = (.noPull) @@ -79,6 +84,7 @@ envParser = <*> envImageCleanupOption <*> envManifestOption <*> envNoCommitOption + <*> envNoCleanOption <*> envNoPullOption <*> envRestrictions @@ -92,6 +98,7 @@ optParser = <*> optImageCleanupOption <*> optManifestOption <*> optNoCommitOption + <*> optNoCleanOption <*> optNoPullOption <*> pure mempty -- Restrictions are ENV-only @@ -124,6 +131,9 @@ instance HasOptions a => HasManifestOption (ThroughOptions a) where instance HasOptions a => HasNoCommitOption (ThroughOptions a) where getNoCommitOption = getNoCommitOption . getOptions +instance HasOptions a => HasNoCleanOption (ThroughOptions a) where + getNoCleanOption = getNoCleanOption . getOptions + instance HasOptions a => HasNoPullOption (ThroughOptions a) where getNoPullOption = getNoPullOption . getOptions diff --git a/src/Restyler/Monad/Git.hs b/src/Restyler/Monad/Git.hs index 8713bb9e..d8677faa 100644 --- a/src/Restyler/Monad/Git.hs +++ b/src/Restyler/Monad/Git.hs @@ -26,6 +26,7 @@ class Monad m => MonadGit m where isGitRepository :: HasCallStack => m Bool gitDiffNameOnly :: HasCallStack => Maybe String -> m [FilePath] gitCommit :: HasCallStack => String -> NonEmpty FilePath -> m String + gitClean :: HasCallStack => m () -- | An instance that invokes the real @git@ newtype ActualGit m a = ActualGit @@ -50,6 +51,7 @@ instance gitCommit msg paths = do runGit_ $ ["commit", "--message", msg, "--"] <> toList paths readGitChomp ["rev-parse", "HEAD"] + gitClean = runGit_ ["clean", "-d", "--force"] runGit_ :: ( MonadUnliftIO m @@ -122,3 +124,4 @@ instance Monad m => MonadGit (NullGit m) where isGitRepository = pure False gitDiffNameOnly _ = pure [] gitCommit _ _ = pure "" + gitClean = pure () diff --git a/src/Restyler/Options/NoClean.hs b/src/Restyler/Options/NoClean.hs new file mode 100644 index 00000000..3878de49 --- /dev/null +++ b/src/Restyler/Options/NoClean.hs @@ -0,0 +1,42 @@ +-- | +-- +-- Module : Restyler.Options.NoClean +-- Copyright : (c) 2024 Patrick Brisbin +-- License : AGPL-3 +-- Maintainer : pbrisbin@gmail.com +-- Stability : experimental +-- Portability : POSIX +module Restyler.Options.NoClean + ( NoCleanOption (..) + , HasNoCleanOption (..) + , getNoClean + , envNoCleanOption + , optNoCleanOption + ) where + +import Restyler.Prelude + +import Env qualified +import Options.Applicative + +newtype NoCleanOption = NoCleanOption + { unwrap :: Any + } + deriving newtype (Semigroup, Monoid) + +class HasNoCleanOption a where + getNoCleanOption :: a -> NoCleanOption + +getNoClean :: (MonadReader env m, HasNoCleanOption env) => m Bool +getNoClean = asks $ getAny . (.unwrap) . getNoCleanOption + +envNoCleanOption :: Env.Parser Env.Error NoCleanOption +envNoCleanOption = + NoCleanOption . Any <$> Env.switch "NO_CLEAN" (Env.help optionHelp) + +optNoCleanOption :: Parser NoCleanOption +optNoCleanOption = + NoCleanOption . Any <$> switch (long "no-clean" <> help optionHelp) + +optionHelp :: String +optionHelp = "Don't run git-clean after restyling" diff --git a/test/SpecHelper.hs b/test/SpecHelper.hs index a5bac1fe..5c183f85 100644 --- a/test/SpecHelper.hs +++ b/test/SpecHelper.hs @@ -64,6 +64,7 @@ import Restyler.Options.FailOnDifferences import Restyler.Options.HostDirectory import Restyler.Options.ImageCleanup import Restyler.Options.Manifest +import Restyler.Options.NoClean import Restyler.Options.NoCommit import Restyler.Options.NoPull import Restyler.Restrictions @@ -154,6 +155,7 @@ testOptions = , imageCleanup = ImageCleanupOption $ Any False , manifest = ManifestOption $ Last Nothing , noCommit = NoCommitOption $ Any False + , noClean = NoCleanOption $ Any False , noPull = NoPullOption $ Any False , restrictions = fullRestrictions } From 3ff33abd756db505260430bde60a24b810b97558 Mon Sep 17 00:00:00 2001 From: patrick brisbin Date: Thu, 10 Oct 2024 08:37:46 -0400 Subject: [PATCH 3/7] Prevent empty changesets when restylers offset each other Sometimes, two restylers can "fight". One changes code one way and then another puts it exactly back. When this happens, each restyler will generate their own commit, but the overall changeset will be empty. If we don't explicitly check for this, using the create-pull-request action will still produce a PR because there are commits, even if the overall diff is empty. This empty PR might just be merged out of habit or process, triggering the process to repeat again. So we'll explicitly check: - If we made changes, - If those changes made commits - If there's no diff against our first commit's parent (`{sha}^`) - Then we'll do a `git reset --hard` back to it --- src/Restyler/Monad/Git.hs | 3 +++ src/Restyler/Restyler/Run.hs | 22 +++++++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/Restyler/Monad/Git.hs b/src/Restyler/Monad/Git.hs index d8677faa..8180ba18 100644 --- a/src/Restyler/Monad/Git.hs +++ b/src/Restyler/Monad/Git.hs @@ -27,6 +27,7 @@ class Monad m => MonadGit m where gitDiffNameOnly :: HasCallStack => Maybe String -> m [FilePath] gitCommit :: HasCallStack => String -> NonEmpty FilePath -> m String gitClean :: HasCallStack => m () + gitResetHard :: HasCallStack => String -> m () -- | An instance that invokes the real @git@ newtype ActualGit m a = ActualGit @@ -52,6 +53,7 @@ instance runGit_ $ ["commit", "--message", msg, "--"] <> toList paths readGitChomp ["rev-parse", "HEAD"] gitClean = runGit_ ["clean", "-d", "--force"] + gitResetHard ref = runGit_ ["reset", "--hard", ref] runGit_ :: ( MonadUnliftIO m @@ -125,3 +127,4 @@ instance Monad m => MonadGit (NullGit m) where gitDiffNameOnly _ = pure [] gitCommit _ _ = pure "" gitClean = pure () + gitResetHard _ = pure () diff --git a/src/Restyler/Restyler/Run.hs b/src/Restyler/Restyler/Run.hs index b5c13dc3..ec6c1774 100644 --- a/src/Restyler/Restyler/Run.hs +++ b/src/Restyler/Restyler/Run.hs @@ -137,11 +137,31 @@ runRestylers config@Config {..} argPaths = do ] for_ cRemoteFiles $ \rf -> downloadFile rf.url rf.path - withFilteredPaths restylers paths $ runRestyler config + + mResults <- withFilteredPaths restylers paths $ runRestyler config + mResetTo <- join <$> traverse checkForNoop mResults + + case mResetTo of + Nothing -> pure mResults + Just ref -> do + logInfo "Restylers offset each other, resetting git state" + Nothing <$ gitResetHard ref where included path = none (`match` path) cExclude restylers = filter rEnabled cRestylers +-- | See if multiple restylers offset each other +-- +-- Returns the git ref to reset to if they did. +checkForNoop :: MonadGit m => NonEmpty RestylerResult -> m (Maybe String) +checkForNoop results = runMaybeT $ do + sha <- hoistMaybe result.sha + let parent = sha <> "^" + changed <- lift $ gitDiffNameOnly $ Just parent + parent <$ guard (null changed) + where + result = head results + -- | Run each @'Restyler'@ with appropriate paths out of the given set -- -- Input is expected to be files (not directories), filtered for existence, and From ba396c0ca6fd3f18c7602eeae704535dbf201601 Mon Sep 17 00:00:00 2001 From: patrick brisbin Date: Thu, 10 Oct 2024 08:40:45 -0400 Subject: [PATCH 4/7] Version bump --- package.yaml | 2 +- restyler.cabal | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package.yaml b/package.yaml index 3417c4b8..b224c3f6 100644 --- a/package.yaml +++ b/package.yaml @@ -1,5 +1,5 @@ name: restyler -version: 0.5.0.1 +version: 0.5.2.0 license: AGPL-3 language: GHC2021 diff --git a/restyler.cabal b/restyler.cabal index 1fd59291..d6d8e3c7 100644 --- a/restyler.cabal +++ b/restyler.cabal @@ -5,7 +5,7 @@ cabal-version: 1.12 -- see: https://github.com/sol/hpack name: restyler -version: 0.5.0.1 +version: 0.5.2.0 license: AGPL-3 build-type: Simple From 9b6bcf8f036f60f8250deaa5c0034a5209edb164 Mon Sep 17 00:00:00 2001 From: patrick brisbin Date: Thu, 10 Oct 2024 08:44:06 -0400 Subject: [PATCH 5/7] Don't expect a restyle-gha executable in test-install --- .github/workflows/ci.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bf37427a..65575b0b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -112,4 +112,3 @@ jobs: - uses: actions/checkout@v4 - run: ./install - run: restyle --help - - run: restyle-gha --help From f17db5b6fde9f08a559d128ceb77f24e69ddbc4a Mon Sep 17 00:00:00 2001 From: patrick brisbin Date: Thu, 10 Oct 2024 08:48:44 -0400 Subject: [PATCH 6/7] Add a little retry to instal fetching latest tag --- install | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/install b/install index aba59816..5ee1c902 100755 --- a/install +++ b/install @@ -44,7 +44,12 @@ done shift $((OPTIND - 1)) if [ -z "$tag" ]; then - tag=$(curl -sSf https://api.github.com/repos/restyled-io/restyler/releases | + tag=$(curl \ + --max-time 10 \ + --retry 5 \ + --retry-delay 0 \ + --retry-max-time 30 \ + -sSf https://api.github.com/repos/restyled-io/restyler/releases | grep -o '"tag_name": ".*"' | sed 's/^.*: "//; s/"$//' | grep -v -- '-rc$' | From 0c3ff9a1173e17c09008243d18b4c5b4a8041747 Mon Sep 17 00:00:00 2001 From: patrick brisbin Date: Thu, 10 Oct 2024 08:53:27 -0400 Subject: [PATCH 7/7] Ensure test-integration runs on demo files --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 65575b0b..c7b1346f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -84,6 +84,7 @@ jobs: options: --limit 3 - uses: restyled-io/actions/run@v2 with: + paths: . log-level: debug release: