Summary: Doesn't seem to make sense to keep two of them.
Test Plan: meh.
Reviewers: #mercurial, durham
Reviewed By: durham
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3789029
Signature: t1:3789029:1472521395:db958a5f050319dfa7c608cc448e2d38acbb8c0c
Summary: Noop as far as the actual result codes are thus far.
Test Plan: compiles, passes rigged test-remotefilelog-datapack.py
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3787517
Tasks: 12932864
Signature: t1:3787517:1472517234:f1a9bac77588e9c3d1d64e5923efc23e5e2d0066
Summary: This allows us to use a typedef'd type for our size fields, and avoid the sketchy int declarations.
Test Plan: pass test-remotefilelog-datapack.py (with hacks)
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: durham, mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3786882
Tasks: 12932864
Signature: t1:3786882:1472515261:91d86f5bda09f4a0beea729eb1f871e99932548b
Summary:
We were only checking the version code in the index file.
Depends on D3786770
Test Plan: now we can pass a hacked up version of test-remotefilelog-datapack.py
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3786805
Tasks: 12932864
Signature: t1:3786805:1472515742:5c411b158f39c6153a3746f734037e17ba274e11
Summary:
1. handle has a status field which indicates how successful it was in opening.
2. in the python abstraction layer, we inspect the status field. we check the error code before we free the block and return control.
Test Plan: a version of test-remotefilelog-datapack.py hacked up to use cdatapack passes (except for version code mismatches in the data file; addressing that in separate diff)
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3786770
Tasks: 12932864
Signature: t1:3786770:1472514907:e907f2c8d75f1ce0a65aff2b2c97541bb5e27801
Summary: Not sure how this snuck in.
Test Plan: make local
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3786739
Signature: t1:3786739:1472514343:a2df360e3aa2c7cb38573031a193db16d30dc747
Summary:
1. return `pack_chain_code_t` structs since they're small. add an error code.
2. set the code for returning the data.
3. `get_delta_chain` examines the return code and passes it back up.
Depends on D3786447
Test Plan:
make local.
tried cdatapack_dump and cdatapack_get without anything blowing up.
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3786546
Tasks: 12932864
Signature: t1:3786546:1472515566:94d4df0d4a2f99d990bab5134df9b8ea934e0d7f
Summary:
1. `delta_chain_t` is a pretty small structure, so I'm modifying the code to return this struct on the stack. This removes the allocation and the error checking needs for it.
2. add a `code` field to indicate the status.
Test Plan:
make local.
unfortunately, we don't have any good tooling to introduce memory allocation failures, so this is mostly visual.
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: durham, mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3786447
Tasks: 12932864
Signature: t1:3786447:1472514697:c09956480b8a5456ffc3f2f955d6a2d2e816769f
remotefilelogserver.py already overrides _walkstreamfiles and makes
sure to include locally committed content, so the override in
shallowstore doesn't seem necessary. It also breaks treemanifests,
since it doesn't include the meta/ directory.
Summary:
It turns out PyArg_ParseTuple doesn't create new refs, so if we put any
arguments in PythonObj wrappers, they will be incorrectly decremented during
destruction. Let's increment the ref before putting any args in PythonObj.
Test Plan:
N/A, I just eyeballed this. I did look at all other uses of
PyArg_ParseTuple to verify no other cases had this issue.
Reviewers: #fastmanifest
Differential Revision: https://phabricator.intern.facebook.com/D3766116
Summary:
With this implemented, it's now possible to run 'hg diff' and have it use
exclusively our treemanifest implementation.
Test Plan:
Ran 'hg diff -r master^ -r master' with some future patches and
verified the result was correct (it originally failed entirely because
__contains__ was not implemented).
Reviewers: #fastmanifest
Differential Revision: https://phabricator.intern.facebook.com/D3766114
Summary: The filesnotin function is necessary for hg diff, so let's implement it.
Test Plan:
Ran 'hg diff -r master^ -r master' with some future patches and
verified the result was correct (it originally failed entirely because
filesnotin was not implemented).
Reviewers: #fastmanifest
Differential Revision: https://phabricator.intern.facebook.com/D3766108
Summary:
Turns out that manifest.diff accepts kwargs, so we need to support that
signature. We'll come back and actually implement clean=True support later.
Test Plan:
Ran 'hg diff -r master^ -r master' with some future patches and
verified the result was correct (it originally failed entirely because diff had
the wrong signature).
Reviewers: #fastmanifest, ttung
Reviewed By: ttung
Subscribers: ttung
Differential Revision: https://phabricator.intern.facebook.com/D3766104
Signature: t1:3766104:1472156875:0ce2a16286b1e4fd6b0f2260249e3318270d8814
Summary:
This implements the matches function of treemanifest. These are necessary for hg
diff to work.
Test Plan:
Ran 'hg diff -r master^ -r master' with some future patches and
verified the result was correct (it originally failed entirely because matches
was not implemented).
Reviewers: #fastmanifest
Differential Revision: https://phabricator.intern.facebook.com/D3766101
Summary:
File iteration is becoming a common pattern, so let's move py_fileiter creation to a
common area. This also move the fileiter initialization logic into the
constructor so it's not duplicated anymore either.
Test Plan:
Ran 'hg files' and 'hg manifest' with future patches that allowed hg
to use the treemanifest and verified they still worked.
Reviewers: #fastmanifest
Differential Revision: https://phabricator.intern.facebook.com/D3775597
Summary:
This moves the file iterator to a separate function so that it may be reused in
other areas. It also further separates the iterator from the python specific
logic.
Two notable changes to the logic:
1. It now returns the node and flag as well as the path.
2. Instead of copying iter->path straight into a python string for returning,
the caller provides a path, node, and flag character array as arguments and the
result is copied into that array. This results in a little more string copying
during iteration, but since the character array is stack allocated it the actual
performance impact is neglible.
Test Plan:
Ran the perf test, verified fulliter perf didn't change (2.2s before
and after). Also used pdb to verify the results were valid and expected paths.
Reviewers: #fastmanifest
Differential Revision: https://phabricator.intern.facebook.com/D3760071
Summary:
This implements the flags api. After this, hg manifest --debug can work with
treemanifest.
Test Plan:
In a future patch, I was able to run 'hg manifests' on a commit and
have it return the manifest contents by reading the treemanifest.
Reviewers: #fastmanifest
Differential Revision: https://phabricator.intern.facebook.com/D3755343
Summary: This implements treemanifest.__getitem__ in the C api.
Test Plan:
In a future patch, I was able to run 'hg manifests' on a commit and
have it return the manifest contents by reading the treemanifest.
Reviewers: #fastmanifest, ttung
Reviewed By: ttung
Differential Revision: https://phabricator.intern.facebook.com/D3755338
Signature: t1:3755338:1471930178:933e1d56f98c6e615624e2501a01a6ae925c2f8d
Summary: This removes the hard coded revs and paths used in the perf test script.
Test Plan: Ran the perf test
Reviewers: #fastmanifest, ttung
Reviewed By: ttung
Differential Revision: https://phabricator.intern.facebook.com/D3755335
Signature: t1:3755335:1471930070:e90a242a8f73d2fc32ea68bc99cae88b00af14c9
Summary:
Adds the initial extension that sets up the ctreemanifest. It currently relies
on the fastmanifest extension to hook into all the manifest APIs to construct
ctreemanifests.
Test Plan:
In a future patch, I was able to run 'hg manifests' on a commit and
have it return the manifest contents by reading the treemanifest.
Reviewers: #fastmanifest, ttung
Reviewed By: ttung
Subscribers: ttung
Differential Revision: https://phabricator.intern.facebook.com/D3755327
Signature: t1:3755327:1472114482:0c5862cba68ed4db643d28c2fae01f33f5352970
Summary: After a recent refactoring these are now unused.
Test Plan: make local
Reviewers: #fastmanifest, ttung
Reviewed By: ttung
Differential Revision: https://phabricator.intern.facebook.com/D3756194
Signature: t1:3756194:1471929972:bdace32b744bfaaf03228f13de0fb6ffec240dab
Summary:
If we don't include Python.hg before some of the standard library (like list),
it causes a build error on some machines ('error: "_POSIX_C_SOURCE" redefined').
So let's move all Python.h to the front.
Also add a missing stdexcept include that's needed for the use of
std::logic_error
Test Plan: make local on centos6
Reviewers: #fastmanifest, ttung
Reviewed By: ttung
Differential Revision: https://phabricator.intern.facebook.com/D3756191
Signature: t1:3756191:1471929906:8afc09ee74b5ab5512fc87fadbed311cf15cb768
Summary: `PyTuple_Pack` does not steal the reference, so we need to decref.
Test Plan: valgrind is my friend.
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: durham, lcharignon, mitrandir
Differential Revision: https://phabricator.intern.facebook.com/D3698714
Signature: t1:3698714:1472175554:d5f1cde0ba1571357d746a45ab9f4dbd0d0ccb6e
Summary: No longer needed.
Test Plan: make local && clion build && make unit tests.
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3780786
Signature: t1:3780786:1472255290:cdf238f772234f8a2e15d69dca6489b6b977511d
Summary:
buffer.h gained the ability to deal with non-char-sized buffers when I built cdatapack. We need to update the callers in ctreemanifest to be aware of this. Most of this is done with macro magic.
Some functionality was dropped from cdatapack's buffer.h (macro definitions to deal with paths). Those are moved to path_buffer.h
Test Plan:
make local && clion build.
pass cfastmanifest unit tests.
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3780767
Signature: t1:3780767:1472255278:40a19edfd171df5804e9cdfa4444d5c6386f00e8
Summary: One giant CMake file. Only I use CLion, so i don't think this affects anyone else.
Test Plan: make in clion. run cfastmanifest unit tests.
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: durham, mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3780573
Signature: t1:3780573:1472250750:ece55fb66a656624335c8432ab3c8334699ea36e
Summary: Ubuntu 14 apparently requires `_BSD_SOURCE` to be defined for nteoh64 to exist. Also, MAP_FILE is not part of POSIX mmap spec, so is not always available. Fortunately, on the platforms we support (linux and osx), it appears to be implied by default.
Test Plan: make local
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3780057
Signature: t1:3780057:1472246090:100c38f613e67dc1c56ab6ef55a3cc7c4f703497
Summary: When a ManifestEntry is destroyed, we reap the heap-allocated resources it holds. However, this (erroneously) happens when we create a ManifestEntry and add it to the list of children. What we really need is a move constructor from C++11. The workaround is to initialize a blank ManifestEntry, add it to the proper location, and then populate it afterwards.
Test Plan: addChild (in a later diff) no longer tramples over memory it no longer owns.
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: mitrandir
Differential Revision: https://phabricator.intern.facebook.com/D3772915
Signature: t1:3772915:1472174144:a4baac5cb5f6e01a38042c5c6cd92570c8f8e100
Summary: This gives us a bit of flexibility when we call this method, as references cannot be reassigned.
Test Plan: compiles
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: mitrandir
Differential Revision: https://phabricator.intern.facebook.com/D3772844
Signature: t1:3772844:1472174021:565d7f25a27c2fd78ce14b86b5bc499c74d47976
Test Plan: haven't used this yet, but it makes a whole lot of sense.
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: mitrandir
Differential Revision: https://phabricator.intern.facebook.com/D3770366
Signature: t1:3770366:1472173969:b83e10e27a63b863a3566d2df7cc861417b8208e
Summary:
1. `children()`, which returns the number of children the manifest has.
2. `removeChild()`, which removes the entry that the iterator references.
Test Plan: used in later diffs
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: mitrandir
Differential Revision: https://phabricator.intern.facebook.com/D3770363
Signature: t1:3770363:1472173823:737043b4613a669b402dcfd7921ac55278c3ece9
Summary: The strings are not necessarily null-terminated and length needs to be considered.
Test Plan: used in later diff to find a path.
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: durham, mitrandir
Differential Revision: https://phabricator.intern.facebook.com/D3770358
Signature: t1:3770358:1472173786:9a2f681c90476aafd481c301ff65ac8b199214ec
Summary:
appendbinfromhex appends the binary representation of a 40-byte hex string onto a std::string. This allows us to reuse a std::string rather than to allocate a new one every time.
Also:
1. converted binfromhex to use this method.
2. updated the docblocks to actually reflect reality.
Test Plan: `make local && cd ~/work/fbsource && PYTHONPATH=~/work/mercurial/facebook-hg-rpms/remotefilelog:~/work/mercurial/facebook-hg-rpms/fb-hgext/:~/work/mercurial/facebook-hg-rpms/remotenames/:~/work/mercurial/facebook-hg-rpms/lz4revlog/ /opt/local/bin/python2.7 ~/work/mercurial/facebook-hg-rpms/hg-crew/hg --config extensions.perftest=~/work/mercurial/facebook-hg-rpms/remotefilelog/tests/perftest.py testtree --kind flat,ctree,fast --test fulliter,diff,find --build "master~5::master"`
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: mitrandir
Differential Revision: https://phabricator.intern.facebook.com/D3770048
Signature: t1:3770048:1472105520:eac79a42360ebfa258519346b68fc4541c2dbb7c
Summary: In most cases, this is pretty straightforward. The only unusual case is `_treemanifest_find`, which does the actual resolution of the root manifest (unlike diff and iter).
Test Plan: make local
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: durham, mitrandir
Differential Revision: https://phabricator.intern.facebook.com/D3763324
Signature: t1:3763324:1472173572:3dcda9b318ad818d2f51e8c3472c7770739faafe
Summary: This more accurately describes its purpose.
Test Plan: simple refactor, so thus just make local
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: durham, mitrandir
Differential Revision: https://phabricator.intern.facebook.com/D3763323
Signature: t1:3763323:1472104318:b48246777db0527c7022066438c211f54c88703e
Summary: It makes a destructor possible.
Test Plan: make local
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: mitrandir
Differential Revision: https://phabricator.intern.facebook.com/D3763322
Signature: t1:3763322:1472104242:e76ef1943c2082ddecff872c6472de4706505922
Summary:
I've seen the failures like https://phabricator.intern.facebook.com/P56599278
which were the result of "enforce_root_files" being set in our global watchman
config.
It appears that the global configs are being picked up only on server start.
Killing server during the test won't help because watchman is automagically
restarted in our dev environment.
The way to do it properly is to mimic
https://github.com/facebook/watchman/blob/master/tests/integration/WatchmanInstance.py
until we do that I've just put a .watchmanconfig to fix the test.
Test Plan: tests is passing now
Reviewers: #mercurial, ttung, durham, wez, zamsden
Reviewed By: wez, zamsden
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3772968
Signature: t1:3772968:1472158987:0e08c5e9f862ba3d74d016d051b852512d06e399
Summary: I ran tests with --noskip and found one more failure
Test Plan: test passing now
Reviewers: #mercurial, ttung
Reviewed By: ttung
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3772978
Signature: t1:3772978:1472155449:1c5d8e16bd51d2b06f15432dd47c6fba48da3abd
You can jump to a patch by pressing its numeric index, but this obviously only works with up to 10 patches. Moreover, with a large number of patches the feature is dangerous, since an accidental number press will lose your place. While we could do something fancy and prompt for multi-digit input in the many-patch case, it doesn't seem worth the complexity, and simply disabling 'goto' seems good enough.
