Fix for incorrectly placed check + control flow modification to avoid such
mistakes in the future.

1 patch for repository http://darcs.net:

Fri Nov  5 19:51:29 EET 2010  Alexey Levan <exlevan@gmail.com>
  * Fix check for --packs flag

Attachments

• fix-check-for-__packs-flag.dpatch
• unnamed

hunk ./src/Darcs/Repository.hs 87
>  #endif
>  import URL ( maxPipelineLength )
>  
> -import Control.Applicative ( (<$>) )
>  import Control.Exception ( finally )
>  import Control.Concurrent ( forkIO )
>  import Control.Concurrent.MVar ( MVar, newMVar, putMVar, takeMVar )

hunk ./src/Darcs/Repository.hs 260
>    debugMessage "Copying prefs"
>    copyFileOrUrl (remoteDarcs opts) (fromDir ++ "/" ++ darcsdir ++ 
"/prefs/prefs")
>      (darcsdir ++ "/prefs/prefs") (MaxAge 600) `catchall` return ()
> -  if isFile fromDir && Packs `elem` opts && NoPacks `notElem` opts
> -    then copyNotPackedRepository fromRepo
> -    else do
> -      b <- (Just <$> fetchFileLazyPS (fromDir ++ "/" ++ darcsdir ++
> -        "/packs/basic.tar.gz") Uncachable) `catchall` return Nothing
> -      case b of
> -        Nothing -> copyNotPackedRepository fromRepo
> -        Just b' -> copyPackedRepository fromRepo b'
> +  -- try packs for remote repositories
> +  if (not . isFile) fromDir && Packs `elem` opts && NoPacks `notElem` 
opts
> +    then copyPackedRepository fromRepo
> +    else copyNotPackedRepository fromRepo
>  
>  putInfo :: [DarcsFlag] -> Doc -> IO ()
>  putInfo opts = unless (Quiet `elem` opts) . hPutDocLn stderr

If I understand correctly, your patch does not change what effectively
happens, but makes the code clearer.

By the way, using (Packs `elem` opts && NoPacks `notElem` opts) is
probably not what you want: you want to give priority to whichever
flag is passed last on the command line. Ie, if i put --no-packs in
my _darcs/prefs, I should be able to override it later on the
command-line. Can you fix that in a followup? You can use getBoolFlag
from Flags.hs for that.


hunk ./src/Darcs/Repository.hs 293
>                     pristineFromWorking torepository
>  
>  copyPackedRepository :: forall p C(r u t). RepoPatch p =>
> -  Repository p C(r u t) -> BL.ByteString -> IO ()
> -copyPackedRepository fromRepo@(Repo fromDir opts _ (DarcsRepository _ 
fromCache)) b = do
> +  Repository p C(r u t) -> IO ()
> +copyPackedRepository r =
> +  -- fallback to no-packs get in case of error
> +  copyPackedRepository2 r `catchall` copyNotPackedRepository r
> +
> +copyPackedRepository2 :: forall p C(r u t). RepoPatch p =>
> +  Repository p C(r u t) -> IO ()
> +copyPackedRepository2 fromRepo@(Repo fromDir opts _ (DarcsRepository 
_ fromCache)) = do
> +  b <- fetchFileLazyPS (fromDir ++ "/" ++ darcsdir ++ 
"/packs/basic.tar.gz") Uncachable
>    Repo toDir _ toFormat (DarcsRepository toPristine toCache) <-
>      identifyRepositoryFor fromRepo "."
>    toCache2 <- unionRemoteCaches toCache fromCache fromDir

This is clearer.

Florent Becker <bugs@darcs.net> writes:

>> +  if (not . isFile) fromDir && Packs `elem` opts && NoPacks `notElem` 

> By the way, using (Packs `elem` opts && NoPacks `notElem` opts) is
> probably not what you want: you want to give priority to whichever
> flag is passed last on the command line. Ie, if i put --no-packs in
> my _darcs/prefs, I should be able to override it later on the
> command-line. Can you fix that in a followup? You can use getBoolFlag
> from Flags.hs for that.

If it's an "exclusive" flag, it doesn't really matter, since the
(No)Packs elements are nub-ed, so you never get both. Hopefully, we can
clear the option parsing issues soon enough (cmdlib has seen more work
recently thanks to Eric's endeavours with it; I believe it would be
feasible in maybe a couple of weeks to try to port darcs over;
unfortunately, it is going to be a huge mass of changes and probably not
very incremental).

Yours,
   Petr.

By request, followup with better check for a packs flag.

2 patches for repository http://darcs.net/screened:

Fri Nov  5 19:51:29 EET 2010  Alexey Levan <exlevan@gmail.com>
  * Fix check for --packs flag

I'm resending this patch since it hasn't been applied to the screened branch.  No changes here.


Mon Nov 22 13:17:02 EET 2010  Alexey Levan <exlevan@gmail.com>
  * Use getBoolFlag to check for a packs flag

Actual change.

Attachments

• fix-check-for-__packs-flag.dpatch
• unnamed

Fix check for --packs flag
--------------------------
Alexey Levan <exlevan@gmail.com>**20101105175129

Already reviewed

Use getBoolFlag to check for a packs flag
-----------------------------------------
Alexey Levan <exlevan@gmail.com>**20101122111702

hunk ./src/Darcs/Flags.hs 25

hunk ./src/Darcs/Flags.hs 189
>  doReverse :: [DarcsFlag] -> Bool
>  doReverse = getBoolFlag Reverse Forward
>  
> +usePacks :: [DarcsFlag] -> Bool
> +usePacks = getBoolFlag Packs NoPacks
> +
>  showChangesOnlyToFiles :: [DarcsFlag] -> Bool
>  showChangesOnlyToFiles = getBoolFlag OnlyChangesToFiles 
ChangesToAllFiles
>  
ok

hunk ./src/Darcs/Repository.hs 132

hunk ./src/Darcs/Repository.hs 268
>    copyFileOrUrl (remoteDarcs opts) (fromDir ++ "/" ++ darcsdir ++ 
"/prefs/prefs")
>      (darcsdir ++ "/prefs/prefs") (MaxAge 600) `catchall` return ()
>    -- try packs for remote repositories
> -  if (not . isFile) fromDir && Packs `elem` opts && NoPacks `notElem` 
opts
> +  if (not . isFile) fromDir && usePacks opts
>      then copyPackedRepository fromRepo
>      else copyNotPackedRepository fromRepo
>  
ok, actually use it.

This patch bundle (with 1 patches) was just applied to the repository http://darcs.net/.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_.html#bundle-321b9149192f0a371895d1edf6f6c990d09bdf1f

This patch bundle (with 2 patches) was just applied to the repository http://darcs.net/.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_.html#bundle-f618c615a4adafe99ed630ded58131e25d108732

This patch bundle (with 2 patches) was just applied to the repository http://darcs.net/reviewed.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_reviewed.html#bundle-f618c615a4adafe99ed630ded58131e25d108732

This patch bundle (with 1 patches) was just applied to the repository http://darcs.net/reviewed.
This message was brought to you by DarcsWatch
http://darcswatch.nomeata.de/repo_http:__darcs.net_reviewed.html#bundle-321b9149192f0a371895d1edf6f6c990d09bdf1f