[darcs-devel] [patch2135] index refactors

Ganesh Sittampalam bugs at darcs.net
Sun Dec 27 13:17:55 UTC 2020

Ganesh Sittampalam <ganesh at earth.li> added the comment:

Partial review:

>   * avoid useless CPP in D.U.Index and D.R.State
>   We can use the same action (renameFile instead of removeFile) for both
>   Windows and Posix. The point here is that the index may still be open via an
>   mmapped ForeignPtr, so removing it is not portable, but renaming it should
>   work on all systems.

> - -- TODO this conditional logic (rename or delete) is mirrored in
> - -- Darcs.Repository.State.checkIndex and should be refactored

I think it'd still be good to have a single function, or to keep the
TODO pointing between the two bits of code, even though the logic is
only one line now.

That way if someone does want to change it in future they won't
accidentally change one and not the other.

>   * index: re-use getFileStatus for getFileID' (Posix)

Is this intended to be behaviour-preserving? getFileStatus (defined in
the same file) seems to behave differently on symbolic links to the old

>   * index: move update of fileid out of createItem
>   This is a preparation for a later refactor.

OK - I shall assume the later refactor is worthwhile :-)

Does the doc comment for createItem need to change?

>   * index: use realPath instead of anchorPath ""


>   * index: in the docs, replace "line" with "Item", clarify size/alignment


>   * show index: show everything, not just hash, id, and path

>  entries <- dumpIndex indexPath `catchall` return []

Should we avoid introducing new uses of catchall? Is there a specific
kind we could catch here?

>   * darcs check: add a warning when we cannot access the index
>   * Darcs.Util.Index: rename updateIndex to treeFromIndex
>   * rename D.Util.Index.readIndex to openIndex


Darcs bug tracker <bugs at darcs.net>

More information about the darcs-devel mailing list