| Submitter | Katsunori FUJIWARA |
|---|---|
| Date | May 28, 2014, 12:25 a.m. |
| Message ID | <ubnuivjex.wl%foozy@lares.dti.ne.jp> |
| Download | mbox | patch |
| Permalink | /patch/4872/ |
| State | Superseded |
| Headers | show |
Comments
On Wed, May 28, 2014 at 5:55 AM, FUJIWARA Katsunori <foozy@lares.dti.ne.jp> wrote: > > 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@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" > Thanks for illuminating me. I was confused in cwd and repository related paths. I agree that this needed to be backed out. This is already backed out for good. > > 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 :-)) >> Thanks for creating this wiki page for providing up to date information for project. I will keep it updated for future use of functions. >> >> Should this detail be described in "bundlerepo.py" as comment, too ? >> >> ---------------------------------------------------------------------- >> [FUJIWARA Katsunori] foozy@lares.dti.ne.jp >> _______________________________________________ >> Mercurial-devel mailing list >> Mercurial-devel@selenic.com >> http://selenic.com/mailman/listinfo/mercurial-devel >> > > ---------------------------------------------------------------------- > [FUJIWARA Katsunori] foozy@lares.dti.ne.jp
Patch
==================== 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]