Patchwork memctx: add a version field for filectxfn

login
register
mail settings
Submitter Siddharth Agarwal
Date Aug. 30, 2014, 12:33 p.m.
Message ID <9d6802afa661cf0ba864.1409401991@devbig136.prn2.facebook.com>
Download mbox | patch
Permalink /patch/5630/
State Accepted
Headers show

Comments

Siddharth Agarwal - Aug. 30, 2014, 12:33 p.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1409401778 25200
#      Sat Aug 30 05:29:38 2014 -0700
# Node ID 9d6802afa661cf0ba864ef0618c69dc3cead68c9
# Parent  bdc0e04df243d3995c7266bf7d138fddd0449ba6
memctx: add a version field for filectxfn

Rev 650b5b6e75ed switched the contract for filectxfn from "raise IOError if
file is missing" to "return None if file is missing". Out of tree extensions
need to be updated for that, but for extensions interested in compatibility
with both Mercurial <= 3.1 and default, it is next to impossible to introspect
core Mercurial to figure out what to do.

This patch adds a version field for extensions to look at.
Sean Farley - Aug. 30, 2014, 12:43 p.m.
Siddharth Agarwal writes:

> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1409401778 25200
> #      Sat Aug 30 05:29:38 2014 -0700
> # Node ID 9d6802afa661cf0ba864ef0618c69dc3cead68c9
> # Parent  bdc0e04df243d3995c7266bf7d138fddd0449ba6
> memctx: add a version field for filectxfn
>
> Rev 650b5b6e75ed switched the contract for filectxfn from "raise IOError if
> file is missing" to "return None if file is missing". Out of tree extensions
> need to be updated for that, but for extensions interested in compatibility
> with both Mercurial <= 3.1 and default, it is next to impossible to introspect
> core Mercurial to figure out what to do.
>
> This patch adds a version field for extensions to look at.
>
> diff --git a/mercurial/context.py b/mercurial/context.py
> --- a/mercurial/context.py
> +++ b/mercurial/context.py
> @@ -1593,6 +1593,13 @@
>      supported by util.parsedate() and defaults to current date, extra
>      is a dictionary of metadata or is left empty.
>      """
> +
> +    # This allows extensions to obey the current filectxfn contract.
> +    # Undefined ("version 0"): filectxfn should raise IOError if the file isn't
> +    #                          present. For Mercurial <= 3.1.
> +    # Version 1: filectxfn should return None if the file isn't present.
> +    filectxfnversion = 1
> +

Wha? Just make all extensions that use filectxfn update to the newest
version of Mercurial. Problem solved!
Matt Mackall - Aug. 30, 2014, 12:46 p.m.
On Sat, 2014-08-30 at 05:33 -0700, Siddharth Agarwal wrote:
> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1409401778 25200
> #      Sat Aug 30 05:29:38 2014 -0700
> # Node ID 9d6802afa661cf0ba864ef0618c69dc3cead68c9
> # Parent  bdc0e04df243d3995c7266bf7d138fddd0449ba6
> memctx: add a version field for filectxfn
> 
> Rev 650b5b6e75ed switched the contract for filectxfn from "raise IOError if
> file is missing" to "return None if file is missing". Out of tree extensions
> need to be updated for that, but for extensions interested in compatibility
> with both Mercurial <= 3.1 and default, it is next to impossible to introspect
> core Mercurial to figure out what to do.
> 
> This patch adds a version field for extensions to look at.
> 
> diff --git a/mercurial/context.py b/mercurial/context.py
> --- a/mercurial/context.py
> +++ b/mercurial/context.py
> @@ -1593,6 +1593,13 @@
>      supported by util.parsedate() and defaults to current date, extra
>      is a dictionary of metadata or is left empty.
>      """
> +
> +    # This allows extensions to obey the current filectxfn contract.
> +    # Undefined ("version 0"): filectxfn should raise IOError if the file isn't
> +    #                          present. For Mercurial <= 3.1.
> +    # Version 1: filectxfn should return None if the file isn't present.
> +    filectxfnversion = 1

Sigh.. I guess.

I'd prefer this to be a feature flag and to add an underbar. For
instance:

  if ctx.get("_emptyreturnsnone"):
    do new thing
  else:
    do old thing

It might be better to stick this right next to the definition of the
function it's "documenting" too.

Patch

diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -1593,6 +1593,13 @@ 
     supported by util.parsedate() and defaults to current date, extra
     is a dictionary of metadata or is left empty.
     """
+
+    # This allows extensions to obey the current filectxfn contract.
+    # Undefined ("version 0"): filectxfn should raise IOError if the file isn't
+    #                          present. For Mercurial <= 3.1.
+    # Version 1: filectxfn should return None if the file isn't present.
+    filectxfnversion = 1
+
     def __init__(self, repo, parents, text, files, filectxfn, user=None,
                  date=None, extra=None, editor=False):
         super(memctx, self).__init__(repo, text, user, date, extra)