Patchwork [1,of,4] bundlerepo: update unlink in getremotechanges to use vfs

login
register
mail settings
Submitter Chinmay Joshi
Date May 27, 2014, 6:58 p.m.
Message ID <5dc8d26e1da18dfb8252.1401217080@ubuntu>
Download mbox | patch
Permalink /patch/4868/
State Accepted
Headers show

Comments

Chinmay Joshi - May 27, 2014, 6:58 p.m.
# HG changeset patch
# User Chinmay Joshi <c@chinmayjoshi.com>
# Date 1401207963 -19800
#      Tue May 27 21:56:03 2014 +0530
# Node ID 5dc8d26e1da18dfb8252115ac306d7f8fdd25582
# Parent  bee0e1cffdd36aab975090f1b51b81b6b14790fa
bundlerepo: update unlink in getremotechanges to use vfs

As per WindowsUTF8 plan, unlink from getremotechanges is updated to use vfs in this change.
Katsunori FUJIWARA - May 27, 2014, 11:22 p.m.
At Wed, 28 May 2014 00:28:00 +0530,
Chinmay Joshi wrote:
> 
> # HG changeset patch
> # User Chinmay Joshi <c@chinmayjoshi.com>
> # Date 1401207963 -19800
> #      Tue May 27 21:56:03 2014 +0530
> # Node ID 5dc8d26e1da18dfb8252115ac306d7f8fdd25582
> # Parent  bee0e1cffdd36aab975090f1b51b81b6b14790fa
> bundlerepo: update unlink in getremotechanges to use vfs
> 
> As per WindowsUTF8 plan, unlink from getremotechanges is updated to use vfs in this change.
> 
> diff --git a/mercurial/bundlerepo.py b/mercurial/bundlerepo.py
> --- a/mercurial/bundlerepo.py
> +++ b/mercurial/bundlerepo.py
> @@ -352,7 +352,7 @@
>      if not incoming:
>          try:
>              if bundlename:
> -                os.unlink(bundlename)
> +                repo.vfs.unlink(bundlename)
>          except OSError:
>              pass
>          return repo, [], other.close
> @@ -394,7 +394,7 @@
>          if bundlerepo:
>              bundlerepo.close()
>          if bundle:
> -            os.unlink(bundle)
> +            repo.vfs.unlink(bundle)
>          other.close()
>  
>      return (localrepo, csets, cleanup)

This patch should be backed out, because "bundlename" and "bundle" in
this case are not relative paths to the root of repositories.

The former is specified via "hg incoming --bundle BUNDLENAME"
(relative path to cwd, or absolute one), and the latter is generated
in "changegroup.writebundle" by "tempfile.mkstemp" for internal
temporary usage (absolute path).

To be exact, the latter hunk in this patch can be applied, because
"os.join" for two absolute paths can generate correct result. But the
former hunk can't, because it may unexpected result, if specified path
is relative to cwd and cwd != root.

This is also because that I haven't applied "vfs" migration for these
code paths, and "mercurial/bundlerepo.py" is marked as "finished but
some are still used for ABS paths" in WindowsUTF8Plan.

    http://mercurial.selenic.com/wiki/WindowsUTF8Plan#Status_of_each_files

(I'll revise "for ABS paths" into "cwd relative paths" or so :-))


Should this detail be described in "bundlerepo.py" as comment, too ?

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp

Patch

diff --git a/mercurial/bundlerepo.py b/mercurial/bundlerepo.py
--- a/mercurial/bundlerepo.py
+++ b/mercurial/bundlerepo.py
@@ -352,7 +352,7 @@ 
     if not incoming:
         try:
             if bundlename:
-                os.unlink(bundlename)
+                repo.vfs.unlink(bundlename)
         except OSError:
             pass
         return repo, [], other.close
@@ -394,7 +394,7 @@ 
         if bundlerepo:
             bundlerepo.close()
         if bundle:
-            os.unlink(bundle)
+            repo.vfs.unlink(bundle)
         other.close()
 
     return (localrepo, csets, cleanup)