Patchwork [1,of,2] archive: change the default prefix to '' from None

login
register
mail settings
Submitter Matt Harbison
Date Feb. 28, 2015, 5:47 a.m.
Message ID <d36fb8953555e4e0c784.1425102438@Envy>
Download mbox | patch
Permalink /patch/7859/
State Accepted
Headers show

Comments

Matt Harbison - Feb. 28, 2015, 5:47 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1424038908 18000
#      Sun Feb 15 17:21:48 2015 -0500
# Node ID d36fb8953555e4e0c7849b1f9e7b61a31e790c28
# Parent  7a21944731557cd18dc5c53969318d80ec547e2d
archive: change the default prefix to '' from None

All current callers supply some sort of prefix, so the issue was hidden.  But if
no parameter was specified, a crash occurred in the write() closure when
concatenating 'prefix' and 'name'.
Mathias De Maré - Feb. 28, 2015, 7:44 a.m.
On Sat, Feb 28, 2015 at 6:47 AM, Matt Harbison <mharbison72@gmail.com>
wrote:

> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1424038908 18000
> #      Sun Feb 15 17:21:48 2015 -0500
> # Node ID d36fb8953555e4e0c7849b1f9e7b61a31e790c28
> # Parent  7a21944731557cd18dc5c53969318d80ec547e2d
> archive: change the default prefix to '' from None
>
> All current callers supply some sort of prefix, so the issue was hidden.
> But if
> no parameter was specified, a crash occurred in the write() closure when
> concatenating 'prefix' and 'name'.
>
Is this the crash we encountered when trying out the extdiff + archive
changes?
Looks good to me.

Greetings,
Mathias

>
> diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
> --- a/hgext/largefiles/overrides.py
> +++ b/hgext/largefiles/overrides.py
> @@ -858,7 +858,7 @@
>          repo._lfcommithooks.pop()
>
>  def overridearchive(orig, repo, dest, node, kind, decode=True,
> matchfn=None,
> -            prefix=None, mtime=None, subrepos=None):
> +            prefix='', mtime=None, subrepos=None):
>      # No need to lock because we are only reading history and
>      # largefile caches, neither of which are modified.
>      lfcommands.cachelfiles(repo.ui, repo, node)
> diff --git a/mercurial/archival.py b/mercurial/archival.py
> --- a/mercurial/archival.py
> +++ b/mercurial/archival.py
> @@ -230,7 +230,7 @@
>      }
>
>  def archive(repo, dest, node, kind, decode=True, matchfn=None,
> -            prefix=None, mtime=None, subrepos=False):
> +            prefix='', mtime=None, subrepos=False):
>      '''create archive of repo as it was at node.
>
>      dest can be name of directory, name of archive file, or file
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
Matt Harbison - Feb. 28, 2015, 5:39 p.m.
On Sat, 28 Feb 2015 02:44:30 -0500, Mathias De Maré  
<mathias.demare@gmail.com> wrote:

> On Sat, Feb 28, 2015 at 6:47 AM, Matt Harbison <mharbison72@gmail.com>
> wrote:
>
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison@yahoo.com>
>> # Date 1424038908 18000
>> #      Sun Feb 15 17:21:48 2015 -0500
>> # Node ID d36fb8953555e4e0c7849b1f9e7b61a31e790c28
>> # Parent  7a21944731557cd18dc5c53969318d80ec547e2d
>> archive: change the default prefix to '' from None
>>
>> All current callers supply some sort of prefix, so the issue was hidden.
>> But if
>> no parameter was specified, a crash occurred in the write() closure when
>> concatenating 'prefix' and 'name'.
>>
> Is this the crash we encountered when trying out the extdiff + archive
> changes?
> Looks good to me.
>
> Greetings,
> Mathias

Yep.  Might as well get the easy stuff out of the way first.

--Matt

>>
>> diff --git a/hgext/largefiles/overrides.py  
>> b/hgext/largefiles/overrides.py
>> --- a/hgext/largefiles/overrides.py
>> +++ b/hgext/largefiles/overrides.py
>> @@ -858,7 +858,7 @@
>>          repo._lfcommithooks.pop()
>>
>>  def overridearchive(orig, repo, dest, node, kind, decode=True,
>> matchfn=None,
>> -            prefix=None, mtime=None, subrepos=None):
>> +            prefix='', mtime=None, subrepos=None):
>>      # No need to lock because we are only reading history and
>>      # largefile caches, neither of which are modified.
>>      lfcommands.cachelfiles(repo.ui, repo, node)
>> diff --git a/mercurial/archival.py b/mercurial/archival.py
>> --- a/mercurial/archival.py
>> +++ b/mercurial/archival.py
>> @@ -230,7 +230,7 @@
>>      }
>>
>>  def archive(repo, dest, node, kind, decode=True, matchfn=None,
>> -            prefix=None, mtime=None, subrepos=False):
>> +            prefix='', mtime=None, subrepos=False):
>>      '''create archive of repo as it was at node.
>>
>>      dest can be name of directory, name of archive file, or file
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@selenic.com
>> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
--- a/hgext/largefiles/overrides.py
+++ b/hgext/largefiles/overrides.py
@@ -858,7 +858,7 @@ 
         repo._lfcommithooks.pop()
 
 def overridearchive(orig, repo, dest, node, kind, decode=True, matchfn=None,
-            prefix=None, mtime=None, subrepos=None):
+            prefix='', mtime=None, subrepos=None):
     # No need to lock because we are only reading history and
     # largefile caches, neither of which are modified.
     lfcommands.cachelfiles(repo.ui, repo, node)
diff --git a/mercurial/archival.py b/mercurial/archival.py
--- a/mercurial/archival.py
+++ b/mercurial/archival.py
@@ -230,7 +230,7 @@ 
     }
 
 def archive(repo, dest, node, kind, decode=True, matchfn=None,
-            prefix=None, mtime=None, subrepos=False):
+            prefix='', mtime=None, subrepos=False):
     '''create archive of repo as it was at node.
 
     dest can be name of directory, name of archive file, or file