Add information about ref handling

This adds some warnings when users with existing `ref`-laden sources try
to `niv update`.

src/Niv/Cli.hs

@@ -176,7 +176,7 @@ cmdInit nixpkgs = do
             -- Imports @niv@ and @nixpkgs@
             say "Importing 'niv' ..."
             cmdAdd
-              (updateCmd githubCmd)
+              githubCmd
               (PackageName "niv")
               ( specToFreeAttrs $ PackageSpec $
                   HMS.fromList
@@ -191,7 +191,7 @@ cmdInit nixpkgs = do
                 let (owner, repo) = case nixpkgs' of
                       Nixpkgs o r -> (o, r)
                 cmdAdd
-                  (updateCmd githubCmd)
+                  githubCmd
                   (PackageName "nixpkgs")
                   ( specToFreeAttrs $ PackageSpec $
                       HMS.fromList
@@ -249,8 +249,8 @@ parseCmdAdd =
     -- implementer: it'll be tricky to have the correct arguments show up
     -- without repeating "PACKAGE PACKAGE PACKAGE" for every package type.
     parseShortcuts = parseShortcut githubCmd
-    parseShortcut cmd = uncurry (cmdAdd (updateCmd cmd)) <$> (parseShortcutArgs cmd)
-    parseCmd cmd = uncurry (cmdAdd (updateCmd cmd)) <$> (parseCmdArgs cmd)
+    parseShortcut cmd = uncurry (cmdAdd cmd) <$> (parseShortcutArgs cmd)
+    parseCmd cmd = uncurry (cmdAdd cmd) <$> (parseCmdArgs cmd)
     parseCmdAddGit =
       Opts.info (parseCmd gitCmd <**> Opts.helper) (description gitCmd)
     parseCmdAddLocal =
@@ -321,15 +321,15 @@ parseCmdArgs cmd = collapse <$> parseNameAndShortcut <*> parsePackageSpec cmd
                 <> Opts.help "Set the package name to <NAME>"
             )
 
-cmdAdd :: Update () a -> PackageName -> Attrs -> NIO ()
-cmdAdd updateFunc packageName attrs = do
+cmdAdd :: Cmd -> PackageName -> Attrs -> NIO ()
+cmdAdd cmd packageName attrs = do
   job ("Adding package " <> T.unpack (unPackageName packageName)) $ do
     fsj <- getFindSourcesJson
     sources <- unSources <$> li (getSources fsj)
     when (HMS.member packageName sources)
       $ li
       $ abortCannotAddPackageExists packageName
-    eFinalSpec <- fmap attrsToSpec <$> li (tryEvalUpdate attrs updateFunc)
+    eFinalSpec <- fmap attrsToSpec <$> li (doUpdate attrs cmd)
     case eFinalSpec of
       Left e -> li (abortUpdateFailed [(packageName, e)])
       Right finalSpec -> do
@@ -413,12 +413,8 @@ cmdUpdate = \case
                 Just "git" -> gitCmd
                 Just "local" -> localCmd
                 _ -> githubCmd
-          fmap attrsToSpec
-            <$> li
-              ( tryEvalUpdate
-                  (specToLockedAttrs cliSpec <> specToFreeAttrs defaultSpec)
-                  (updateCmd cmd)
-              )
+              spec = specToLockedAttrs cliSpec <> specToFreeAttrs defaultSpec
+          fmap attrsToSpec <$> li (doUpdate spec cmd)
         Nothing -> li $ abortCannotUpdateNoSuchPackage packageName
       case eFinalSpec of
         Left e -> li $ abortUpdateFailed [(packageName, e)]
@@ -438,13 +434,7 @@ cmdUpdate = \case
               Just "git" -> gitCmd
               Just "local" -> localCmd
               _ -> githubCmd
-        finalSpec <-
-          fmap attrsToSpec
-            <$> li
-              ( tryEvalUpdate
-                  initialSpec
-                  (updateCmd cmd)
-              )
+        finalSpec <- fmap attrsToSpec <$> li (doUpdate initialSpec cmd)
         pure finalSpec
     let (failed, sources') = partitionEithersHMS esources'
     unless (HMS.null failed)
@@ -452,6 +442,12 @@ cmdUpdate = \case
       $ abortUpdateFailed (HMS.toList failed)
     li $ setSources fsj $ Sources sources'
 
+-- | pretty much tryEvalUpdate but we might issue some warnings first
+doUpdate :: Attrs -> Cmd -> IO (Either SomeException Attrs)
+doUpdate attrs cmd = do
+  forM_ (extraLogs cmd attrs) $ tsay
+  tryEvalUpdate attrs (updateCmd cmd)
+
 partitionEithersHMS ::
   (Eq k, Hashable k) =>
   HMS.HashMap k (Either a b) ->

src/Niv/Cmd.hs

@@ -15,5 +15,7 @@ data Cmd
         parseCmdShortcut :: T.Text -> Maybe (PackageName, Aeson.Object),
         parsePackageSpec :: Opts.Parser PackageSpec,
         updateCmd :: Update () (),
-        name :: T.Text
+        name :: T.Text,
+        -- | Some notes to print
+        extraLogs :: Attrs -> [T.Text]
       }

src/Niv/Git/Cmd.hs

@@ -31,9 +31,28 @@ gitCmd =
       parseCmdShortcut = parseGitShortcut,
       parsePackageSpec = parseGitPackageSpec,
       updateCmd = gitUpdate',
-      name = "git"
+      name = "git",
+      extraLogs = gitExtraLogs
     }
 
+gitExtraLogs :: Attrs -> [T.Text]
+gitExtraLogs attrs = noteRef <> warnRefBranch <> warnRefTag
+  where
+    noteRef =
+      textIf (HMS.member "ref" attrs) $
+        mkNote
+          "Your source contains a `ref` attribute. Make sure your sources.nix is up-to-date and consider using a `branch` or `tag` attribute."
+    warnRefBranch =
+      textIf (member "ref" && member "branch") $
+        mkWarn
+          "Your source contains both a `ref` and a `branch`. Niv will update the `branch` but the `ref` will be used by Nix to fetch the repo."
+    warnRefTag =
+      textIf (member "ref" && member "tag") $
+        mkWarn
+          "Your source contains both a `ref` and a `tag`. The `ref` will be used by Nix to fetch the repo."
+    member x = HMS.member x attrs
+    textIf cond txt = if cond then [txt] else []
+
 parseGitShortcut :: T.Text -> Maybe (PackageName, Aeson.Object)
 parseGitShortcut txt'@(T.dropWhileEnd (== '/') -> txt) =
   -- basic heuristics for figuring out if something is a git repo
@@ -71,6 +90,7 @@ parseGitPackageSpec =
       ("branch",) . Aeson.String
         <$> Opts.strOption
           ( Opts.long "branch"
+              <> Opts.short 'b'
               <> Opts.metavar "BRANCH"
           )
     parseAttr =

src/Niv/GitHub/Cmd.hs

@@ -38,7 +38,8 @@ githubCmd =
       parseCmdShortcut = parseAddShortcutGitHub,
       parsePackageSpec = parseGitHubPackageSpec,
       updateCmd = githubUpdate',
-      name = "github"
+      name = "github",
+      extraLogs = const []
       -- TODO: here filter by type == tarball or file or builtin-
     }
 

src/Niv/Local/Cmd.hs

@@ -27,7 +27,8 @@ localCmd =
       updateCmd = proc () -> do
         useOrSet "type" -< ("local" :: Box T.Text)
         returnA -< (),
-      name = "local"
+      name = "local",
+      extraLogs = const []
     }
 
 parseLocalShortcut :: T.Text -> Maybe (PackageName, Aeson.Object)

src/Niv/Logger.hs

@@ -8,6 +8,9 @@ module Niv.Logger
     bug,
     tsay,
     say,
+    twarn,
+    mkWarn,
+    mkNote,
     green,
     tgreen,
     red,
@@ -73,6 +76,15 @@ say msg = do
   -- the end
   liftIO $ putStrLn $ intercalate "\n" $ (indent <>) <$> lines msg
 
+mkWarn :: T.Text -> T.Text
+mkWarn w = tbold (tyellow "WARNING") <> ": " <> w
+
+twarn :: MonadIO io => T.Text -> io ()
+twarn = tsay . mkWarn
+
+mkNote :: T.Text -> T.Text
+mkNote w = tbold (tblue "NOTE") <> ": " <> w
+
 green :: S
 green str =
   ANSI.setSGRCode [ANSI.SetConsoleIntensity ANSI.BoldIntensity]

src/Niv/Sources.hs

@@ -242,9 +242,9 @@ warnIfOutdated :: IO ()
 warnIfOutdated = do
   tryAny (BL8.readFile pathNixSourcesNix) >>= \case
     Left e ->
-      tsay $
-        T.unlines -- warn with tsay
-          [ T.unwords [tyellow "WARNING:", "Could not read", T.pack pathNixSourcesNix],
+      twarn $
+        T.unlines
+          [ T.unwords ["Could not read", T.pack pathNixSourcesNix],
             T.unwords ["  ", "(", tshow e, ")"]
           ]
     Right content -> do

tests/git/default.nix

@@ -63,7 +63,7 @@ pkgs.runCommand "git-test"
         exit 42
       fi
 
-      niv update my-git-repo -a ref=branch
+      niv update my-git-repo -b branch
       nivrev2=$(nix eval --json '(import ./nix/sources.nix).my-git-repo.rev' | jq -r)
       if [ ! "$gitrev2" = "$nivrev2" ]; then
         echo "Mismatched revs: $gitrev2 != $nivrev2"