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

Detect and propagate renames/moves instead of delete+create #576

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions src/.depend
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ common.cmo : \
name.cmi \
fspath.cmi \
fileinfo.cmi \
features.cmi \
common.cmi
common.cmx : \
uutil.cmx \
Expand All @@ -75,6 +76,7 @@ common.cmx : \
name.cmx \
fspath.cmx \
fileinfo.cmx \
features.cmx \
common.cmi
common.cmi : \
uutil.cmi \
Expand Down Expand Up @@ -630,6 +632,37 @@ main.cmx : \
os.cmx
make_tools.cmo :
make_tools.cmx :
moves.cmo : \
uutil.cmi \
ubase/util.cmi \
update.cmi \
ubase/trace.cmi \
ubase/safelist.cmi \
props.cmi \
ubase/prefs.cmi \
path.cmi \
os.cmi \
name.cmi \
features.cmi \
common.cmi \
moves.cmi
moves.cmx : \
uutil.cmx \
ubase/util.cmx \
update.cmx \
ubase/trace.cmx \
ubase/safelist.cmx \
props.cmx \
ubase/prefs.cmx \
path.cmx \
os.cmx \
name.cmx \
features.cmx \
common.cmx \
moves.cmi
moves.cmi : \
path.cmi \
common.cmi
name.cmo : \
ubase/util.cmi \
ubase/umarshal.cmi \
Expand Down Expand Up @@ -845,6 +878,7 @@ recon.cmo : \
pred.cmi \
path.cmi \
name.cmi \
moves.cmi \
globals.cmi \
fileinfo.cmi \
common.cmi \
Expand All @@ -863,6 +897,7 @@ recon.cmx : \
pred.cmx \
path.cmx \
name.cmx \
moves.cmx \
globals.cmx \
fileinfo.cmx \
common.cmx \
Expand Down Expand Up @@ -1055,6 +1090,7 @@ test.cmo : \
ubase/prefs.cmi \
path.cmi \
os.cmi \
moves.cmi \
lwt/lwt_unix.cmi \
lwt/lwt.cmi \
globals.cmi \
Expand All @@ -1077,6 +1113,7 @@ test.cmx : \
ubase/prefs.cmx \
path.cmx \
os.cmx \
moves.cmx \
lwt/lwt_unix.cmx \
lwt/lwt.cmx \
globals.cmx \
Expand Down Expand Up @@ -1119,6 +1156,7 @@ transport.cmo : \
ubase/prefs.cmi \
path.cmi \
osx.cmi \
moves.cmi \
lwt/lwt_util.cmi \
lwt/lwt.cmi \
globals.cmi \
Expand All @@ -1135,6 +1173,7 @@ transport.cmx : \
ubase/prefs.cmx \
path.cmx \
osx.cmx \
moves.cmx \
lwt/lwt_util.cmx \
lwt/lwt.cmx \
globals.cmx \
Expand Down
2 changes: 1 addition & 1 deletion src/Makefile.OCaml
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ OCAMLOBJS = \
props.cmo fileinfo.cmo os.cmo lock.cmo clroot.cmo common.cmo \
tree.cmo checksum.cmo transfer.cmo xferhint.cmo \
remote.cmo external.cmo negotiate.cmo globals.cmo fswatchold.cmo \
fpcache.cmo update.cmo copy.cmo stasher.cmo \
fpcache.cmo update.cmo moves.cmo copy.cmo stasher.cmo \
files.cmo sortri.cmo recon.cmo transport.cmo \
strings.cmo uicommon.cmo uitext.cmo test.cmo \
main.cmo
Expand Down
76 changes: 65 additions & 11 deletions src/common.ml
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,11 @@ type prevState251 =
Previous of Fileinfo.typ * Props.t251 * Os.fullfingerprint * Osx.ressStamp
| New

type prevState =
type prevState1 =
Previous of Fileinfo.typ * Props.t * Os.fullfingerprint * Osx.ressStamp
| New

let mprevState = Umarshal.(sum2
let mprevState1 = Umarshal.(sum2
(prod4 Fileinfo.mtyp Props.m Os.mfullfingerprint Osx.mressStamp id id)
unit
(function
Expand All @@ -91,6 +91,53 @@ let mprevState = Umarshal.(sum2
| I21 (a, b, c, d) -> Previous (a, b, c, d)
| I22 () -> New))

type prevState =
| PrevDir of Props.t
| PrevFile of Props.t * Os.fullfingerprint * Fileinfo.stamp * Osx.ressStamp
| PrevSymlink
| New

let mprevState2 = Umarshal.(sum4
Props.m
(prod4 Props.m Os.mfullfingerprint Fileinfo.mstamp Osx.mressStamp id id)
unit
unit
(function
| PrevDir a -> I41 a
| PrevFile (a, b, c, d) -> I42 (a, b, c, d)
| PrevSymlink -> I43 ()
| New -> I44 ())
(function
| I41 a -> PrevDir a
| I42 (a, b, c, d) -> PrevFile (a, b, c, d)
| I43 () -> PrevSymlink
| I44 () -> New))

let prev_to_old = function
| PrevDir desc ->
Previous (`DIRECTORY, desc, Os.fullfingerprint_dummy, Osx.ressDummy)
| PrevFile (desc, fp, _, ress) ->
Previous (`FILE, desc, fp, ress)
| PrevSymlink ->
Previous (`SYMLINK, Props.dummy, Os.fullfingerprint_dummy, Osx.ressDummy)
| New ->
New

let prev_of_old = function
| Previous (`DIRECTORY, desc, _, _) -> PrevDir desc
| Previous (`FILE, desc, fp, ress) -> PrevFile (desc, fp, NoStamp, ress)
| Previous (`SYMLINK, _, _, _) -> PrevSymlink
| Previous (`ABSENT, _, _, _)
| New -> New

let featPrevState2 = Features.register "prevState2" None

let newPrevStateType () = Features.enabled featPrevState2

let mprevState = Umarshal.(switch newPrevStateType
mprevState2 id id
mprevState1 prev_to_old prev_of_old)

(* IMPORTANT!
This is the 2.51-compatible version of type [Common.contentschange]. It
must always remain exactly the same as the type [Common.contentschange]
Expand Down Expand Up @@ -207,16 +254,16 @@ let mupdateContent, mupdateItem =
(* Compatibility conversion functions *)

let prev_to_compat251 (prev : prevState) : prevState251 =
match prev with
match prev_to_old prev with
| Previous (typ, props, fp, ress) ->
Previous (typ, Props.to_compat251 props, fp, ress)
| New -> New

let prev_of_compat251 (prev : prevState251) : prevState =
match prev with
prev_of_old (match prev with
| Previous (typ, props, fp, ress) ->
Previous (typ, Props.of_compat251 props, fp, ress)
| New -> New
| New -> New)

let change_to_compat251 (c : contentschange) : contentschange251 =
match c with
Expand Down Expand Up @@ -273,9 +320,11 @@ type status =
| `Modified
| `PropsChanged
| `Created
| `MovedOut of Path.t * replicaContent * replicaContent
| `MovedIn of Path.t * replicaContent * replicaContent
| `Unchanged ]

type replicaContent =
and replicaContent =
{ typ : Fileinfo.typ;
status : status;
desc : Props.t; (* Properties (for the UI) *)
Expand Down Expand Up @@ -335,7 +384,12 @@ let rcLength rc rc' =
if riAction rc rc' = `SetProps then
Uutil.Filesize.zero
else
snd rc.size
match rc'.status with
| `MovedIn _ ->
(* A move/rename will be reverted, count its size too *)
Uutil.Filesize.add (snd rc.size) (snd rc'.size)
| _ ->
snd rc.size

let riLength ri =
match ri.replicas with
Expand All @@ -355,17 +409,17 @@ let riLength ri =
let fileInfos ui1 ui2 =
match ui1, ui2 with
(Updates (File (desc1, ContentsUpdated (fp1, _, ress1)),
Previous (`FILE, desc2, fp2, ress2)),
PrevFile (desc2, fp2, _, ress2)),
NoUpdates)
| (Updates (File (desc1, ContentsUpdated (fp1, _, ress1)),
Previous (`FILE, desc2, fp2, ress2)),
PrevFile (desc2, fp2, _, ress2)),
Updates (File (_, ContentsSame), _))
| (NoUpdates,
Updates (File (desc2, ContentsUpdated (fp2, _, ress2)),
Previous (`FILE, desc1, fp1, ress1)))
PrevFile (desc1, fp1, _, ress1)))
| (Updates (File (_, ContentsSame), _),
Updates (File (desc2, ContentsUpdated (fp2, _, ress2)),
Previous (`FILE, desc1, fp1, ress1)))
PrevFile (desc1, fp1, _, ress1)))
| (Updates (File (desc1, ContentsUpdated (fp1, _, ress1)), _),
Updates (File (desc2, ContentsUpdated (fp2, _, ress2)), _)) ->
(desc1, fp1, ress1, desc2, fp2, ress2)
Expand Down
72 changes: 70 additions & 2 deletions src/common.mli
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,9 @@ and updateContent251 =
of string (* - link text *)

type prevState =
Previous of Fileinfo.typ * Props.t * Os.fullfingerprint * Osx.ressStamp
| PrevDir of Props.t
| PrevFile of Props.t * Os.fullfingerprint * Fileinfo.stamp * Osx.ressStamp
| PrevSymlink
| New

type contentschange =
Expand Down Expand Up @@ -118,15 +120,81 @@ val uc_of_compat251 : updateContent251 -> updateContent
(* COMMON TYPES SHARED BY RECONCILER AND TRANSPORT AGENT *)
(*****************************************************************************)

(* `MovedOut is set as the status on the old path. In this case, the new path
will not have a separate difference record at all (the corresponding
replicaContent records for both replicas are embedded in the `MovedOut
status). Status of the new path is guaranteed to be not conflicting.
`MovedOut is a combination of `Deleted on the old path and `Created on the
new path. The virtual status equivalent for both paths combined is
`Unchanged or `PropsChanged, meaning that except for the path change (and
potentially the props), the file/dir contents have not changed.

(in the illustrations below, the boxes with double lines represent
the two replicaContents of the one difference record that is
visible to the user and will be propagated)

REPLICA A REPLICA B

on path n'
/ +--------------+ +-----------------+
on path n | | p = `Created | | q = `Unchanged |
+======================+ | +--------------+ +-----------------+
| | /
| `MovedOut (n', p, q) | < on path n
| | \ +--------------+ +=================+
+======================+ | | `Deleted | | anything except |
| +--------------+ | `Deleted |
\ +=================+


If `MovedOut can not be set (for example, there is a conflict on the new
path) then `MovedIn may be set instead. `MovedOut and `MovedIn are never
used together on a pair of paths.
`MovedIn is set as the status of the new path. In this case, the old path
will not have a separate difference record at all (the corresponding
replicaContent records for both replicas are embedded in the `MovedIn
status). Status of the old path is guaranteed to be not conflicting.
`MovedIn is a combination of `Created on the new path and `Deleted on the
old path. The virtual status equivalent for both paths combined is
`Unchanged or `PropsChanged, meaning tat except for the path change (and
potentially the props), the file/dir contents have not changed.

REPLICA A REPLICA B

on path n
/ +--------------+ +================+
on path n | | `Created | | anything |
+======================+ | +--------------+ +================+
| | /
| `MovedIn (n', p, q) | <
| | \ on path n'
+======================+ | +--------------+ +----------------+
| | p = `Deleted | | q = `Unchanged |
\ +--------------+ +----------------+

(Usually the status of replica b on path n will not be `Unchanged, because
then `MovedOut will be used instead. It can be `Unchanged if typ is not
`ABSENT. In other words, `Create in replica b is overwriting something.)

Note that even though path for only one replica is recorded in `MovedOut/
`MovedIn, it will not cause trouble when in case insensitive mode on a
case sensitive filesystem (commit 005a53075b998dba27eeff74a1fc8f9d73558fb8
for details). Correct path translation is done by [Update.translatePath]. *)
type status =
[ `Deleted
| `Modified
| `PropsChanged
| `Created
| `MovedOut of Path.t (* new path *)
* replicaContent (* rc of new path, this replica *)
* replicaContent (* rc of new path, other replica *)
| `MovedIn of Path.t (* old path *)
* replicaContent (* rc of old path, this replica *)
* replicaContent (* rc of old path, other replica *)
| `Unchanged ]

(* Variable name prefix: "rc" *)
type replicaContent =
and replicaContent =
{ typ : Fileinfo.typ;
status : status;
desc : Props.t; (* Properties (for the UI) *)
Expand Down
Loading