Patchwork [4,of,5] lfs: add a repo requirement for this extension when converting to lfs

login
register
mail settings
Submitter Matt Harbison
Date Nov. 26, 2017, 10:45 p.m.
Message ID <1dfe0c6abea0374834bb.1511736304@Envy>
Download mbox | patch
Permalink /patch/25760/
State Accepted
Headers show

Comments

Matt Harbison - Nov. 26, 2017, 10:45 p.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1511408330 18000
#      Wed Nov 22 22:38:50 2017 -0500
# Node ID 1dfe0c6abea0374834bbb100b5402361e9d7954a
# Parent  652a95c77a386d6cd99480a7063d791b000229b3
lfs: add a repo requirement for this extension when converting to lfs

This covers both the vanilla repo -> lfs repo and largefiles -> lfs conversions.
The largefiles extension adds the requirement directly, because it has a
dedicated command to convert.  Using the convert extension is better, because it
supports more features.

I'd like ideas about how to ensure that converting away from lfs works on all
files.  (See comments in test-lfs.t)

Patch

diff --git a/hgext/lfs/__init__.py b/hgext/lfs/__init__.py
--- a/hgext/lfs/__init__.py
+++ b/hgext/lfs/__init__.py
@@ -128,6 +128,9 @@ 
     wrapfilelog(filelog.filelog)
 
     wrapfunction = extensions.wrapfunction
+
+    wrapfunction(scmutil, 'wrapconvertsink', wrapper.convertsink)
+
     wrapfunction(changegroup,
                  'supportedoutgoingversions',
                  wrapper.supportedoutgoingversions)
diff --git a/hgext/lfs/wrapper.py b/hgext/lfs/wrapper.py
--- a/hgext/lfs/wrapper.py
+++ b/hgext/lfs/wrapper.py
@@ -165,6 +165,31 @@ 
 def filectxislfs(self):
     return _islfs(self.filelog(), self.filenode())
 
+def convertsink(orig, sink):
+    sink = orig(sink)
+    if sink.repotype == 'hg':
+        class lfssink(sink.__class__):
+            def putcommit(self, files, copies, parents, commit, source, revmap,
+                          full, cleanp2):
+                pc = super(lfssink, self).putcommit
+                node = pc(files, copies, parents, commit, source, revmap, full,
+                          cleanp2)
+
+                if 'lfs' not in self.repo.requirements:
+                    ctx = self.repo[node]
+
+                    # The file list may contain removed files, so check for
+                    # membership before assuming it is in the context.
+                    if any(f in ctx and ctx[f].islfs() for f, n in files):
+                        self.repo.requirements.add('lfs')
+                        self.repo._writerequirements()
+
+                return node
+
+        sink.__class__ = lfssink
+
+    return sink
+
 def vfsinit(orig, self, othervfs):
     orig(self, othervfs)
     # copy lfs related options
diff --git a/tests/test-lfs-largefiles.t b/tests/test-lfs-largefiles.t
--- a/tests/test-lfs-largefiles.t
+++ b/tests/test-lfs-largefiles.t
@@ -290,14 +290,16 @@ 
   0 remove large_by_size.bin
   $ cd nolargefiles
 
-BUG: This should have a requires line for 'lfs'
+The requirement is added to the destination repo
 
   $ cat .hg/requires
   dotencode
   fncache
   generaldelta
+  lfs
   revlogv1
   store
+
   $ hg log -r 'all()' -G -T '{rev} {join(lfs_files, ", ")} ({desc})\n'
   o  8  (remove large_by_size.bin)
   |
diff --git a/tests/test-lfs.t b/tests/test-lfs.t
--- a/tests/test-lfs.t
+++ b/tests/test-lfs.t
@@ -557,3 +557,82 @@ 
   repo: repo8
   repo: repo9
   repo: repo10
+
+lfs -> normal -> lfs round trip conversions are possible.  The threshold for the
+lfs destination is specified here because it was originally listed in the local
+.hgrc, and the global one is too high to trigger lfs usage.  For lfs -> normal,
+there's no 'lfs' destination repo requirement.  For normal -> lfs, there is.
+
+XXX: There's not a great way to ensure that the conversion to normal files
+actually converts _everything_ to normal.  The extension needs to be loaded for
+the source, but there's no way to disable it for the destination.  The best that
+can be done is to raise the threshold so that lfs isn't used on the destination.
+It doesn't like using '!' to unset the value on the command line.
+
+  $ hg --config extensions.convert= --config lfs.threshold=1000M \
+  >    convert repo8 convert_normal
+  initializing destination convert_normal repository
+  scanning source...
+  sorting...
+  converting...
+  2 a
+  1 b
+  0 meta
+  $ grep 'lfs' convert_normal/.hg/requires
+  [1]
+  $ hg --cwd convert_normal debugdata a1 0
+  THIS-IS-LFS-BECAUSE-10-BYTES
+
+  $ hg --config extensions.convert= --config lfs.threshold=10B \
+  >    convert convert_normal convert_lfs
+  initializing destination convert_lfs repository
+  scanning source...
+  sorting...
+  converting...
+  2 a
+  1 b
+  0 meta
+  $ hg --cwd convert_lfs debugdata a1 0
+  version https://git-lfs.github.com/spec/v1
+  oid sha256:5bb8341bee63b3649f222b2215bde37322bea075a30575aa685d8f8d21c77024
+  size 29
+  x-is-binary 0
+  $ grep 'lfs' convert_lfs/.hg/requires
+  lfs
+
+This convert is trickier, because it contains deleted files (via `hg mv`)
+
+  $ hg --config extensions.convert= --config lfs.threshold=1000M \
+  >    convert repo3 convert_normal2
+  initializing destination convert_normal2 repository
+  scanning source...
+  sorting...
+  converting...
+  4 commit with lfs content
+  3 renames
+  2 large to small, small to large
+  1 random modifications
+  0 switch large and small again
+  $ grep 'lfs' convert_normal2/.hg/requires
+  [1]
+  $ hg --cwd convert_normal2 debugdata large 0
+  LONGER-THAN-TEN-BYTES-WILL-TRIGGER-LFS
+
+  $ hg --config extensions.convert= --config lfs.threshold=10B \
+  >    convert convert_normal2 convert_lfs2
+  initializing destination convert_lfs2 repository
+  scanning source...
+  sorting...
+  converting...
+  4 commit with lfs content
+  3 renames
+  2 large to small, small to large
+  1 random modifications
+  0 switch large and small again
+  $ grep 'lfs' convert_lfs2/.hg/requires
+  lfs
+  $ hg --cwd convert_lfs2 debugdata large 0
+  version https://git-lfs.github.com/spec/v1
+  oid sha256:66100b384bf761271b407d79fc30cdd0554f3b2c5d944836e936d584b88ce88e
+  size 39
+  x-is-binary 0