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

[FUJIWARA Katsunori]

At Wed, 28 May 2014 08:22:40 +0900,
FUJIWARA Katsunori wrote:
> 
> At Wed, 28 May 2014 00:28:00 +0530,
> Chinmay Joshi wrote:
> > 
> > # HG changeset patch
> > # User Chinmay Joshi <c at 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.
                         ^^^^^^^^^^^ "cwd != root/.hg"


Below patch for "test-incoming-outgoing.t" can detect this problem:

====================
diff -r 764b691b8bda tests/test-incoming-outgoing.t
--- a/tests/test-incoming-outgoing.t    Tue May 27 23:02:05 2014 +0530
+++ b/tests/test-incoming-outgoing.t    Wed May 28 09:14:11 2014 +0900
@@ -484,8 +484,14 @@ incoming from empty remote repository
   $ echo a > r1/foo
   $ hg -R r1 ci -Ama
   adding foo
+  $ touch x.hg
   $ hg -R r1 incoming r2 --bundle x.hg
   comparing with r2
   searching for changes
   no changes found
   [1]
+
+"./x.hg" should be unlinked, if there is no incoming changeset.
+
+  $ test -f x.hg
+  [1]
====================

Recent implementation can't unlink target bundle file, if there is no
incoming changeset: getremotechanges tries to unlink
"$TESTTMP/r1/.hg/x.hg" and avoids its failure.


> 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 at lares.dti.ne.jp
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
> 

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