Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade versions usage #900

Closed
wants to merge 3 commits into from

Conversation

fosskers
Copy link
Contributor

Hi, I'm the author of versions and noticed that you use it. There were some breaking changes in version 6.0, and this PR updates ghcup to utilize those.

Comment on lines +72 to +76
pvp'' :: Parsec Void T.Text (V.PVP, T.Text)
pvp'' = do
p <- V.pvp'
s <- getParserState
pure (p, stateInput s)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of manually traversing into the structure of the Version, this just reparses the PVP normally and yields whatever was left by using megaparsec functionality.

Comment on lines +27 to +28
- github: fosskers/versions
commit: 7bc3355348aac3510771d4622aff09ac38c9924d
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Temporary until we've confirmed that the fixes are good. Then I'll release 6.0.3 on my end and fix the pin here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't actually use stack to build in CI

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't push to your branch, despite github claiming I should, so here:

commit 94888e9d8edcdde70338d2641b345c2237bf0cb8
Author: Julian Ospald <hasufell@posteo.de>
Date:   Fri Oct 13 17:52:39 2023 +0800

    Add temp git ref to versions to fix CI

diff --git a/cabal.project b/cabal.project
index 2a1fda2..fa6ad49 100644
--- a/cabal.project
+++ b/cabal.project
@@ -8,6 +8,11 @@ package ghcup
 constraints: http-io-streams -brotli,
              any.aeson >= 2.0.1.0
 
+source-repository-package
+  type: git
+  location: https://github.com/fosskers/versions.git
+  tag: 7bc3355348aac3510771d4622aff09ac38c9924d
+
 package libarchive
   flags: -system-libarchive
 
@@ -24,4 +29,4 @@ package streamly
   flags: +use-unliftio
 
 package *
-  test-show-details: direct
\ No newline at end of file
+  test-show-details: direct
diff --git a/cabal.project.release b/cabal.project.release
index b7e3c80..c8c11c4 100644
--- a/cabal.project.release
+++ b/cabal.project.release
@@ -4,6 +4,11 @@ optional-packages: ./vendored/*/*.cabal
 
 optimization: 2
 
+source-repository-package
+  type: git
+  location: https://github.com/fosskers/versions.git
+  tag: 7bc3355348aac3510771d4622aff09ac38c9924d
+
 if os(linux)
   package ghcup
     flags: +tui

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks I'll try that.

@@ -30,15 +32,15 @@ checkList =
(Just $ GHCVersion
$ GHCTargetVersion
Nothing
(mkVersion $ (Digits 9 :| []) :| [Digits 2 :| []]))
$(versionQ "9.2"))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compile-time construction! :D

@fosskers
Copy link
Contributor Author

fosskers commented Oct 13, 2023

Ah, cabal in CI isn't able to pull the dep because it's not released yet.

@hasufell
Copy link
Member

Thanks for the PR. I actually wanted to contact you in private whether you're interested to do some co-maintenance of ghcup (the program, not the distribution channel). It's a fairly small codebase (well, at least to me) and afaik you're familiar with package management and the like.

@hasufell
Copy link
Member

hasufell commented Oct 13, 2023

There are also some property test failures in GHCup.Utils:

From 16ae69e994e6ca35a477ebe1b48241a910d7d0c3 Mon Sep 17 00:00:00 2001
From: Julian Ospald <hasufell@posteo.de>
Date: Fri, 13 Oct 2023 18:08:16 +0800
Subject: [PATCH] Fix property tests

---
 lib/GHCup/Utils.hs | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/GHCup/Utils.hs b/lib/GHCup/Utils.hs
index eb27f64..dc816d5 100644
--- a/lib/GHCup/Utils.hs
+++ b/lib/GHCup/Utils.hs
@@ -119,11 +119,11 @@ import Data.Time (Day(..), diffDays, addDays)
 -- >>> let lc = LoggerConfig { lcPrintDebug = False, consoleOutter = mempty, fileOutter = mempty, fancyColors = False }
 -- >>> dirs' <- getAllDirs
 -- >>> let installedVersions = [ ([pver|8.10.7|], "-debug+lol", Nothing), ([pver|8.10.4|], "", Nothing), ([pver|8.8.4|], "", Nothing), ([pver|8.8.3|], "", Nothing) ]
--- >>> let settings = Settings True 0 False Never Curl False GHCupURL True GPGNone False
+-- >>> let settings = defaultSettings { cache = True, metaCache = 0, noNetwork = True }
 -- >>> let leanAppState = LeanAppState settings dirs' defaultKeyBindings lc
 -- >>> cwd <- getCurrentDirectory
 -- >>> (Right ref) <- pure $ parseURI strictURIParserOptions $ "file://" <> E.encodeUtf8 (T.pack cwd) <> "/data/metadata/" <> (urlBaseName . view pathL' $ ghcupURL)
--- >>> (VRight r) <- (fmap . fmap) _ghcupDownloads $ flip runReaderT leanAppState . runE @'[DigestError, GPGError, JSONError , DownloadFailed , FileDoesNotExistError] $ liftE $ getBase ref
+-- >>> (VRight r) <- (fmap . fmap) _ghcupDownloads $ flip runReaderT leanAppState . runE @'[DigestError, GPGError, JSONError , DownloadFailed , FileDoesNotExistError, ContentLengthError] $ liftE $ getBase ref
 
 
 
@@ -730,7 +730,7 @@ getGHCForPVP pvpIn mt = do
 -- | Like 'getGHCForPVP', except with explicit input parameter.
 --
 -- >>> getGHCForPVP' [pver|8|] installedVersions Nothing
--- Just (GHCTargetVersion {_tvTarget = Nothing, _tvVersion = Version {_vEpoch = Nothing, _vChunks = (Digits 8 :| []) :| [Digits 10 :| [],Digits 7 :| []], _vRel = [Str "debug" :| []], _vMeta = Just "lol"}})
+-- Just (GHCTargetVersion {_tvTarget = Nothing, _tvVersion = Version {_vEpoch = Nothing, _vChunks = Chunks (Numeric 8 :| [Numeric 10,Numeric 7]), _vRel = Just (Release (Alphanum "debug" :| [])), _vMeta = Just "lol"}})
 -- >>> fmap prettyShow $ getGHCForPVP' [pver|8.8|] installedVersions Nothing
 -- "Just 8.8.4"
 -- >>> fmap prettyShow $ getGHCForPVP' [pver|8.10.4|] installedVersions Nothing
@@ -756,11 +756,11 @@ getGHCForPVP' pvpIn ghcs' mt = do
 -- | Get the latest available ghc for the given PVP version, which
 -- may only contain parts.
 --
--- >>> (fmap . fmap) fst $ getLatestToolFor GHC [pver|8|] r
+-- >>> (fmap . fmap) (\(p, _, _) -> p) $ getLatestToolFor GHC Nothing [pver|8|] r
 -- Just (PVP {_pComponents = 8 :| [10,7]})
--- >>> (fmap . fmap) fst $ getLatestToolFor GHC [pver|8.8|] r
+-- >>> (fmap . fmap) (\(p, _, _) -> p) $ getLatestToolFor GHC Nothing [pver|8.8|] r
 -- Just (PVP {_pComponents = 8 :| [8,4]})
--- >>> (fmap . fmap) fst $ getLatestToolFor GHC [pver|8.8.4|] r
+-- >>> (fmap . fmap) (\(p, _, _) -> p) $ getLatestToolFor GHC Nothing [pver|8.8.4|] r
 -- Just (PVP {_pComponents = 8 :| [8,4]})
 getLatestToolFor :: MonadThrow m
                  => Tool
@@ -1031,7 +1031,7 @@ applyPatches pdir ddir = do
 
   patches <- liftIO $ quilt `catchIO` (\e ->
     if isDoesNotExistError e || isPermissionError e then
-      lexicographical 
+      lexicographical
     else throwIO e)
   forM_ patches $ \patch' -> applyPatch patch' ddir
 
-- 
2.41.0

run via: cabal-docspec -XCPP -XTypeSynonymInstances -XOverloadedStrings -XPackageImports --check-properties

@hasufell
Copy link
Member

Merged and updated manually

@hasufell hasufell closed this Oct 21, 2023
@fosskers
Copy link
Contributor Author

Merged and updated manually

... did it work? This PR included things I haven't released yet in the official version.

@fosskers
Copy link
Contributor Author

fosskers commented Oct 23, 2023

Ah I see now, you pinned the versions version to a git commit. I'll make a release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants