[Bug 5446] New: Fix Bugzilla so that it doesn't comment on its own issues
https://bz.mercurial-scm.org/show_bug.cgi?id=5446 Bug ID: 5446 Summary: Fix Bugzilla so that it doesn't comment on its own issues Product: Mercurial Version: unspecified Hardware: PC OS: Mac OS Status: UNCONFIRMED Severity: feature Priority: wish Component: infrastructure Assignee: bugzi...@mercurial-scm.org Reporter: kbullock+mercur...@ringworld.org CC: kbullock+mercur...@ringworld.org, mercurial-de...@selenic.com -- You are receiving this mail because: You are on the CC list for the bug. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] revlog: merge hash checking subfunctions
On 12/13/2016 03:25 PM, Remi Chaintron wrote: # HG changeset patch # User Remi Chaintron # Date 1481638896 0 # Tue Dec 13 14:21:36 2016 + # Branch stable # Node ID d34c04da52a8f2eb61b51bb0aa9cedf1e4b43474 # Parent 7817df5585db1d87d3f6c7f496085c86d87e2e9a revlog: merge hash checking subfunctions This patch factors the behavior of both methods into 'checkhash'. Pushed, thanks. -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH] chg: ignore HG_* in confighash
# HG changeset patch # User Jun Wu # Date 1481681879 0 # Wed Dec 14 02:17:59 2016 + # Node ID b215e409984eb261508e24b6e55f4c18716cb6f6 # Parent af482d20004c97e7ad8d6e2e5932e82a934761e3 # Available At https://bitbucket.org/quark-zju/hg-draft # hg pull https://bitbucket.org/quark-zju/hg-draft -r b215e409984e chg: ignore HG_* in confighash The environment variables `HG_*` are usually used by hooks. Unlike `HGPLAIN` etc, they do not actually affect hg's behavior. So do not include them in confighash. This would avoid spawning an unbound number of chg server processes if commit hook calls hg frequently. diff --git a/mercurial/chgserver.py b/mercurial/chgserver.py --- a/mercurial/chgserver.py +++ b/mercurial/chgserver.py @@ -78,5 +78,5 @@ def _hashlist(items): _envre = re.compile(r'''\A(?: CHGHG -|HG.* +|HG[A-Z].* |LANG(?:UAGE)? |LC_.* ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH RFC v7] scmutil: add a simple key-value file helper
On 12/06/2016 10:53 PM, Jun Wu wrote: Excerpts from Kostia Balytskyi's message of 2016-12-06 13:41:07 -0800: class CorruptedState(Exception): """error raised when a command is not able to read its state from file""" + +class MissingRequiredKeyInFileException(Exception): +"""error raised when simple key-value file misses a required key""" I still think "CorruptedState" is better. It fits the use-case, is short and concise. 6-word exception sounds strange to me, partially because everything else in error.py is at most 4 words. If we have to use a new exception, maybe just "MissingRequiredKey", or just "KeyError". I've not looked at any logic or any context for this, but "MissingRequiredKeyInFileException" is most certainly a TooLongNameToBeAccepted See https://lwn.net/Articles/455265/ for details ;-) Cheers, -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[Bug 5445] New: Host html documentation/man pages on mercurial-scm.org
https://bz.mercurial-scm.org/show_bug.cgi?id=5445 Bug ID: 5445 Summary: Host html documentation/man pages on mercurial-scm.org Product: Mercurial Version: unspecified Hardware: All URL: https://www.selenic.com/mercurial/hgrc.5.html OS: All Status: UNCONFIRMED Severity: feature Priority: wish Component: infrastructure Assignee: bugzi...@mercurial-scm.org Reporter: a...@dwimlabs.net CC: kbullock+mercur...@ringworld.org, mercurial-de...@selenic.com Looks like html versions of man pages like this: https://www.selenic.com/mercurial/hgrc.5.html are not on mercurial-scm.org. -- You are receiving this mail because: You are on the CC list for the bug. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 2] bookmarks: make bookmarks.comparebookmarks accept binary nodes (API)
On Fri, Dec 09, 2016 at 03:31:45AM -0800, Stanislau Hlebik wrote: > # HG changeset patch > # User Stanislau Hlebik > # Date 1481282546 28800 > # Fri Dec 09 03:22:26 2016 -0800 > # Node ID df861963a18c00d72362b415a77a62d2c18660bf > # Parent 08ab8f9d0abcbd1b2405ecb0a8670d212866af1f > bookmarks: make bookmarks.comparebookmarks accept binary nodes (API) I've queued these two, thanks. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[Bug 5444] New: 7x perf regression in huge hg pulls
https://bz.mercurial-scm.org/show_bug.cgi?id=5444 Bug ID: 5444 Summary: 7x perf regression in huge hg pulls Product: Mercurial Version: default branch Hardware: PC OS: Linux Status: UNCONFIRMED Severity: feature Priority: wish Component: Mercurial Assignee: bugzi...@mercurial-scm.org Reporter: dur...@fb.com CC: mercurial-de...@selenic.com It looks like e240e914d226 (revlog: seek to end of file before writing (issue4943)) causes a significant perf regression in revlogs writes when doing large pulls. In our case (linux on ext4), pulling 100k changelog entries went from 17s before the change, to 130s after the change. -- You are receiving this mail because: You are on the CC list for the bug. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] parsers: use buffer to store revlog index
Excerpts from Gregory Szorc's message of 2016-12-13 08:28:14 -0800: > On Tue, Dec 13, 2016 at 6:04 AM, Yuya Nishihara wrote: > > > On Mon, 12 Dec 2016 22:11:02 -0800, Gregory Szorc wrote: > > > This patch LGTM. Only thing I would have done differently is store a > > > Py_buffer* instead of a Py_buffer to ensure alignment of indexObject > > struct > > > entries. But I don't think this struct is that performance sensitive to > > > care. Feel free to address in a follow-up if you disagree. > > > > Just curious. What kind of alignment issue would be solved by not storing > > a Py_buffer struct? > > > > Without looking at what is actually inside Py_buffer, I'm somewhat paranoid > that struct members won't be aligned on word boundaries, creating members > that span words, leading to sub-optimal access of highly-accessed members. > I know Python is generally pretty good about struct alignment. And the > compiler may not be "packing" structs unless ordered. So perhaps we're fine > here? I think we are fine as the struct is not explicitly packed. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 6 of 6 v3] convert: Create commits from revmap list if needed
> On Dec 8, 2016, at 05:00, Kostia Balytskyi wrote: > > On 12/8/16 12:18 AM, David Soria Parra wrote: >> On Wed, Dec 07, 2016 at 10:23:08PM +, Kostia Balytskyi wrote: >>> On 12/7/16 9:51 PM, David Soria Parra wrote: # HG changeset patch # User David Soria Parra # Date 1481143876 28800 # Wed Dec 07 12:51:16 2016 -0800 # Node ID 109de539306c5bc49d38d6f1c802c4a8d092b485 # Parent be68e4436851b7e20f3b8cb34666418f5840dd66 convert: Create commits from revmap list if needed >>> This commit demonstrates the need to add some sort of comment to >>> self.revmap = {} in constructor. Otherwise, it's very confusing IMO. >> I honestly don't undersetand what is so confusing about it. It's a >> revmap and it seems straight forward to me that it's a dictionary >> that maps commits, at least thats what all revmaps in convert ever do. > The confusing (at least to me) part is what revmap maps commits to. > There's no example > of setrevmap usage, so it's unclear how it is used. Feel free to ignore > this if you think this is unreasonable though. I'm also confused by the addition of setrevmap when it doesn't appear to be used by anything. Do you have follow-up work that this is leading to? pacem in terris / мир / शान्ति / سَلاَم / 平和 Kevin R. Bullock ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
RE: [PATCH 1 of 2] error: add ProgrammingError
> -- This message, including its attachments, is confidential. For more information please read NNG's email policy here: http://www.nng.com/emailpolicy/ By responding to this email you accept the email policy. -Original Message- > From: Mercurial-devel [mailto:mercurial-devel-boun...@mercurial-scm.org] > On Behalf Of Gábor STEFANIK > Sent: Tuesday, December 13, 2016 5:53 PM > To: Jun Wu ; mercurial-devel@mercurial-scm.org > Subject: RE: [PATCH 1 of 2] error: add ProgrammingError > > > > > > -- > This message, including its attachments, is confidential. For more information > please read NNG's email policy here: > http://www.nng.com/emailpolicy/ > By responding to this email you accept the email policy. > > > -Original Message- > > From: Mercurial-devel > > [mailto:mercurial-devel-boun...@mercurial-scm.org] > > On Behalf Of Jun Wu > > Sent: Tuesday, December 6, 2016 6:14 PM > > To: mercurial-devel@mercurial-scm.org > > Subject: [PATCH 1 of 2] error: add ProgrammingError > > > > # HG changeset patch > > # User Jun Wu > > # Date 1481036267 0 > > # Tue Dec 06 14:57:47 2016 + > > # Node ID 67bcd43e64ca03f486a817fccf38e83020b06793 > > # Parent 2463b16bbd3710b619e0e948651db9948341f990 > > # Available At https://bitbucket.org/quark-zju/hg-draft > > # hg pull https://bitbucket.org/quark-zju/hg-draft -r > > 67bcd43e64ca > > error: add ProgrammingError > > > > We have requirement to express "this is clearly an error caused by the > > programmer". The code base uses RuntimeError for that in some places, > > not ideal. So let's add a formal exception for that. > > > > diff --git a/mercurial/error.py b/mercurial/error.py > > --- a/mercurial/error.py > > +++ b/mercurial/error.py > > @@ -169,4 +169,7 @@ class PushRaced(RuntimeError): > > """An exception raised during unbundling that indicate a push race""" > > > > +class ProgrammingError(RuntimeError): > > I would suggest InternalError, assuming it's not reserved by the Python > language, or used for something else already in Hg. > "ProgrammingError" could be interpreted as a programming error in Hg, but > also a programming error in tracked code. It's more of a stretch than with > "developer", but "internal" is still clearer than "programming". > > See also: GCC's ICEs (but "in-SCM exception" is clunky IMO) Looks like I was wrong - ICE apparently stands for "internal compiler error", not "in-compiler exception". So, InternalError is in agreement with GCC's nomenclature. > > > +"""Raised if a developer has made some mistake""" > > + > > # bundle2 related errors > > class BundleValueError(ValueError): > > ___ > > Mercurial-devel mailing list > > Mercurial-devel@mercurial-scm.org > > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel > ___ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: Constant naming convention
> On Dec 13, 2016, at 10:29, Pierre-Yves David > wrote: > > On 12/12/2016 04:12 PM, Augie Fackler wrote: > >> Can we just align with prevailing Python style here and start using >> all caps for constants? This comes up roughly monthly, and I think >> it'd be a good clarity win. > > As much as there is many thing in our current coding still, it is our current > coding style and our codebase is consistent with it for the very vast > majority. As stated previously, my current stance is that we should refrain > for changing this category for policy for about one year after Matt stepped > away. So I would like us to keep to lower case constants for now. > At the 4.4 sprint, I'll be very happy to have a large discussion about our > style (and very happy to us CAPS for constant at that time). Ehh? I don't recall us ever having discussed a one-year moratorium on style changes, and I don't see a reason we would need to wait until the sprint to decide a matter of style. That being said, the prevailing (though by no means universal) style does seem to be lowercase for constants. If we were to change those to all-caps, it would mean a fair amount of churn. If we were starting from scratch I'd say we should unequivocally use all-caps; since that's not the case I'm about -0 on the idea. pacem in terris / мир / शान्ति / سَلاَم / 平和 Kevin R. Bullock ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 7 upgraderepo V2] debugcommands: stub for debugupgraderepo command
On 12/13/2016 06:18 AM, Gregory Szorc wrote: On Mon, Dec 12, 2016 at 2:17 AM, Pierre-Yves David mailto:pierre-yves.da...@ens-lyon.org>> wrote: On 11/25/2016 04:28 AM, Gregory Szorc wrote: # HG changeset patch # User Gregory Szorc mailto:gregory.sz...@gmail.com>> # Date 1480033449 28800 # Thu Nov 24 16:24:09 2016 -0800 # Node ID 4fffbdc2bca22e666482b55562c0e592c8a68027 # Parent c60995fce14b0e34bd1416dab3712a6c33bb29bb debugcommands: stub for debugupgraderepo command Currently, if Mercurial introduces a new repository/store feature or changes behavior of an existing feature, users must perform an `hg clone` to create a new repository with hopefully the correct/optimal settings. Unfortunately, even `hg clone` may not give the correct results. For example, if you do a local `hg clone`, you may get hardlinks to revlog files that inherit the old state. If you `hg clone` from a remote or `hg clone --pull`, changegroup application may bypass some optimization, such as converting to generaldelta. Optimizing a repository is harder than it seems and requires more than a simple `hg` command invocation. This commit starts the process of changing that. We introduce `hg debugupgraderepo`, a command that performs an in-place upgrade of a repository to use new, optimal features. The command is just a stub right now. Features will be added in subsequent commits. I like the idea of this commit, this should probably be the first one in the stack. The revlog cloning feature can be delayed later in the stack until we actually start performing upgrade. This commit does foreshadow some of the behavior of the new command, notably that it doesn't do anything by default and that it takes arguments that influence what actions it performs. These will be explained more in subsequent commits. diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py --- a/mercurial/debugcommands.py +++ b/mercurial/debugcommands.py @@ -849,3 +849,28 @@ def debugdeltachain(ui, repo, file_=None extradist=extradist, extraratio=extraratio) fm.end() + +@command('debugupgraderepo', [ +('o', 'optimize', [], _('extra optimization to perform'), _('NAME')), +('', 'run', False, _('performs an upgrade')), We should drop these option from the command until we actually use them this will allow to discuss them with their actual behavior. +]) +def debugupgraderepo(ui, repo, run=False, optimize=None): +"""upgrade a repository to use different features + +If no arguments are specified, the repository is evaluated for upgrade +and a list of problems and potential optimizations is printed. + +With ``--run``, a repository upgrade is performed. Behavior of the upgrade +can be influenced via additional arguments. More details will be provided +by the command output when run without ``--run``. (I would also delayed the help about --run until we actually run something) + +During the upgrade, the repository will be locked and no writes will be +allowed. + +At the end of the upgrade, the repository may not be readable while new +repository data is swapped in. This window will be as long as it takes to +rename some directories inside the ``.hg`` directory. On most machines, this +should complete almost instantaneously and the chances of a consumer being +unable to access the repository should be low. (I would also delay that doc until we actually behave that way) I know it would bot solve "everything" but we discussed the option of having a "repositorybeingupgraded" (or similar) in .hg/required to reduce the change for a reader to access the repository at that time. I added all this code and documentation to foreshadow the feature so a reviewer isn't guessing where things are going. It also cuts down on the amount of patches and/or churn in future patches, making code review easier, IMO. I think it is busy work to change it at this stage. But if you insist, I will. This definitely does not help me. Sure, this gives some hint of what might happen in the future, but this also make use take an early decision with very partial information on something we might end up doing different. For example the --optimize flag looks strange to me and I'm not sure it will survive later review. So gluing the 'we have a debugupgraderepo command' steps with 'we have an --optimize flag' increase the odd that the whole t
RE: [PATCH 1 of 2] error: add ProgrammingError
> -- This message, including its attachments, is confidential. For more information please read NNG's email policy here: http://www.nng.com/emailpolicy/ By responding to this email you accept the email policy. -Original Message- > From: Mercurial-devel [mailto:mercurial-devel-boun...@mercurial-scm.org] > On Behalf Of Jun Wu > Sent: Tuesday, December 6, 2016 6:14 PM > To: mercurial-devel@mercurial-scm.org > Subject: [PATCH 1 of 2] error: add ProgrammingError > > # HG changeset patch > # User Jun Wu > # Date 1481036267 0 > # Tue Dec 06 14:57:47 2016 + > # Node ID 67bcd43e64ca03f486a817fccf38e83020b06793 > # Parent 2463b16bbd3710b619e0e948651db9948341f990 > # Available At https://bitbucket.org/quark-zju/hg-draft > # hg pull https://bitbucket.org/quark-zju/hg-draft -r > 67bcd43e64ca > error: add ProgrammingError > > We have requirement to express "this is clearly an error caused by the > programmer". The code base uses RuntimeError for that in some places, not > ideal. So let's add a formal exception for that. > > diff --git a/mercurial/error.py b/mercurial/error.py > --- a/mercurial/error.py > +++ b/mercurial/error.py > @@ -169,4 +169,7 @@ class PushRaced(RuntimeError): > """An exception raised during unbundling that indicate a push race""" > > +class ProgrammingError(RuntimeError): I would suggest InternalError, assuming it's not reserved by the Python language, or used for something else already in Hg. "ProgrammingError" could be interpreted as a programming error in Hg, but also a programming error in tracked code. It's more of a stretch than with "developer", but "internal" is still clearer than "programming". See also: GCC's ICEs (but "in-SCM exception" is clunky IMO) > +"""Raised if a developer has made some mistake""" > + > # bundle2 related errors > class BundleValueError(ValueError): > ___ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
RE: [PATCH] update: introduce config option ui.allowdirtyupdate for dirty nonlinear updates
> -- This message, including its attachments, is confidential. For more information please read NNG's email policy here: http://www.nng.com/emailpolicy/ By responding to this email you accept the email policy. -Original Message- > From: Augie Fackler [mailto:r...@durin42.com] > Sent: Thursday, December 8, 2016 4:02 PM > To: Gábor STEFANIK > Cc: Jun Wu ; mercurial-devel scm.org> > Subject: Re: [PATCH] update: introduce config option ui.allowdirtyupdate for > dirty nonlinear updates > > On Thu, Dec 8, 2016 at 9:13 AM, Gábor STEFANIK > wrote: > >> > > > > > > -- > > This message, including its attachments, is confidential. For > > more information please read NNG's email policy here: > > http://www.nng.com/emailpolicy/ > > By responding to this email you accept the email policy. > > > > > > -Original Message- > >> From: Jun Wu [mailto:qu...@fb.com] > >> Sent: Wednesday, December 7, 2016 6:44 PM > >> To: Gábor STEFANIK > >> Cc: mercurial-devel > >> Subject: Re: [PATCH] update: introduce config option > >> ui.allowdirtyupdate for dirty nonlinear updates > >> > >> Excerpts from Gábor Stefanik's message of 2016-12-07 16:56:09 +0100: > >> > # HG changeset patch > >> > # User Gábor Stefanik # Date 1481126137 - > 3600 > >> > # Wed Dec 07 16:55:37 2016 +0100 > >> > # Node ID dabbe365b843fcf9b8a0de6c08e9db6100b391e9 > >> > # Parent 6472c33e16326b8c817a8bae0e75053b19badb2c > >> > update: introduce config option ui.allowdirtyupdate for dirty > >> > nonlinear updates > >> > > >> > Make it easier to test codepaths common to graft and update without > >> > having to mess around with obsolete markers to force a nonlinear > update. > >> > Named by analogy with ui.allowemptycommit. > >> > > >> > diff -r 6472c33e1632 -r dabbe365b843 mercurial/merge.py > >> > --- a/mercurial/merge.pyMon Dec 05 17:40:01 2016 +0100 > >> > +++ b/mercurial/merge.pyWed Dec 07 16:55:37 2016 +0100 > >> > @@ -1536,7 +1536,10 @@ > >> > > >> > if pas not in ([p1], [p2]): # nonlinear > >> > dirty = wc.dirty(missing=True) > >> > -if dirty or onode is None: > >> > +# experimental config: ui.allowdirtyupdate > >> > +if repo.ui.configbool('ui', 'allowdirtyupdate', False): > >> > >> I think experimental config options are usually under the "experimental" > >> section. > > > > Format.generaldelta was also experimental. > > > > Grepping through the repo for "experimental config", I'm under the > > impression that "experimental" is to be used for options that can't be > > cleanly categorized under any existing sections, and don't warrant > introducing a new section. > > [experimental] is also for things that aren't sure to be supported forever. > > > > > This is experimental for now, since we need to support "hg update > > --abort" to make this safe for users, but eventually I hope to get > > this de-experimentalized and maybe even enabled by default. It would > > be better not to break backwards compatibility just because this becomes > no longer experimental. > > Until we're confident that this feature will live forever, it should be in > experimental. I would argue that this will live forever, because: a) If we eventually decide to make this the default, some users will want to still have the old behavior - which they can get by explicitly setting ui.allowdirtyupdate=False. I don't envision ever dropping the current behavior completely. b) If we decide that nonlinear merge updates shall never be supported using the "update" command for whatever reason, we will still need it for the same debugging uses that we have now. This is as likely to live forever as merge.preferancestor, which is also marked with "experimental config:", but not in [experimental]. In terms of backwards compatibility, I would also argue that we shouldn't use [experimental] for options that _may_ live forever either, only for those that are guaranteed to be temporary (e.g. experimental.bundle2-exp, which is to be removed as bundle2 becomes mandatory for GD->GD clones): An "experimental, may live forever" option can have 2 outcomes: a) it's dropped before it matures, which is always a BC break, or b) it matures, and loses its experimental status. With a), it doesn't matter if we use [experimental] or the appropriate regular section - the outcome is always a BC break. With b), going straight for the final intended section is a clear BC win - those using the experimental-era option won't have to change anything when it matures, whereas if we initially use [experimental], then move the option to another section once mature, everyone using [experimental] in their hgrcs needs to take action (or we must keep the [experimental] version as alias). I know we don't offer BC guarantees for experimental and debug features, but I would sti
Re: Constant naming convention
On Tue, Dec 13, 2016 at 11:29 AM, Pierre-Yves David wrote: > > > On 12/12/2016 04:12 PM, Augie Fackler wrote: >> >> Can we just align with prevailing Python style here and start using >> all caps for constants? This comes up roughly monthly, and I think >> it'd be a good clarity win. > > > As much as there is many thing in our current coding still, it is our > current coding style and our codebase is consistent with it for the very > vast majority. Restated: the codebase is inconsistent. > As stated previously, my current stance is that we should > refrain for changing this category for policy for about one year after Matt > stepped away. To what end? (If I knew about this position of yours, I forgot about it, so we get to have the discussion as to why we'd have this policy now). > So I would like us to keep to lower case constants for now. > At the 4.4 sprint, I'll be very happy to have a large discussion about our > style (and very happy to us CAPS for constant at that time). There seems to be pretty strong consensus already that CAPS for constants is a standard thing, and it keeps tripping up both new and seasoned contributors. I'd much rather change this today. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: Constant naming convention
On 12/12/2016 04:12 PM, Augie Fackler wrote: On Mon, Dec 12, 2016 at 11:04:07AM +0100, Pierre-Yves David wrote: On 11/25/2016 04:28 AM, Gregory Szorc wrote: diff --git a/mercurial/revlog.py b/mercurial/revlog.py --- a/mercurial/revlog.py +++ b/mercurial/revlog.py @@ -1818,3 +1818,95 @@ class revlog(object): if not self._inline: res.append(self.datafile) return res + +DELTAREUSEALWAYS = 'always' +DELTAREUSESAMEREVS = 'samerevs' +DELTAREUSENEVER = 'never' We don't use cap in our names, even for constant, these should be lower case. Can we just align with prevailing Python style here and start using all caps for constants? This comes up roughly monthly, and I think it'd be a good clarity win. As much as there is many thing in our current coding still, it is our current coding style and our codebase is consistent with it for the very vast majority. As stated previously, my current stance is that we should refrain for changing this category for policy for about one year after Matt stepped away. So I would like us to keep to lower case constants for now. At the 4.4 sprint, I'll be very happy to have a large discussion about our style (and very happy to us CAPS for constant at that time). Cheers, -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] parsers: use buffer to store revlog index
On Tue, Dec 13, 2016 at 6:04 AM, Yuya Nishihara wrote: > On Mon, 12 Dec 2016 22:11:02 -0800, Gregory Szorc wrote: > > This patch LGTM. Only thing I would have done differently is store a > > Py_buffer* instead of a Py_buffer to ensure alignment of indexObject > struct > > entries. But I don't think this struct is that performance sensitive to > > care. Feel free to address in a follow-up if you disagree. > > Just curious. What kind of alignment issue would be solved by not storing > a Py_buffer struct? > Without looking at what is actually inside Py_buffer, I'm somewhat paranoid that struct members won't be aligned on word boundaries, creating members that span words, leading to sub-optimal access of highly-accessed members. I know Python is generally pretty good about struct alignment. And the compiler may not be "packing" structs unless ordered. So perhaps we're fine here? ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 3 of 7 upgraderepo V2] repair: determine what upgrade will do
On Mon, Dec 12, 2016 at 12:34:31PM +0100, Pierre-Yves David wrote: > > > On 11/25/2016 04:28 AM, Gregory Szorc wrote: > ># HG changeset patch > ># User Gregory Szorc > ># Date 1480035243 28800 > ># Thu Nov 24 16:54:03 2016 -0800 > ># Node ID 328d95ac1fd128359a2665f2959e030ee8cce046 > ># Parent 4fffbdc2bca22e666482b55562c0e592c8a68027 > >repair: determine what upgrade will do > > > >This commit is rather large. It started off as multiple commits > >but I couldn't figure out a good way to split up the functionality > >while still conveying the overall strategy. To the reviewer, I > >apologize. > > From a reviewer: this changeset is just too large It's much smaller than it looks - it's mostly configurational stuff that's easy to work through. I've provided Greg with some comments, and would welcome the change being resent without splitting it into tons of patches (which might be a lot of work for relatively little practical benefit.) > , I've currently too much > backlog on my plate to dive into such scary monster. I've looked into that > area myself and I don't see why we could not have something more iterative. > I could see two ways to go here, > > Proposal 1: detect all then implement all > > 1. detect possible upgrade A > 2. detect possible upgrade B > 3. detect possible upgrade C > 4. detect possible upgrade D > 5. implement upgrade A > 6. implement upgrade B > 7. implement upgrade C > 8. implement upgrade D > > Proposal 2: detect then implement iteratively > > 1. detect possible upgrade A > 2. implement upgrade A > 3. detect possible upgrade B > 4. implement upgrade B > 5. detect possible upgrade C > 6. implement upgrade C > 7. detect possible upgrade D > 8. implement upgrade D > > Can get your to submit a new series with smaller patches? Ping me on IRC if > you have extra questions. (I would not be surprised if we end with multiple > sub-series). I'd rather not split this series too far - it's a good conceptual unit right now. > > Cheers, > > -- > Pierre-Yves David > ___ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 3 of 7 upgraderepo V2] repair: determine what upgrade will do
On Thu, Nov 24, 2016 at 07:28:31PM -0800, Gregory Szorc wrote: > # HG changeset patch > # User Gregory Szorc > # Date 1480035243 28800 > # Thu Nov 24 16:54:03 2016 -0800 > # Node ID 328d95ac1fd128359a2665f2959e030ee8cce046 > # Parent 4fffbdc2bca22e666482b55562c0e592c8a68027 > repair: determine what upgrade will do > > This commit is rather large. It started off as multiple commits > but I couldn't figure out a good way to split up the functionality > while still conveying the overall strategy. To the reviewer, I > apologize. > > This commit introducts a ton of functionality around determining > how a repository can be upgraded. > > The first code you'll see is related to repository requirements and > updates. There are no less than 5 functions returnings sets of > requirements that control upgrading. Why so many functions? > Mainly extensions. These would ideally be module variables. But > extensions will need to inject themselves into the upgrade process. > And if you are going to make 1 set extensible, you might as well > make them all extensible. > > Astute readers will see that we don't support "manifestv2" and > "treemanifest" requirements in the upgrade mechanism. I don't have > a great answer for why other than this is a complex set of patches > and I don't want to deal with the complexity of these experimental > features just yet. We can teach the upgrade mechanism about them > later, once the basic upgrade mechanism is in place. > > The "upgradefindimprovements" function introduces a mechanism to > return a list of improvements that can be made to a repository. > Each improvement is effectively an action that an upgrade will > perform. Associated with each of these improvements is metadata > that will be used to inform users what's wrong and what an > upgrade will do. > > Each "improvement" is categorized as a "deficiency" or an > "optimization." TBH, I'm not thrilled about the terminology and > am receptive to constructive bikeshedding. The main difference > between a "deficiency" and an "optimization" is a deficiency > is always corrected (if it deviates from the current config) and > an "optimization" is an optional action that goes above and beyond > to improve the state of the repository (usually by requiring more > CPU during upgrade). > > Our initial set of improvements identifies missing repository > requirements, a single, easily correctable problem with > changelog storage, and a set of "optimizations" related to delta > recalculation. > > The main "upgraderepo" function glues all of this code together. > It verifies that the repository is capable of being upgraded by > looking at the state of current and proposed requirements and > every difference therein. It then queries for the list of > improvements and determines which of them will run based on the > current repository state and user arguments. A non-trivial amount > of code is required to print all the improvements and what > actions will be taken by an upgrade. Some of this is factored into > inline functions to facilitate use in a subsequent commit. > > I went through numerous iterations of the output format before > settling on a ReST-inspired definition list format. (I used > bulleted lists in the first submission of this commit and could > not get it to format just right.) Even with the various iterations, > I'm still not super thrilled with the format. But, this is a debug* > command, so that should mean we can refine the output without BC > concerns. > > Rounding out the code is a series of new tests. > > diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py > --- a/mercurial/debugcommands.py > +++ b/mercurial/debugcommands.py > @@ -34,6 +34,7 @@ from . import ( > localrepo, > lock as lockmod, > pycompat, > +repair, > revlog, > scmutil, > setdiscovery, > @@ -873,4 +874,4 @@ def debugupgraderepo(ui, repo, run=False > should complete almost instantaneously and the chances of a consumer > being > unable to access the repository should be low. > """ > -raise error.Abort(_('not yet implemented')) > +return repair.upgraderepo(ui, repo, run=run, optimize=optimize) > diff --git a/mercurial/repair.py b/mercurial/repair.py > --- a/mercurial/repair.py > +++ b/mercurial/repair.py > @@ -360,3 +360,417 @@ def deleteobsmarkers(obsstore, indices): > newobsstorefile.write(bytes) > newobsstorefile.close() > return n [...] > +def upgradefindimprovements(repo): > +"""Determine improvements that can be made to the repo during upgrade. > + > +Returns a list of dicts describing repository deficiencies and > +optimizations. Each dict value has the following keys: Can we use namedtuples instead for this? > + > +name > + Machine-readable string uniquely identifying this improvement. It > + will be mapped to an action later in the upgrade process. > + > +type > + Either ``deficiency`` or ``optimization``. A defi
Re: [PATCH 2 of 7 upgraderepo V2] debugcommands: stub for debugupgraderepo command
On Mon, Dec 12, 2016 at 09:18:51PM -0800, Gregory Szorc wrote: > On Mon, Dec 12, 2016 at 2:17 AM, Pierre-Yves David < > pierre-yves.da...@ens-lyon.org> wrote: > > > > > > > On 11/25/2016 04:28 AM, Gregory Szorc wrote: > > > >> # HG changeset patch > >> # User Gregory Szorc > >> # Date 1480033449 28800 > >> # Thu Nov 24 16:24:09 2016 -0800 > >> # Node ID 4fffbdc2bca22e666482b55562c0e592c8a68027 > >> # Parent c60995fce14b0e34bd1416dab3712a6c33bb29bb > >> debugcommands: stub for debugupgraderepo command > >> > >> Currently, if Mercurial introduces a new repository/store feature or > >> changes behavior of an existing feature, users must perform an > >> `hg clone` to create a new repository with hopefully the > >> correct/optimal settings. Unfortunately, even `hg clone` may not > >> give the correct results. For example, if you do a local `hg clone`, > >> you may get hardlinks to revlog files that inherit the old state. > >> If you `hg clone` from a remote or `hg clone --pull`, changegroup > >> application may bypass some optimization, such as converting to > >> generaldelta. > >> > >> Optimizing a repository is harder than it seems and requires more > >> than a simple `hg` command invocation. > >> > >> This commit starts the process of changing that. We introduce > >> `hg debugupgraderepo`, a command that performs an in-place upgrade > >> of a repository to use new, optimal features. The command is just > >> a stub right now. Features will be added in subsequent commits. > >> > > > > I like the idea of this commit, this should probably be the first one in > > the stack. The revlog cloning feature can be delayed later in the stack > > until we actually start performing upgrade. > > > > This commit does foreshadow some of the behavior of the new command, > >> notably that it doesn't do anything by default and that it takes > >> arguments that influence what actions it performs. These will be > >> explained more in subsequent commits. > >> > >> diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py > >> --- a/mercurial/debugcommands.py > >> +++ b/mercurial/debugcommands.py > >> @@ -849,3 +849,28 @@ def debugdeltachain(ui, repo, file_=None > >> extradist=extradist, extraratio=extraratio) > >> > >> fm.end() > >> + > >> +@command('debugupgraderepo', [ > >> +('o', 'optimize', [], _('extra optimization to perform'), _('NAME')), > >> +('', 'run', False, _('performs an upgrade')), > >> > > > > We should drop these option from the command until we actually use them > > this will allow to discuss them with their actual behavior. > > > > +]) > >> +def debugupgraderepo(ui, repo, run=False, optimize=None): > >> +"""upgrade a repository to use different features > >> + > >> +If no arguments are specified, the repository is evaluated for > >> upgrade > >> +and a list of problems and potential optimizations is printed. > >> + > >> +With ``--run``, a repository upgrade is performed. Behavior of the > >> upgrade > >> +can be influenced via additional arguments. More details will be > >> provided > >> +by the command output when run without ``--run``. > >> > > > > (I would also delayed the help about --run until we actually run something) > > > > + > >> +During the upgrade, the repository will be locked and no writes will > >> be > >> +allowed. > >> + > >> +At the end of the upgrade, the repository may not be readable while > >> new > >> +repository data is swapped in. This window will be as long as it > >> takes to > >> +rename some directories inside the ``.hg`` directory. On most > >> machines, this > >> +should complete almost instantaneously and the chances of a consumer > >> being > >> +unable to access the repository should be low. > >> > > > > (I would also delay that doc until we actually behave that way) > > I know it would bot solve "everything" but we discussed the option of > > having a "repositorybeingupgraded" (or similar) in .hg/required to reduce > > the change for a reader to access the repository at that time. > > > > I added all this code and documentation to foreshadow the feature so a > reviewer isn't guessing where things are going. It also cuts down on the > amount of patches and/or churn in future patches, making code review > easier, IMO. I think it is busy work to change it at this stage. But if you > insist, I will. I liked having the documentation early in the stack, for what it's worth. > > > > > > +""" > >> +raise error.Abort(_('not yet implemented')) > >> diff --git a/tests/test-completion.t b/tests/test-completion.t > >> --- a/tests/test-completion.t > >> +++ b/tests/test-completion.t > >> @@ -109,6 +109,7 @@ Show debug commands if there are no othe > >>debugsub > >>debugsuccessorssets > >>debugtemplate > >> + debugupgraderepo > >>debugwalk > >>debugwireargs > >> > >> @@ -274,6 +275,7 @@ Show all commands + options > >>debugsub: rev > >>debugsuccessorssets:
[PATCH V6] py3: make keys of keyword arguments strings
# HG changeset patch # User Pulkit Goyal <7895pul...@gmail.com> # Date 1481642620 -19800 # Tue Dec 13 20:53:40 2016 +0530 # Node ID 3728b3d6924f6d4c92a9f7673a792805e8ba3c4b # Parent 95b076f5ddee5ab7bd208b8c2f42121ce9907129 py3: make keys of keyword arguments strings keys of keyword arguments on Python 3 has to be string. We are dealing with bytes in our codebase so the keys are also bytes. Done that using pycompat.strkwargs(). Also after this patch, `hg version` now runs on Python 3.5. Hurray! diff -r 95b076f5ddee -r 3728b3d6924f mercurial/dispatch.py --- a/mercurial/dispatch.py Mon Nov 28 05:45:22 2016 + +++ b/mercurial/dispatch.py Tue Dec 13 20:53:40 2016 +0530 @@ -803,7 +803,8 @@ msg = ' '.join(' ' in a and repr(a) or a for a in fullargs) ui.log("command", '%s\n', msg) -d = lambda: util.checksignature(func)(ui, *args, **cmdoptions) +strcmdopt = pycompat.strkwargs(cmdoptions) +d = lambda: util.checksignature(func)(ui, *args, **strcmdopt) try: return runcommand(lui, repo, cmd, fullargs, ui, options, d, cmdpats, cmdoptions) diff -r 95b076f5ddee -r 3728b3d6924f tests/test-check-py3-commands.t --- a/tests/test-check-py3-commands.t Mon Nov 28 05:45:22 2016 + +++ b/tests/test-check-py3-commands.t Tue Dec 13 20:53:40 2016 +0530 @@ -9,6 +9,6 @@ > $PYTHON3 `which hg` $cmd 2>&1 2>&1 | tail -1 > done version - TypeError: Can't convert 'bytes' object to str implicitly + warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. debuginstall TypeError: Can't convert 'bytes' object to str implicitly ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH] revlog: merge hash checking subfunctions
# HG changeset patch # User Remi Chaintron # Date 1481638896 0 # Tue Dec 13 14:21:36 2016 + # Branch stable # Node ID d34c04da52a8f2eb61b51bb0aa9cedf1e4b43474 # Parent 7817df5585db1d87d3f6c7f496085c86d87e2e9a revlog: merge hash checking subfunctions This patch factors the behavior of both methods into 'checkhash'. diff --git a/contrib/perf.py b/contrib/perf.py --- a/contrib/perf.py +++ b/contrib/perf.py @@ -861,7 +861,7 @@ def dohash(text): if not cache: r.clearcaches() -r._checkhash(text, node, rev) +r.checkhash(text, node, rev=rev) def dorevision(): if not cache: diff --git a/mercurial/bundlerepo.py b/mercurial/bundlerepo.py --- a/mercurial/bundlerepo.py +++ b/mercurial/bundlerepo.py @@ -147,7 +147,7 @@ delta = self._chunk(chain.pop()) text = mdiff.patches(text, [delta]) -self._checkhash(text, node, rev) +self.checkhash(text, node, rev=rev) self._cache = (node, rev, text) return text diff --git a/mercurial/filelog.py b/mercurial/filelog.py --- a/mercurial/filelog.py +++ b/mercurial/filelog.py @@ -104,9 +104,9 @@ return True -def checkhash(self, text, p1, p2, node, rev=None): +def checkhash(self, text, node, p1=None, p2=None, rev=None): try: -super(filelog, self).checkhash(text, p1, p2, node, rev=rev) +super(filelog, self).checkhash(text, node, p1=p1, p2=p2, rev=rev) except error.RevlogError: if _censoredtext(text): raise error.CensoredNodeError(self.indexfile, node, text) diff --git a/mercurial/revlog.py b/mercurial/revlog.py --- a/mercurial/revlog.py +++ b/mercurial/revlog.py @@ -1221,9 +1221,7 @@ bins = bins[1:] text = mdiff.patches(text, bins) - -text = self._checkhash(text, node, rev) - +self.checkhash(text, node, rev=rev) self._cache = (node, rev, text) return text @@ -1235,12 +1233,14 @@ """ return hash(text, p1, p2) -def _checkhash(self, text, node, rev): -p1, p2 = self.parents(node) -self.checkhash(text, p1, p2, node, rev) -return text +def checkhash(self, text, node, p1=None, p2=None, rev=None): +"""Check node hash integrity. -def checkhash(self, text, p1, p2, node, rev=None): +Available as a function so that subclasses can extend hash mismatch +behaviors as needed. +""" +if p1 is None and p2 is None: +p1, p2 = self.parents(node) if node != self.hash(text, p1, p2): revornode = rev if revornode is None: @@ -1415,7 +1415,7 @@ basetext = self.revision(self.node(baserev), _df=fh) btext[0] = mdiff.patch(basetext, delta) try: -self.checkhash(btext[0], p1, p2, node) +self.checkhash(btext[0], node, p1=p1, p2=p2) if flags & REVIDX_ISCENSORED: raise RevlogError(_('node %s is not censored') % node) except CensoredNodeError: ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] parsers: use buffer to store revlog index
On Mon, 12 Dec 2016 22:11:02 -0800, Gregory Szorc wrote: > This patch LGTM. Only thing I would have done differently is store a > Py_buffer* instead of a Py_buffer to ensure alignment of indexObject struct > entries. But I don't think this struct is that performance sensitive to > care. Feel free to address in a follow-up if you disagree. Just curious. What kind of alignment issue would be solved by not storing a Py_buffer struct? ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: Constant naming convention (was: Re: [PATCH 1 of 7 upgraderepo V2] revlog: add clone method)
On Mon, 12 Dec 2016 10:12:48 -0500, Augie Fackler wrote: > On Mon, Dec 12, 2016 at 11:04:07AM +0100, Pierre-Yves David wrote: > > > > > > On 11/25/2016 04:28 AM, Gregory Szorc wrote: > > >diff --git a/mercurial/revlog.py b/mercurial/revlog.py > > >--- a/mercurial/revlog.py > > >+++ b/mercurial/revlog.py > > >@@ -1818,3 +1818,95 @@ class revlog(object): > > > if not self._inline: > > > res.append(self.datafile) > > > return res > > >+ > > >+DELTAREUSEALWAYS = 'always' > > >+DELTAREUSESAMEREVS = 'samerevs' > > >+DELTAREUSENEVER = 'never' > > > > We don't use cap in our names, even for constant, these should be lower > > case. > > Can we just align with prevailing Python style here and start using > all caps for constants? This comes up roughly monthly, and I think > it'd be a good clarity win. I'm fine with ALLCAPS constants. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH v2] graft: support grafting changes to new file in renamed directory (issue5436)
On Tue, 06 Dec 2016 18:09:40 +0100, Gábor Stefanik wrote: > # HG changeset patch > # User Gábor Stefanik > # Date 1480956001 -3600 > # Mon Dec 05 17:40:01 2016 +0100 > # Node ID 6472c33e16326b8c817a8bae0e75053b19badb2c > # Parent cbeb54ec0481a4bf9723ba4b80a5861a813c8531 > graft: support grafting changes to new file in renamed directory (issue5436) No problem spotted by comparing to the other "move" handling, so queued, thanks. > --- a/tests/test-graft.t Wed Nov 30 19:23:04 2016 + > +++ b/tests/test-graft.t Mon Dec 05 17:40:01 2016 +0100 > @@ -1286,3 +1286,22 @@ >$ hg ci -qAmc >$ hg up -q .~2 >$ hg graft tip -qt:fail Inserted "cd .." here. > +Graft a change into a new file previously grafted into a renamed directory > + > + $ hg init dirmovenewfile > + $ cd dirmovenewfile > + $ mkdir a > + $ echo a > a/a > + $ hg ci -qAma > + $ echo x > a/x > + $ hg ci -qAmx > + $ hg up -q 0 > + $ hg mv -q a b > + $ hg ci -qAmb > + $ hg graft -q 1 # a/x grafted as b/x, but no copy information recorded > + $ hg up -q 1 > + $ echo y > a/x > + $ hg ci -qAmy > + $ hg up -q 3 > + $ hg graft -q 4 and added "hg status --change ." to make sure b/x is modified by the grafted changeset. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel