Page MenuHomePhabricator

pulkit (Pulkit Goyal)
Spy

Projects

User Details

User Since
Jun 28 2017, 8:54 PM (169 w, 2 d)

Recent Activity

Today

pulkit added a comment to D9065: merge: make low-level update() private (API).

tests/test-install.t and tests/test-duplicateoptions.py are unhappy about this change.

--- /tmp/mercurial-ci/tests/test-duplicateoptions.py.out
+++ /tmp/mercurial-ci/tests/test-duplicateoptions.py.err
@@ -0,0 +1,9 @@
+Traceback (most recent call last):
+  File "/tmp/hgtests.j0rlbd/install/lib/python/mercurial/extensions.py", line 264, in _runextsetup
+    extsetup(ui)
+  File "/tmp/hgtests.j0rlbd/install/lib/python/hgext/fsmonitor/__init__.py", line 730, in extsetup
+    extensions.wrapfunction(merge, b'update', wrapupdate)
+  File "/tmp/hgtests.j0rlbd/install/lib/python/mercurial/extensions.py", line 652, in wrapfunction
+    origfn = getattr(container, funcname)
+AttributeError: 'module' object has no attribute 'update'
+*** failed to set up extension fsmonitor: 'module' object has no attribute 'update'
ERROR: test-duplicateoptions.py output changed
--- /tmp/mercurial-ci/tests/test-install.t
+++ /tmp/mercurial-ci/tests/test-install.t.err
@@ -164,12 +164,28 @@
 debuginstall extension support
   $ hg debuginstall --config extensions.fsmonitor= --config fsmonitor.watchman_exe=false | grep atchman
-  fsmonitor checking for watchman binary... (false)
-   watchman binary missing or broken: warning: Watchman unavailable: watchman exited with code 1
+  Traceback (most recent call last):
+    File "/tmp/hgtests.j0rlbd/install/lib/python/mercurial/extensions.py", line 264, in _runextsetup
+      extsetup(ui)
+    File "/tmp/hgtests.j0rlbd/install/lib/python/hgext/fsmonitor/__init__.py", line 730, in extsetup
+      extensions.wrapfunction(merge, b'update', wrapupdate)
+    File "/tmp/hgtests.j0rlbd/install/lib/python/mercurial/extensions.py", line 652, in wrapfunction
+      origfn = getattr(container, funcname)
+  AttributeError: 'module' object has no attribute 'update'
+  *** failed to set up extension fsmonitor: 'module' object has no attribute 'update'
+  [1]
 Verify the json works too:
   $ hg debuginstall --config extensions.fsmonitor= --config fsmonitor.watchman_exe=false -Tjson | grep atchman
-    "fsmonitor-watchman": "false",
-    "fsmonitor-watchman-error": "warning: Watchman unavailable: watchman exited with code 1",
+  Traceback (most recent call last):
+    File "/tmp/hgtests.j0rlbd/install/lib/python/mercurial/extensions.py", line 264, in _runextsetup
+      extsetup(ui)
+    File "/tmp/hgtests.j0rlbd/install/lib/python/hgext/fsmonitor/__init__.py", line 730, in extsetup
+      extensions.wrapfunction(merge, b'update', wrapupdate)
+    File "/tmp/hgtests.j0rlbd/install/lib/python/mercurial/extensions.py", line 652, in wrapfunction
+      origfn = getattr(container, funcname)
+  AttributeError: 'module' object has no attribute 'update'
+  *** failed to set up extension fsmonitor: 'module' object has no attribute 'update'
+  [1]
 Verify that Mercurial is installable with pip. Note that this MUST be
 the last test in this file, because we do some nasty things to the
ERROR: test-install.t output changed
Fri, Sep 25, 8:35 AM
pulkit added a comment to D9067: merge: replace calls to hg.updaterepo() by merge.update().

While I agree that resulting API is better than the previous one, however I think:

  • This is an API breakage we can prevent
  • merge.update() used to do a different thing before and now does something else before

Want me to add a new merge.plain_update() or something like that instead?

How about this question? Do you want me to call the new function merge.plain_update() instead? Another option is to do that while leaving merge.update() as a proxy to merge._update() with a call to ui.deprecwarn(). I'm fine with any of these 3 approaches (I'd prefer what I currently have -- it should be pretty simple for callers to check if _update() exists and prefer that for compatibility until they've finished a migration).

Fri, Sep 25, 3:24 AM
D9065: merge: make low-level update() private (API) is now accepted and ready to land.
Fri, Sep 25, 3:21 AM
D9063: merge: use merge.clean_update() when applicable is now accepted and ready to land.
Fri, Sep 25, 3:19 AM
pulkit added inline comments to D9066: merge: add a higher-level update() for the common `hg update` use case.
Fri, Sep 25, 3:18 AM
pulkit committed rHG10284ce3d5ed: scmutil: introduce function to check whether repo uses treemanifest or not.
scmutil: introduce function to check whether repo uses treemanifest or not
Fri, Sep 25, 3:16 AM
pulkit added a comment to D9076: rebase: teach in-memory rebase to not restart with on-disk rebase on conflict.

I've made this not dependent on the merge.update() changes, so it can now be queued separately.

Fri, Sep 25, 3:15 AM
pulkit added a comment to D8973: log: add support for bookmarks.

For the record, I'm -1 on this, for the following reasons:

  • We have revsets in order to avoid adding flags to hg log and other commands. However, I think it makes sense to have flags for the most common options (-k and -u are the ones I use most). I'm not sure -B will be common enough.
  • The semantics are unintuitive. When I first saw it, I assumed it would show just the bookmarked commit itself. When I saw some of the tests and saw that it also showed other commits, I figured it must be showing only($bookmark) or maybe only($bookmark, $all_other_bookmarks). It instead seems to do the same as hg log --follow $bookmark
  • We have a bookmark() revset, which matches exactly the bookmarked commit, not including ancestors. It seems unfortunate for that revset and this new flag to disagree.
Fri, Sep 25, 3:14 AM
pulkit committed rHG5a16798ab57f: scmutil: introduce function to check whether repo uses treemanifest or not.
scmutil: introduce function to check whether repo uses treemanifest or not
Fri, Sep 25, 12:31 AM

Yesterday

pulkit closed D8981: scmutil: introduce function to check whether repo uses treemanifest or not.
Thu, Sep 24, 9:29 AM
pulkit committed rHGdfe585655c3b: scmutil: introduce function to check whether repo uses treemanifest or not.
scmutil: introduce function to check whether repo uses treemanifest or not
Thu, Sep 24, 9:27 AM
D9006: chg: make is possible to call by default an hg binary located next to chg is now accepted and ready to land.
Thu, Sep 24, 3:38 AM
pulkit added a comment to D9067: merge: replace calls to hg.updaterepo() by merge.update().

While I agree that resulting API is better than the previous one, however I think:

  • This is an API breakage we can prevent
  • merge.update() used to do a different thing before and now does something else before

Want me to add a new merge.plain_update() or something like that instead?

Although, we don't provide any API's compatibility but preventing such breakages when we can sound a good thing to me. I think we can leave hg.updaterepo() as it is.

Sure, I can do that. So just leave it without callers you mean? Or drop this patch completely?

Thu, Sep 24, 3:36 AM
pulkit added a comment to D9067: merge: replace calls to hg.updaterepo() by merge.update().

While I agree that resulting API is better than the previous one, however I think:

Thu, Sep 24, 3:08 AM
D9072: phases: fix performance regression with Python 2. is now accepted and ready to land.
Thu, Sep 24, 3:02 AM
D9061: test: add bookmark for 'log' in test-completion is now accepted and ready to land.
Thu, Sep 24, 3:00 AM
D9060: test: test 'hg log' using a bookmark is now accepted and ready to land.
Thu, Sep 24, 3:00 AM
pulkit added a comment to D8973: log: add support for bookmarks.

Folded next couple of patches in this one and queued. Thank you!

Thu, Sep 24, 2:59 AM
D9078: repoview: don't crash if mergestate points to non-existent node is now accepted and ready to land.
Thu, Sep 24, 2:53 AM
D9077: tests: demonstrate crash caused by pinning of non-existent mergestate node is now accepted and ready to land.
Thu, Sep 24, 2:53 AM
D9076: rebase: teach in-memory rebase to not restart with on-disk rebase on conflict is now accepted and ready to land.
Thu, Sep 24, 2:53 AM
D9075: rebase: move check for unresolved conflicts into lower-level rebasenode() is now accepted and ready to land.
Thu, Sep 24, 2:51 AM
D9074: rebase: add dryrun property to rebaseruntime is now accepted and ready to land.
Thu, Sep 24, 2:51 AM
D9073: rebase: when collapsing, p1 == dest, so use the former only is now accepted and ready to land.
Thu, Sep 24, 2:50 AM
D9070: rebase: remove redundant isinmemory argument from _origrebase() is now accepted and ready to land.
Thu, Sep 24, 2:49 AM
D9069: largefiles: prevent in-memory merge instead of switching to on-disk is now accepted and ready to land.
Thu, Sep 24, 2:49 AM
D9064: merge: add a back_out() function to encapsulate update() is now accepted and ready to land.
Thu, Sep 24, 2:48 AM

Wed, Sep 23

pulkit closed D9045: tests: update test-share-safe to work with non-zstd versions.
Wed, Sep 23, 11:37 AM
pulkit committed rHG68906595016c: tests: update test-share-safe to work with non-zstd versions.
tests: update test-share-safe to work with non-zstd versions
Wed, Sep 23, 11:37 AM
pulkit closed D9025: mergestate: define NO_OP_ACTION in module scope instead of inside mergeresult.
Wed, Sep 23, 7:40 AM
pulkit closed D9002: mergestate: introduce a new ACTION_KEEP_NEW.
Wed, Sep 23, 7:40 AM
pulkit committed rHG590a840fa367: mergestate: define NO_OP_ACTION in module scope instead of inside mergeresult.
mergestate: define NO_OP_ACTION in module scope instead of inside mergeresult
Wed, Sep 23, 7:39 AM
pulkit committed rHG6877b0ee5f9d: mergestate: introduce a new ACTION_KEEP_NEW.
mergestate: introduce a new ACTION_KEEP_NEW
Wed, Sep 23, 7:39 AM
pulkit updated the diff for D8673: config: add a .hg/hgrc-not-shared which won't be shared in share-safe mode.
Wed, Sep 23, 3:24 AM
pulkit updated the diff for D9047: dispatch: load shared source repository config in share-safe mode.
Wed, Sep 23, 3:22 AM
pulkit updated the diff for D9046: tests: add test showing broken extension loading in case of share-safe.
Wed, Sep 23, 3:22 AM
pulkit retitled D9045: tests: update test-share-safe to work with non-zstd versions from tests: update test-share-safe to work with pure-python versions to tests: update test-share-safe to work with non-zstd versions.
Wed, Sep 23, 3:22 AM

Sat, Sep 19

D9041: mergedriver: delete it is now accepted and ready to land.

Looks good to me. Will push it if I don't hear any concerns in the next 4 days.

Sat, Sep 19, 3:33 AM
D9057: rebase: stop clearing on-disk mergestate when running in memory is now accepted and ready to land.
Sat, Sep 19, 3:32 AM
D9055: rebase: delete unused p1 argument to _concludenode() is now accepted and ready to land.
Sat, Sep 19, 3:31 AM
D9054: rebase: fix an inconsistent hyphenation in a debug message is now accepted and ready to land.
Sat, Sep 19, 3:30 AM

Fri, Sep 18

D9024: run-test: allow relative path in `--blacklist` and `--whitelist` (issue6351) is now accepted and ready to land.
Fri, Sep 18, 10:43 AM
pulkit retitled D8673: config: add a .hg/hgrc-not-shared which won't be shared in share-safe mode from config: add a .hg/nonsharedrc which won't be shared in share-safe mode to config: add a .hg/hgrc-not-shared which won't be shared in share-safe mode.
Fri, Sep 18, 10:43 AM
pulkit created D9047: dispatch: load shared source repository config in share-safe mode.
Fri, Sep 18, 10:41 AM
pulkit created D9046: tests: add test showing broken extension loading in case of share-safe.
Fri, Sep 18, 10:41 AM
pulkit added a comment to D8673: config: add a .hg/hgrc-not-shared which won't be shared in share-safe mode.

Before approving this, I'd like to have a quick conversation about the file name and documentation.
Do we think we'd be better off having the new config begin with hgrc so all the config files are in proximity in sorted file lists?

Fri, Sep 18, 9:18 AM
pulkit added a comment to D8977: merge: check for dir rename dest before adding ACTION_KEEP.

I have some changes which need to be made to this patch according to Yuya's review on mailing list.

Fri, Sep 18, 9:04 AM
pulkit created D9045: tests: update test-share-safe to work with non-zstd versions.
Fri, Sep 18, 8:08 AM
D9043: mergestate: move commit() from base class to on-disk subclass is now accepted and ready to land.
Fri, Sep 18, 2:57 AM
D9044: phabricator: fix loadhgrc() override broken by D8656 is now accepted and ready to land.
Fri, Sep 18, 2:56 AM
pulkit added inline comments to D9043: mergestate: move commit() from base class to on-disk subclass.
Fri, Sep 18, 2:53 AM
D9042: mergestate: make in-memory mergestate not clear on-disk mergestate on reset() is now accepted and ready to land.
Fri, Sep 18, 2:51 AM

Thu, Sep 17

pulkit closed D8983: merge: move initial handling of mergeactions near to later one.
Thu, Sep 17, 10:37 PM
pulkit committed rHGc4f14db3da1d: merge: move initial handling of mergeactions near to later one.
merge: move initial handling of mergeactions near to later one
Thu, Sep 17, 10:35 PM
pulkit closed D8633: share: introduce config option to store requires in .hg/store.
Thu, Sep 17, 10:07 PM
pulkit closed D8659: config: add `--shared` flag to edit config file of shared source.
Thu, Sep 17, 10:07 PM
pulkit closed D8656: localrepo: load the share source .hg/hgrc also in share-safe mode (API).
Thu, Sep 17, 10:07 PM
pulkit committed rHG78f0bb37f52d: upgrade: support running upgrade if repository has share-safe requirement.
upgrade: support running upgrade if repository has share-safe requirement
Thu, Sep 17, 10:05 PM
pulkit closed D8660: upgrade: support running upgrade if repository has share-safe requirement.
Thu, Sep 17, 10:05 PM
pulkit committed rHGac7a3da0dbb6: config: add `--shared` flag to edit config file of shared source.
config: add `--shared` flag to edit config file of shared source
Thu, Sep 17, 10:05 PM
pulkit committed rHGb71858b42963: localrepo: load the share source .hg/hgrc also in share-safe mode (API).
localrepo: load the share source .hg/hgrc also in share-safe mode (API)
Thu, Sep 17, 10:05 PM
pulkit closed D8913: scmutil: introduce filterrequirements() to split reqs into wc and store ones.
Thu, Sep 17, 10:04 PM
pulkit committed rHG63eb1b5c580d: helptext: document exp-sharesafe in internals/requirements.txt.
helptext: document exp-sharesafe in internals/requirements.txt
Thu, Sep 17, 10:04 PM
pulkit committed rHGd252f51ab032: share: introduce config option to store requires in .hg/store.
share: introduce config option to store requires in .hg/store
Thu, Sep 17, 10:04 PM
pulkit closed D8914: helptext: document exp-sharesafe in internals/requirements.txt.
Thu, Sep 17, 10:04 PM
pulkit closed D8952: remotefilelog: acquire lock before writing requirements on clone.
Thu, Sep 17, 10:04 PM
pulkit committed rHG9a99ab8217bd: scmutil: introduce filterrequirements() to split reqs into wc and store ones.
scmutil: introduce filterrequirements() to split reqs into wc and store ones
Thu, Sep 17, 10:04 PM
pulkit committed rHGbddc4f2ef317: remotefilelog: acquire lock before writing requirements on clone.
remotefilelog: acquire lock before writing requirements on clone
Thu, Sep 17, 10:04 PM
pulkit added a comment to D9024: run-test: allow relative path in `--blacklist` and `--whitelist` (issue6351).

Missed adding, for bug fixes, we specify bug id at end of commit message. https://www.mercurial-scm.org/wiki/ContributingChanges#Patch_descriptions

Thu, Sep 17, 8:45 AM
pulkit added inline comments to D9024: run-test: allow relative path in `--blacklist` and `--whitelist` (issue6351).
Thu, Sep 17, 8:43 AM
pulkit committed rHG6916e6b81fef: tests: run test-check-py3-compat only in pure python mode.
tests: run test-check-py3-compat only in pure python mode
Thu, Sep 17, 7:49 AM
pulkit updated the diff for D8981: scmutil: introduce function to check whether repo uses treemanifest or not.
Thu, Sep 17, 5:52 AM
pulkit updated the diff for D9003: merge: store cases when a file is absent post merge in commitinfo.
Thu, Sep 17, 5:52 AM
pulkit updated the diff for D8989: commit: force create a new filenode if it was set in mergestate by merge.
Thu, Sep 17, 5:51 AM
pulkit created D9029: tests: add some more debugmergestate calls in `test-merge-criss-cross.t`.
Thu, Sep 17, 5:51 AM
pulkit updated the diff for D8988: merge: store commitinfo if mergetool resolved a dc or cd conflict.
Thu, Sep 17, 5:51 AM
pulkit updated the diff for D8982: merge: disable `m2-vs-ma` diff optimization in case of flat manifests.
Thu, Sep 17, 5:51 AM
pulkit created D9028: tests: add some debugmergestate calls in `test-merge-criss-cross.t`.
Thu, Sep 17, 5:50 AM
pulkit updated the diff for D8987: tests: add few debugrevlogindex and a log call to see changes in next test.
Thu, Sep 17, 5:50 AM
pulkit created D9027: merge: check new filenode creation config before disabling optimization.
Thu, Sep 17, 5:49 AM
pulkit created D9026: configitems: add a new config option to control new filenode functionality.
Thu, Sep 17, 5:49 AM
pulkit created D9025: mergestate: define NO_OP_ACTION in module scope instead of inside mergeresult.
Thu, Sep 17, 5:49 AM
pulkit updated the diff for D9002: mergestate: introduce a new ACTION_KEEP_NEW.
Thu, Sep 17, 5:48 AM
pulkit updated the summary of D8977: merge: check for dir rename dest before adding ACTION_KEEP.
Thu, Sep 17, 5:48 AM
pulkit updated the diff for D8983: merge: move initial handling of mergeactions near to later one.
Thu, Sep 17, 5:40 AM

Wed, Sep 16

pulkit updated the diff for D8673: config: add a .hg/hgrc-not-shared which won't be shared in share-safe mode.
Wed, Sep 16, 7:58 AM
pulkit updated the diff for D8633: share: introduce config option to store requires in .hg/store.
Wed, Sep 16, 7:58 AM
pulkit updated the diff for D8659: config: add `--shared` flag to edit config file of shared source.
Wed, Sep 16, 7:57 AM
pulkit updated the diff for D8656: localrepo: load the share source .hg/hgrc also in share-safe mode (API).
Wed, Sep 16, 7:57 AM
pulkit closed D8941: merge: add missing ACTION_KEEP when both remote and ancestor are not present.
Wed, Sep 16, 7:49 AM
pulkit closed D8940: merge: store ACTION_KEEP_ABSENT when we are keeping the file absent locally.
Wed, Sep 16, 7:49 AM
pulkit committed rHG6e474eec4be6: merge: update commitinfo from all mergeresults during bid merge.
merge: update commitinfo from all mergeresults during bid merge
Wed, Sep 16, 7:49 AM
pulkit closed D8965: merge: update commitinfo from all mergeresults during bid merge.
Wed, Sep 16, 7:49 AM
pulkit closed D8974: merge: add `ACTION_KEEP_ABSENT` to represent files we want to keep absent.
Wed, Sep 16, 7:49 AM
pulkit committed rHG49ffaa4f65f6: merge: add missing ACTION_KEEP when both remote and ancestor are not present.
merge: add missing ACTION_KEEP when both remote and ancestor are not present
Wed, Sep 16, 7:49 AM
pulkit committed rHG09edbff6ae8d: merge: store ACTION_KEEP_ABSENT when we are keeping the file absent locally.
merge: store ACTION_KEEP_ABSENT when we are keeping the file absent locally
Wed, Sep 16, 7:49 AM
pulkit committed rHGbb9888d32601: merge: add `ACTION_KEEP_ABSENT` to represent files we want to keep absent.
merge: add `ACTION_KEEP_ABSENT` to represent files we want to keep absent
Wed, Sep 16, 7:49 AM
pulkit committed rHG14b3dbfa4eeb: mergeresult: introduce dedicated tuple for no-op actions.
mergeresult: introduce dedicated tuple for no-op actions
Wed, Sep 16, 7:49 AM
D8801: obsstore: refactor v1 logic to fix 32 byte hash support is now accepted and ready to land.
Wed, Sep 16, 3:32 AM
pulkit added a comment to D9023: branchmap: add a cache validation cache, avoid expensive re-hash on every use.

Looks good to me apart from the probable unrelated change.

Wed, Sep 16, 3:29 AM