[darcs-devel] [patch918] Import and adapt original patch-index co... (and 23 more)

Aditya bsrkaditya at gmail.com
Fri Nov 2 07:18:21 UTC 2012


On Thu, Nov 1, 2012 at 7:22 PM, Florent Becker <bugs at darcs.net> wrote:

>
> Florent Becker <florent.becker at ens-lyon.org> added the comment:
>
> This looks good, with some questions. Ganesh, do you want to weigh in or
> should I push this to reviewed?
>
>
> New patches:
>
> [Import and adapt original patch-index code from Benedikt.
> bsrkaditya at gmail.com**20120824160813
>  Ignore-this: 504f5382b48ee145e410bfc366368d32
>
>  Rebased by Eric Kow <kowey at darcs.net>
>  I've consolidated these from several patches by Aditya.
> ]
>
> […] Already reviewed
>
>
> [Tidy up patch-index code and remove unused parts.
> bsrkaditya at gmail.com**20120825100236
>  Ignore-this: efab4931b41463d5e78cad25e3235d4
>
>  Rebased by Eric Kow <kowey at darcs.net>
>  This is an amalgam of patches from Aditya's GSoC 2012 project.
> ]
>
> […] Already reviewed
>
> [Flags related to patch-index support.
> bsrkaditya at gmail.com**20120825101016
>  Ignore-this: f0376f725182b2f717b33acc2cd6317
>
>  Rebased by Eric Kow <kowey at darcs.net>
>  This is an amalgam of patches from Aditya's GSoC 2012 project.
> ]
>
> This is all standard, except:
>
> +runPatchIndex :: [DarcsFlag] -> RF.PatchIndexOption
> +runPatchIndex fs | not $ NoPatchIndexFlag `elem` fs = RF.YesPatchIndex
> +                 | otherwise = RF.NoPatchIndex
> +
>
> Shouldn't this rather use "getBoolFlag" to allow overriding defaults?
>
> [Add optimize --patch-index (and --disable-patch-index).
> bsrkaditya at gmail.com**20120825103641
>  Ignore-this: 5d9fb02041f61b09983a64c792e6a2f2
>
>  Rebased by Eric Kow <kowey at darcs.net>
>  This is an amalgam of patches from Aditya's GSoC 2012 project.
> ]
>
> […] Already reviewed
>
> [darcs show patch-index subcommands.
> bsrkaditya at gmail.com**20120825103718
>  Ignore-this: 85344a93da889ecba910bdcda760ab1a
>
>  - show patch-index-all: dumps the patch index
>  - show patch-index-files: shows files known by the index
>  - show patch-index-status: reports if it's in sync or not
>
>  Rebased by Eric Kow <kowey at darcs.net>
>  This is an amalgam of patches from Aditya's GSoC 2012 project.
> ]
>
> […] Already reviewed
>
> [Move readRepo to new Darcs.Repository.Read module.
> bsrkaditya at gmail.com**20120825103718
>  Ignore-this: a02a295e788b936923a2d3d49f239b19
>
>  This function is needed by the patch index work.
>  It's moved out to avoid an import cycle later on.
>
>  Rebased by Eric Kow <kowey at darcs.net>
>  This is an amalgam of patches from Aditya's GSoC 2012 project.
> ]
>
> […] Already reviewed
>
> [Update patch index when creating or modifying a repository.
> bsrkaditya at gmail.com**20120825103718
>  Ignore-this: c7cb266b949cc2fa8bffd49af6d81b6e
>
>  Rebased by Eric Kow <kowey at darcs.net>
>  This is an amalgam of patches from Aditya's GSoC 2012 project.
> ]
>
> […] Already reviewed
>
> [Use patch index in darcs annotate and changes
> bsrkaditya at gmail.com**20120825103718
>  Ignore-this: a74db55b1c3693d2a67fa3d03960aac0
>
>  Rebased by Eric Kow <kowey at darcs.net>
>  This is an amalgam of patches from Aditya's GSoC 2012 project.
> ]
>
> […] Already reviewed
>
> [resolve ambiguous options in tests
> bsrkaditya at gmail.com**20120825103718
>  Ignore-this: f1f1c4de03d603044be9cdc91c829ab
>
>  Rebased by Eric Kow <kowey at darcs.net>
> ]
> --patch not being a valid abbreviation for --patches worries me a bit,
> but I guess we should
> retrain ourselves. Can't we use --use-patch-index or --with-patch-index
> instead?
>
> [Add test for annotate on directories
> bsrkaditya at gmail.com**20120825103718
>  Ignore-this: 713b00f2f4bdce53e000be0a2199ad30
>
>  Rebased by Eric Kow <kowey at darcs.net>
> ]
>
> Reduced to nothingness by rebase?
>
> [disable patch index in tests for repair-corrupt
> bsrkaditya at gmail.com**20120825103718
>  Ignore-this: 17967897855d07d55a2c95f2d3c9f534
>
>  Rebased by Eric Kow <kowey at darcs.net>
> ]
>
> Patch index prevents us from doing silly things, but in tests, we need to.
>
> [disable patch index in lazy-optimize-reorder test
> bsrkaditya at gmail.com**20120825103718
>  Ignore-this: 89691dc76d8bc155336da5ebc8f9c66e
>
>  Rebased by Eric Kow <kowey at darcs.net>
> ]
>
> same idea, patch index means we fetch the patches on "darcs changes".
>
> [Print a message when building patch index on old repos
> bsrkaditya at gmail.com**20120825103718
>  Ignore-this: 871ad9539abb21be128d99d07ef2ba02
>
>  Rebased by Eric Kow <kowey at darcs.net>
> ]
>
> […] Already reviewed
>
> [Add a test command
> bsrkaditya at gmail.com**20120825103718
>  Ignore-this: faa5e28f7b601ad73ff9caa7071d25de
>
>  Rebased by Eric Kow <kowey at darcs.net>
> ]
>
> […] Already reviewed
>
> [If user uses ctrl-c at get, do not create patch index
> bsrkaditya at gmail.com**20120825103718
>  Ignore-this: bc9ec39af5b0f8661fbe12b84d4aa6aa
>
>  Rebased by Eric Kow <kowey at darcs.net>
> ]
>
> […] Already reviewed
>
> [Do not create patch index at get if --disable-patch-index is passed
> bsrkaditya at gmail.com**20120825103718
>  Ignore-this: eef6daa072304457d77341350d27c85
>
>  Rebased by Eric Kow <kowey at darcs.net>
> ]
>
> What it says on the tin
>
> [Add -fno-warn-missing-methods compiler option to FileModMonad
> bsrkaditya at gmail.com**20120825103719
>  Ignore-this: fe49c21e881ce012cca33015989a6bd2
> ]
>
> I'd rather you had used "undefined" as definition for the methods, but
> nevermind.
>
> [Allow user interrupt when building patch index on existing repsitories
> bsrkaditya at gmail.com**20120825103719
>  Ignore-this: 26c1196658e2e9422eaa6c961020fd3
>
>  Rebased by Eric Kow <kowey at darcs.net>
>  This is an amalgam of patches from Aditya's GSoC 2012 project.
> ]
>
> […] Already reviewed
>
> [more compact filterPatches function
> bsrkaditya at gmail.com**20120825103719
>  Ignore-this: ff677df3caad8063cebf8c3484c3d2c4
>  Rebased by Eric Kow <kowey at darcs.net>
> ]
>
> Trivial
>
> [Add patch index correctness and timing scripts to contrib
> bsrkaditya at gmail.com**20120825103719
>  Ignore-this: 7f74ce2b61d320d2a76deaf5251ba8f7
>  Rebased by Eric Kow <kowey at darcs.net>
> ]
>
> Ok
>
> [Add haddock for lookupFid
> bsrkaditya at gmail.com**20120825103719
>  Ignore-this: 25bdcd272e527dfab73d1f4f71fcee0b
>  Rebased by Eric Kow <kowey at darcs.net>
> ]
>
> Ok
>
> [Add test changes-duplicate
> bsrkaditya at gmail.com**20120825103719
>  Ignore-this: 8e13b5e0faf2a0d6d8bcf8a34f4879fc
>
>  Rebased by Eric Kow <kowey at darcs.net>
>  This is an amalgam of patches from Aditya's GSoC 2012 project.
> ]
>
> Ok
>
> [Add patch-index tests.
> bsrkaditya at gmail.com**20120825103756
>  Ignore-this: 1f1805db4d8028256816fb82b56959f1
>
>  Rebased by Eric Kow <kowey at darcs.net>
>  This is an amalgam of patches from Aditya's GSoC 2012 project.
> ]
>
> Ok
>
> [If the path does not have a file, lookupFids should return an empty list
> bsrkaditya at gmail.com**20120825103803
>  Ignore-this: 2228f12c7bfca2475f38487716070c0e
>  Rebased by Eric Kow <kowey at darcs.net>
> ]
>
>  -- | lookup current fid of filepath
> -lookupFid :: FileName -> String -> PIM FileId
> -lookupFid fn hint = do
> -  fidm <- gets fidspans
> -  case M.lookup fn fidm of
> -    Just (FidSpan fid _ _:_) -> return fid
> -    Nothing ->
> -      error $ "lookupFid: impossible, no entry for "++show fn++" in
> FileIdSpans in "++hint
> -    Just [] ->
> -      error $ "lookupFid: impossible, no entry for "++show fn++" in
> FileIdSpans in "
> -              ++hint++", empty list "
> +lookupFid :: FileName -> PIM FileId
> +lookupFid fn = fromJust <$> lookupFid' fn
> +
> +-- | lookup current fid of filepatch, returning a Maybe to allow failure
> +lookupFid' :: FileName -> PIM (Maybe FileId)
> +lookupFid' fn = do
> +   fidm <- gets fidspans
> +   case M.lookup fn fidm of
> +    Just (FidSpan fid _ _:_) -> return $ Just fid
> +    _ -> return Nothing
> +
>
>
>  -- | lookup all the file ids of a given path
>  lookupFidf' :: FileName -> PIM [FileId]
> hunk ./src/Darcs/Repository/FileMod.hs 274
>  --   at any point in repository history
>  lookupFids' :: FileName -> PIM [FileId]
>  lookupFids' fn = do
> -   fid <- lookupFid fn ""
> -   info_map <- gets infom
> -   fps_spans <- gets fpspans
> -   case M.lookup fid info_map of
> -      Just (FileInfo True _) -> return [fid]
> -      Just (FileInfo False _) -> do
> -         let file_names = map (\(FpSpan x _ _) -> x) (fps_spans M.! fid)
> -         nub . concat <$> mapM lookupFids file_names
> -      Nothing -> error "lookupFids' : could not find file"
> +  info_map <- gets infom
> +  fps_spans <- gets fpspans
> +  a <- lookupFid' fn
> +  if isJust a then do
> +                let fid = fromJust a
> +                case M.lookup fid info_map of
> +                  Just (FileInfo True _) -> return [fid]
> +                  Just (FileInfo False _) ->
> +                    let file_names = map (\(FpSpan x _ _) -> x)
> (fps_spans M.! fid)
> +                    in nub . concat <$> mapM lookupFids file_names
> +                  Nothing -> error "lookupFids' : could not find file"
> +              else return []
>
> Why return [] instead of keeping the "impossible" from the previous
> version of lookupFid?
>
>
I think it's is because the user may input a non existent file at changes,
and lookupFids'
should not crash it that is the case.


> __________________________________
> Darcs bug tracker <bugs at darcs.net>
> <http://bugs.darcs.net/patch918>
> __________________________________
> _______________________________________________
> darcs-devel mailing list
> darcs-devel at darcs.net
> http://lists.osuosl.org/mailman/listinfo/darcs-devel
>



-- 
BSRK Aditya
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osuosl.org/pipermail/darcs-devel/attachments/20121102/a964926f/attachment-0001.html>


More information about the darcs-devel mailing list