Commit Graph

8 Commits

Author SHA1 Message Date
Jun Wu
cdc9accca5 lfs: do not store mercurial filelog metadata in lfs blobs
Summary:
Per discussion with @davidsp, it's better for LFS to not store Mercurial
filelog metadata, which is currently used to store rename information. That has
many advantages:

  - Large blobs could be reused across renames
  - No need to special handle files starting with `\1\n`
  - P4 LFS server implementation is much easier
  - remotefilelog LFS support is easier and cleaner

That said, the rename information is stored as lfs metadata using the
non-standard `x-hg-copy`, `x-hg-copyrev` keys. So they still exist and are
functional.

The disadvantage is that rename gets no longer hashed, which is probably fine.


Test Plan: Added a test

Reviewers: davidsp, #sourcecontrol, rmcelroy

Reviewed By: rmcelroy

Subscribers: jsgf, rmcelroy, stash, mjpieters, davidsp

Differential Revision: https://phabricator.intern.facebook.com/D4849764

Signature: t1:4849764:1491580506:1d80ad476b9cbd6773843cb52aee6745f478a0b0
2017-04-07 18:29:35 -07:00
Jun Wu
de84869150 lfs: cleanup user-facing messages
Summary:
The diff cleans up messages shown to the user. It makes verbose messages
gated by `if ui.verbose`, and simplifies some words.

The resulting user experience is, when there is no large file involved, lfs
shows nothing. When there are largefiles being downloaded or uploaded, show
progress bar if it takes long. The progress bar is the only user visible
output from lfs by default.

Test Plan: `rt test-lfs.t`

Reviewers: #mercurial, simonfar

Reviewed By: simonfar

Subscribers: simonfar, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4813297

Signature: t1:4813297:1491213089:c9ec363eb65df4c85282c90b230da3321b93b5e0
2017-04-05 15:58:56 -07:00
Jun Wu
f0b6b81970 lfs: simplify remote blobstore.logic
Summary:
Previously, the remote blobstore could be either `git-lfs` or `dummy`. The
application code does not really care, it only wants a "remote" blobstore.

This diff adds a factory method and makes `git-lfs` and `dummy` stores private.

The `@staticmethod get(vfs)` interface is also removed as it's duplicated and
unnecessary - as long as mercurial calls `reposetup`, the blob store objects
are set, and they cannot be missing.

The error message about an unsupported store is also changed to be consistent
with mercurial style. A test was added to test the error.

Test Plan: `rt test-lfs.t`

Reviewers: #mercurial, simonfar

Reviewed By: simonfar

Subscribers: simonfar, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4813023

Signature: t1:4813023:1491382755:b9d8ec6518141d0ba8263e16c53f430ce80c39f0
2017-04-05 15:48:10 -07:00
Jun Wu
6d19f83af9 lfs: add a "bypass" config option
Summary:
The bypass option limits lfs's functionality to only skip hash checks. It is
intended to be used server-side, to make it more predictable - the server
never interacts with the lfs blob service.

Test Plan: Added a test case

Reviewers: #mercurial, rmcelroy

Reviewed By: rmcelroy

Subscribers: rmcelroy, simonfar, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4811828

Signature: t1:4811828:1491301114:c2e3c4200ce4cc84b9c5872a8b9a040176bb002a
2017-04-04 16:08:36 -07:00
Jun Wu
abb7a83354 lfs: make tests stronger
Summary:
This diff makes the `test-lfs.t` much more stronger. It reveals a lot of core
hg issues in this area. I'll send patches to fix them all.

Regarding on lfs, there are some changes:

  - An existence check in its push hook was added. Otherwise pushing a revision
    with rename will cause crash.
  - The "read" processor is responsible for downloading blobs, and translate
    raw revision to lfs text. It should always return lfs text. But it may
    return raw revision text on error currently. That error handler was
    removed to avoid further damage.


Test Plan: Added new test cases. I also added `hg verify` to sanity check things are good.

Reviewers: #mercurial, rmcelroy

Reviewed By: rmcelroy

Subscribers: rmcelroy, mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4792641

Signature: t1:4792641:1490868946:8f09c84dc9ebda3889d6a1ea04c49a06acbf38a3
2017-03-30 16:41:46 -07:00
Jun Wu
eeeb0210ae lfs: allow blobstore to be outside the repo
Summary:
Previously `lfs.blobstore` must be a relative path. If an absolute
blobstore path is set, it will traceback because vfs audit fails:

```
  File "hg/mercurial/revlog.py", line 1356, in _processflags
    text, vhash = writetransform(self, text)
  File "fb-hgext/hgext3rd/lfs/wrapper.py", line 69, in writetostore
    blobstore.local.get(self.opener).write(storeid, chunk)
  File "fb-hgext/hgext3rd/lfs/blobstore.py", line 41, in write
    fp = self._opener(self.filename(storeid), 'w+', atomictemp=True)
  File "hg/mercurial/vfs.py", line 344, in __call__
    self.audit(path)
  File "hg/mercurial/pathutil.py", line 64, in __call__
    raise error.Abort(_("path contains illegal component: %s") % path)
  Abort: path contains illegal component: /home/quark/lfslocalblobstore/d7/dbc611df1fe7dfacfe267a2bfd32ba8fc27ad16aa72af7e6c553a120b92f18
```

That was because the code was using `repo.vfs`. This diff adds a new `lfsvfs`
to avoid the issue. The `lfsvfs` also did the correct filename check (the
old `re.match` check will not match the whole string), so `blobstore.local`
could be simplified a lot.


Test Plan:
A new test case was added to make sure absolute blobstore path works. I also
did some cleanups for the test file to de-dup hgrc, and avoid writing files
outside `$TESTTMP`.

Reviewers: #mercurial, rmcelroy

Reviewed By: rmcelroy

Subscribers: rmcelroy, mjpieters, remi

Differential Revision: https://phabricator.intern.facebook.com/D4785084

Signature: t1:4785084:1490693306:b42eef8e53af107897c2e1bc2984c090bdd2e465
2017-03-28 15:41:59 -07:00
Jun Wu
c3fd525bf8 lfs: fix tests
Summary:
A bunch of modifications to get the test pass with the new lfs code.

- Move `lfs` to `hgext3rd`. The code was supposed for hg-core. For now, we do
  them in fb-hgext to speed up the process
- Remove the windows test, which is not supported by `run-tests.py` and is
  duplicated with `test-lfs.t`.
- Do import `mercurial.i18n._` correctly.
- Change some i18n logic a bit so it's more translator-friendly.
- Change `revlog.RevlogError` to `error.RevlogError`.
- Avoid direct symbol import of `mercurial.util.bytecount`, which will fail the
  upstream importchecker test.
- Fix various lint issues like lines being too long etc.
- Document lfs config options.

Test Plan: `arc unit`

Reviewers: #sourcecontrol, rmcelroy

Reviewed By: rmcelroy

Subscribers: mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4772216

Signature: t1:4772216:1490401458:1ad3c18ab80e1d31085d0b6b4c630e62a7dc7930
2017-03-24 19:01:42 -07:00
Remi Chaintron
dcdf35ff63 lfs: new extension
Add the lfs extension and its tests.

----
quark: This imports @remi's lfs work for core hg. See D3980880, D4770352,
and D4771511.
2017-03-24 19:00:30 -07:00