In some situations the remotefilelog setup logic could be called, which will
wrap certain functions, and then later a call will happen to a repo that wasn't
remotefilelog which will run some remotefilelog code because of the wrapping.
Normally we take care of this by checking for the remotefilelog requirement. We
missed it in this one spot though.
Summary:
During chg pull or push over ssh, ssh is started by chgserver which does not
have a controlling tty. Therefore the ssh process won't be able to ask for
passwords interactively.
This is actually a hard issue because an unprivileged process without a ctty
cannot attach to a ctty of another process.
The discussion at upstream tends to make it clear it's part of limitations
of chg. Therefore if we decide to workaround it, it has to live outside core,
thus fb-hgext.
GUI ssh-askpass is actually a good and clean choice. See D3510178 and D3515604.
However, they are for OS X but not Linux.
This diff is a very hacky solution to make ssh-askpass works in terminal.
It starts a "tty server" providing tty I/O fds and set `SSH_ASKPASS` to use a
custom script talking to the "tty server".
Test Plan:
Run the new test. Start a sshd locally and try:
```
$ hg push ssh://root@localhost/tmp
pushing to ssh://root@localhost/tmp
root@localhost's password:
remote: Permission denied (publickey,password).
abort: no suitable response from remote hg!
$ chg push ssh://root@localhost/tmp
pushing to ssh://root@localhost/tmp
==== SSH Authenticating ====
root@localhost's password:
remote: Permission denied (publickey,password).
abort: no suitable response from remote hg!
```
Reviewers: #mercurial, ttung, mpm
Reviewed By: mpm
Subscribers: durham, mpm, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3577509
Tasks: 12029680
Signature: t1:3577509:1469467700:cd93565bd47e535bb4cb41fcdaa39e45dddfae28
Summary:
All of hg journal is now fully upstreamed to mercurial core and remotenames.
Remove the outdated copy here.
Test Plan: --
Reviewers: #mercurial, ttung
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3611123
Tasks: 10804988
Summary: This is more robust and keeps the style consistent throughout
Test Plan: ran tests against zsh and bash
Reviewers: #sourcecontrol, ttung, zamsden
Reviewed By: zamsden
Subscribers: zamsden, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3597346
Signature: t1:3597346:1469073711:de76132bd9c161242d8d8171da765ddb54a806a2
Summary:
The old scm-prompt uses this longer, less awesome name.
Let's be backwards compatible with this and all that jazz.
Test Plan: tests pass under bash and zsh
Reviewers: #sourcecontrol, ttung, quark
Reviewed By: quark
Subscribers: quark, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3597315
Signature: t1:3597315:1469153052:98600400bb3cabf561294fd62f7cc601c280a1bd
Summary:
Previously, aliases could screw up scm-prompt. We had seen earlier
issues with this (thus the --color=never), but this is a more generic and
robust solution, applied everywhere.
We can't use full paths because they differ on different hosts, so we still
rely on a reasonable $PATH, as we always have.
Test Plan: ran scm-prompt tests under bash and zsh
Reviewers: #sourcecontrol, ttung, zamsden
Reviewed By: zamsden
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3597300
Tasks: 12298139
Signature: t1:3597300:1469060969:e88be6bd79234167f039f2964117a738ffbb95f5
Summary:
Since statprof is not an extension, it needs to be treated like a normal python
module.
Test Plan:
```
~/local/fb-hgext> python setup.py build
~/local/fb-hgext> ls build/lib.linux-x86_64-2.6/
cfastmanifest.so hgext3rd/ sqldirstate/
fastmanifest/ phabricator/ statprof.py
```
Reviewers: ttung, #sourcecontrol, quark
Reviewed By: quark
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3607202
Signature: t1:3607202:1469216683:9aa08aee567425c239ddc5d1bb68fd892a8cf6a9
Summary: vfs.join(None) will return its base. doesn't matter if it's nested or whatever.
Test Plan:
cloned fbjava, then stripped a revision (with evolve off). then added a new revision, and ran `hg incoming`. this resulted in the same stacktrace!
then with the fix run `hg incoming` again. no crash this time.
Reviewers: #fastmanifest, simonfar
Reviewed By: simonfar
Subscribers: mitrandir, simonfar, mjpieters, lcharignon
Differential Revision: https://phabricator.intern.facebook.com/D3598348
Tasks: 12305684
Signature: t1:3598348:1469093045:440cf3d314589f46dd8083901b584d7cfd54a95a
Summary:
Let's use deltas between the subsequent samples instead of sample count
to count the time spent.
Rationale:
When the process is IO blocked the other thread doing the sampling can be waken
up much more often (GIL is not held) causing the profiler to collect much more
samples in that state.
Test Plan:
works in my sandbox
do we have any tests for statprof.py?
Reviewers: durham
Reviewed By: durham
Subscribers: quark, mjpieters, #mercurial
Differential Revision: https://phabricator.intern.facebook.com/D3551431
Signature: t1:3551431:1468543377:f6b86245c957dd59d0334dbb1898f6ad0bb8c617
Summary:
`import hgsubversion` can error out if demandimport is disabled and svn
bindings are not found. In that case, we should be able to continue and
just skip handling svn revisions.
Test Plan: Code review
Reviewers: #mercurial, ttung, rmcelroy
Reviewed By: rmcelroy
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3594810
Signature: t1:3594810:1469042572:3f9ab69f5503f6925f455769a78dac42a47087de
Summary: Sometimes opener.vfs is an _fncachevfs, which has a .vfs member, not a .base member. When you get one of these, go deeper.
Test Plan:
Run testrunner.py flib/intern/sandcastle/vcs/ without this change, see it traceback.
Rerun testrunner.py flib/intern/sandcastle/vcs/ with this change, see it fail in the same way as the current released version.
Reviewers: durham, lcharignon, ttung, rmcelroy
Reviewed By: rmcelroy
Subscribers: rmcelroy, mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D3593097
Tasks: 12305684
Signature: t1:3593097:1469033487:aec4c4a5de6f92a64f4730f03e9780e02f086819
Summary:
The `.py` tests are different from `.t` ones. They need special care about
`import`.
Also adds unlink `socketpath` to make sure the `.py` file runs directly
with python with additional effort to clean up the test dir.
Test Plan:
```
unset PYTHONPATH
run-tests.py test-patchpython.py
python2 test-patchpython.py
```
Reviewers: ttung, #mercurial, simonfar
Reviewed By: simonfar
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3579579
Signature: t1:3579579:1468848206:3711a4714080fbcc2d4360ded8316603be48fa25
In two place, we were checking if a revlog was an instance of
revlog.revlog and, I think, treating it as a
remotefilelog.remotefilelog otherwise. I noticed this when I created
another non-revlog.revlog revlog in narrowhg and remotefilelog thought
it was a remotefilelog.remotefilelog. Let's specifically check if it's
a remotefilelog.remotefilelog instead.
Summary:
From sqlite manual:
With synchronous OFF (0), SQLite continues without syncing as soon as it
has handed data off to the operating system. If the application running
SQLite crashes, the data will be safe, but the database might become
corrupted if the operating system crashes or the computer loses power
before that data has been written to the disk surface. On the other hand,
commits can be orders of magnitude faster with synchronous OFF.
I believe this is a proper tradeoff for dirstate as we can regenerate it easily if db becomes corrupted.
Test Plan: tests are passing
Reviewers: #mercurial, ttung, durham
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3559167
Summary: We can be more lazy - it's propertycache so it will populate itself when needed.
Test Plan: tests are passing
Reviewers: #mercurial, ttung, rmcelroy
Reviewed By: rmcelroy
Subscribers: rmcelroy, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3558232
Signature: t1:3558232:1468460942:e5e03743ec9a33cdb5da6d5f7541af28316f6c34
Summary:
The tests that are modifying the hgrc are blacklisted for
sqldirstate because they are effectively switching exitension off.
Test Plan: tests are passing
Reviewers: #mercurial, ttung, durham, rmcelroy
Reviewed By: rmcelroy
Subscribers: rmcelroy, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3559268
Signature: t1:3559268:1468453765:38140f2ab4c392dc2aeefec230cbd1d4bb1b7170
Summary:
It's a common mistake that our tests require foreign extensions (namely evolve
and remotenames) without checking them first.
This diff adds checks to catch these mistakes, adds missing checks, and unifies
our checking logic using `require-ext.sh`, which is aware of `hgext3rd` and
prints skip message.
This affects `arc lint` so hopefully our new testing code would be free of this
kind of mistakes.
Test Plan: `arc lint` would catch errors
Reviewers: #mercurial, ttung, rmcelroy
Reviewed By: rmcelroy
Subscribers: rmcelroy, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3550977
Signature: t1:3550977:1468455857:e849dfd9e3cbc446cc6e6c662050ee88a3366e6c
Summary: Rather than have fastmanifestcache as a singleton, we attach it to each opener.
Test Plan: pass existing unit tests.
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3553542
Tasks: 12169797
Signature: t1:3553542:1468373683:b0a7fcdf5dbf12046e5b4be9aa0bc5d46e55ce06
Summary: `prune` is essentially `makeroomfor(0, set())`.
Test Plan: pass existing unit tests. output is slightly different, but that's just because we no longer output that debugging line.
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3547069
Signature: t1:3547069:1468352305:6185132648f871e26d3cfd449c059523b5eb6d6b
Summary:
When running large repack operations, the resident size of the process
could become quite large, since we're scanning in entire pack files. Linux/OSX
have api calls for telling the kernel it's ok to release some of that memory,
but those apis are not exposed to python.
So instead, let's unmap and remap the mmap's once a certain amount of data has
been read. I also tried changing the mmap accessors to use the file oriented api
(mmap.read(), mmap.seek(), etc) so we could switch to actual file handles during
repack, but it had a drastic affect on normal performance (repack took 1 hour
instead of a few minutes).
Long term we should move all of this logic to c++ so we can use the more
powerful APIs.
Test Plan:
Did a full repack on a laptop and verified memory capped out at 2GB
instead of exceeding 5GB.
Reviewers: #sourcecontrol, ttung
Differential Revision: https://phabricator.intern.facebook.com/D3545171
Summary: It's caught by the contbuild script. I forgot to change this test.
Test Plan: Run `test-patchpython.t`.
Reviewers: #mercurial, ttung, simonfar
Reviewed By: simonfar
Subscribers: simonfar, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3550459
Signature: t1:3550459:1468349937:07123651fb103d283a382d1323d9f69a5c5d81b7
Blame Revision: D3534311
Summary:
Be a better citizen under system python path.
Fix all tests issues and change setup.py to use glob pattern to include
all extensions.
Test Plan:
Run tests and `make local`.
Also build and install the package and run `hg sl` in major repos.
Reviewers: #mercurial, ttung, rmcelroy
Reviewed By: rmcelroy
Subscribers: rmcelroy, durham, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3534311
Signature: t1:3534311:1468275426:fe122646c8bd6c541e1889e73e9df28f86747ff2
Summary:
It's kind of silly to write the FM cache entry, and then immediately blow away the entry if it doesn't fit in the limit. Instead, just calculate if we have enough space, and raise CacheFullException if we don't.
Depends on D3538944
Test Plan: pass existing tests.
Reviewers: lcharignon, durham
Reviewed By: durham
Subscribers: mitrandir, mjpieters, quark
Differential Revision: https://phabricator.intern.facebook.com/D3545446
Signature: t1:3545446:1468283437:7e4167e3b5a13bf3d01fca45a965d7aaa8843166
Summary:
There's two issues with `setwithlimit`:
1) It does not have a docblock, nor an API that fully covers all the possible conditions.
2) It returns None if the cache entry is already present.
This modifies `setwithlimit` to return True if the cache entry ultimately makes it (whether it previously existed or the write was successful), False if the write fails, and raises CacheFullException if the cache is full.
Test Plan: pass existing tests. the already-in-cache case is a race condition that is not practical to reproduce.
Reviewers: lcharignon, durham
Reviewed By: durham
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3545407
Signature: t1:3545407:1468283390:64d6cdba7a17cca9bd4465c6e03b9143e4b4cc4b
Summary:
Instead of pruning at the end, we need to prune as we are writing entries.
Depends on D3545060
Test Plan: pass test in D3545060
Reviewers: lcharignon, durham
Reviewed By: durham
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3545267
Signature: t1:3545267:1468279898:5332838efa52bb55733d0e6dcf613b27e71e9d3e
Summary: Currently, this test breaks! This is awful because that means for users, when the cache becomes full, we never accept new manifests. :( :( :(
Test Plan: it breaks. :( did i say :( ?
Reviewers: lcharignon, durham
Reviewed By: durham
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3545060
Tasks: 12136039
Signature: t1:3545060:1468279919:ff8928557c8ad03e06370ee75f2386ffb46f54fa
Summary:
Previously, depending on the code path, the limit specified would not actually take effect. For instance, if we came in from debugmanifestcache, and attempted to populated the cache, we would use `systemawarecachelimit` when filling the cache, and the fixedsize limit specified by the user when pruning.
With this change, we unify the all the cache limit decisions to `fastmanifestcache`. If the user actually overrides the limit, we set the limit in `fastmanifestcache` and let that make the decisions.
We also change the definitions of limit in `hg debugcachemanifest` to:
1) >0 => it's the limit.
2) =0 => use systemawarecachelimit
3) <0 => no limit!
Test Plan: pass existing unit tests. there's a small change in the test output, because we always evaluate the limit now, plus we remove the test for limit=0, since it means something different now.
Reviewers: lcharignon, durham
Reviewed By: durham
Subscribers: trunkagent, mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3544997
Signature: t1:3544997:1468281604:8f78f00ebf2afd8f3f1fbefbd82316b97cc4b193
Summary:
We need a fastmanifest object in order to size it. Once we know its size, we can make room in the cache.
This slightly affects one of the tests, as we request the manifest text earlier than we previously did.
Depends on D3537904
Test Plan: used in later diff.
Reviewers: lcharignon, durham
Reviewed By: durham
Subscribers: mitrandir, mjpieters, quark
Differential Revision: https://phabricator.intern.facebook.com/D3538991
Signature: t1:3538991:1468280648:41c65d91529babe0559eac7b75509481adf2765f
Summary: This is actually, strictly speaking, an upper bound. This is because we haven't gone and done the work of compacting the tree. Since the tree may have undergone deletions, the directory nodes may be oversized.
Test Plan: used in later diff.
Reviewers: lcharignon, durham
Reviewed By: durham
Subscribers: mitrandir, mjpieters, quark
Differential Revision: https://phabricator.intern.facebook.com/D3538944
Signature: t1:3538944:1468268102:45f2cbaa3dc156352ff3f71d0c53393f2445813a
Summary: This allows us to convert a manifest once, and try to repeatedly jam it into the cache (presumably while evicting things).
Test Plan: pass existing tests.
Reviewers: lcharignon, durham
Reviewed By: durham
Subscribers: mitrandir, mjpieters, quark
Differential Revision: https://phabricator.intern.facebook.com/D3537904
Signature: t1:3537904:1468268209:6f1746d3cee5071321d830bb91efd591a31a8616
Summary: We don't need this since we loosely lock the cache worker.
Test Plan: pass existing tests.
Reviewers: lcharignon, durham
Reviewed By: durham
Subscribers: mitrandir, mjpieters, quark
Differential Revision: https://phabricator.intern.facebook.com/D3537355
Signature: t1:3537355:1468268248:ebf7de5bf5030b90039823e1111a073d7eacc447
Test Plan: ran tests, ran it with chg
Reviewers: durham, quark
Reviewed By: quark
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3533231
Signature: t1:3533231:1467986141:dcc020b8cd29dc58c532690a73737a5ce149da51
Summary: @quark suggested it in D3519183. It does not work in both places because the other is not an immediate return.
Test Plan: make local && pass fastmanifest unit tests
Reviewers: lcharignon, quark
Reviewed By: quark
Subscribers: mitrandir, mjpieters, quark
Differential Revision: https://phabricator.intern.facebook.com/D3530652
Signature: t1:3530652:1467927107:99c9aeb6b873ab3cdca974f526c3984f4256e201
Summary:
There was a race condition where if a repack is running and another hg process
launches, the new process will only see the original packs, and not any of the
new packs (even though the source blobs are being deleted from disk by the
repack).
The fix is to allow our pack store to refresh it's list of packs every so often.
In this particular implementation we do it at most every 100ms. A more robust
strategy would be to group key misses and only check for new packs at the end
once we have a list of all the misses, but this would require significant
refactoring to make everything grouped. This case should only ever happen during
repacks, so it should almost never occur more than once during a command, so the
100ms version is probably good enough.
Test Plan:
Ran `hg up && hg pull && sleep 0.2 && hg up master` in a loop with a
break point in the refresh code and caught it executing in a situation where the
background repack had removed the original sources and put them in a new pack.
Verified that it loaded the data from the new pack correctly.
Reviewers: #mercurial, ttung, lcharignon
Reviewed By: lcharignon
Subscribers: lcharignon
Differential Revision: https://phabricator.intern.facebook.com/D3524314
Signature: t1:3524314:1467907680:85be07ad953811000c468852eb0626f4d8b53a13
Summary:
The shared cache needs to be completely g+ws so that all members of the group
can write to each directory in it. The old code only applied g+ws to the leaf
directories, so other users aren't able to write to non leaf directories (like
hgcache/7a/83beca8.../ others couldn't write to 7a/)
Test Plan:
Updated a test to view group permissions for the intermediate
directories
Reviewers: #mercurial, ttung, simpkins
Reviewed By: simpkins
Subscribers: lcharignon, net-systems-diffs@, simpkins, mbolin
Differential Revision: https://phabricator.intern.facebook.com/D3523918
Signature: t1:3523918:1467930221:452b11b56a2e69896bf8d2cd0acd7131b41f90d8
Summary:
Previously, the history repack logic would stop traversing history for a given
filename once it encountered a rename. This isn't quite right, since the history
could eventually be traversed back to the original file, where we'd need to
continue processing. So now we check for when the copyfrom becomes the filename.
Also, if the copy source file and the copy target file have two nodes with the
same value, we would not process the one in the copy target (since it was marked
do not process). We fix this by explicitly checking if the node is one of the
known entries in the file being processed.
Test Plan: Added a test
Reviewers: #mercurial, ttung, mitrandir, rmcelroy
Reviewed By: mitrandir, rmcelroy
Differential Revision: https://phabricator.intern.facebook.com/D3523215
Signature: t1:3523215:1467828169:bd487c8f296352c1a1b9355cb55f9001bd5e19a9
Summary:
Before this patch we were doing two mistakes in the ratio computation.
- We were not recording global cache hit or miss, it was always a hit because
a function is truthy is python
- We were not deduping cache miss for the same manifest multiple times
This patch fixes these two mistakes.
It also changes the logging of cache hit and miss to include the name of the
operation that triggered the cache hit or miss (diff or filesnotin).
Test Plan: Test output changed and commented
Reviewers: durham, ttung
Differential Revision: https://phabricator.intern.facebook.com/D3484658
Summary:
Before this patch we were using config instead of configbool for reading
the debugmetrics config causing "False" to be evaluated as a truthy value
for the config. This patch fixes the issue and sets the config to false for
some of the tests to reduce the noise of the output.
Test Plan: Tests pass
Reviewers: ttung
Differential Revision: https://phabricator.intern.facebook.com/D3524501
Summary:
There's no need for the `setup()` to be called in `reposetup()` any more.
Depends on D3510966
Test Plan:
1) pass existing unit tests.
2) kill chg server instances, then run `USE_CHG= hg diff -c . --debug` repeatedly. (USE_CHG tells my hg wrapper to use chg)
Reviewers: quark, lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3511004
Signature: t1:3511004:1467410145:0761c80da77d5a193fbdb08527fcc4aae9f8065e
Summary: Whenever fastmanifest needs an ui object, we pass it a global proxy object. Whenever someone calls `reposetup(..)`, we update the backing object.
Test Plan:
1) pass existing unit tests
2) enable chg, then run `hg diff -c . --debug`. saw fastmanifest debug output in the console.
Reviewers: lcharignon, quark
Reviewed By: lcharignon
Subscribers: trunkagent, mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3510966
Signature: t1:3510966:1467410140:88fe810d03b9a9aa6818f17eb605a70eee931364
Summary:
There are two parts to add logging to blackbox. Logging events
and changing the configuration of blackbox. This diff does the former.
We don't reuse all the metrics event as:
- blackbox does not support wildcard for events like fastmanifest-*
- they are not meant to be human readable, they are for performance logging
We instead log a few new events:
- What triggered a caching operation?
- What command we run to cache the manifest?
- What revisions are to be cached?
- What revisions are actuallt cached?
- Do we overflow the cache?
This will allow us to understand all the cases where caching didn't trigger.
Like in t11877434.
Test Plan:
Test output does not change
Verified manually by changing the blackbox configuration that events get logged:
CHGDISABLE=1 hg book food --config blackbox.track='command, commandfinish, commandexception, exthook, fsmonitor, pythonhook, fastmanifest' --config extensions.fastmanifest=/home/lcharignon/facebook-hg-rpms/fb-hgext/fastmanifest --config extensions.blackbox=/home/lcharignon/facebook-hg-rpms/hg-crew/hgext/blackbox.py
Reviewers: ttung
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3504158
Tasks: 11877434
