Bug 2528 - hg clone with target directory as empty string trips recursion limit
Summary: hg clone with target directory as empty string trips recursion limit
Status: RESOLVED ARCHIVED
Alias: None
Product: Mercurial
Classification: Unclassified
Component: Mercurial (show other bugs)
Version: unspecified
Hardware: All All
: normal bug
Assignee: Bugzilla
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-11-29 23:56 UTC by Matthew Fernandez
Modified: 2014-07-31 13:22 UTC (History)
4 users (show)

See Also:
Python Version: ---


Attachments
(34 bytes, application/x-gzip)
2010-11-29 23:56 UTC, Matthew Fernandez
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Matthew Fernandez 2010-11-29 23:56 UTC
hg clone MY_REPO "" fails with the python errors attached. Obviously it
doesn't make sense to clone to a blank target directory, but a sensible
error message would be nice.

hg --version : 1.7.1
uname -r : 2.6.35-23-generic
python --version : 2.6.6
tail -4 ~/.hgrc :
[extensions]
rebase = 
mq = 
color =
Comment 1 Nicolas Dumazet 2010-11-30 02:51 UTC
yep.

It only happens if source repository is not copiable. A nice way to
reproduce is to try to clone from a bundle.

Following patch can fix makedirs:

# HG changeset patch
# User Nicolas Dumazet <nicdumz.commits@gmail.com>
# Date 1291106892 -32400
# Branch stable
# Node ID f4852cdaec01c70005b893e4fc087188d67690c1
# Parent  417f3c27983bcb6ab95b169801edc63c5c138dcf
util: do not recurse in makedirs if name is '' (issue2528)

diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -844,7 +844,7 @@
     except OSError, err:
         if err.errno == errno.EEXIST:
             return
-        if err.errno != errno.ENOENT:
+        if not name or err.errno != errno.ENOENT:
             raise
     parent = os.path.abspath(os.path.dirname(name))
     makedirs(parent, mode)
diff --git a/tests/test-bundle.t b/tests/test-bundle.t
--- a/tests/test-bundle.t
+++ b/tests/test-bundle.t
@@ -399,6 +399,13 @@
   
   $ rm -r full-clone
 
+When cloning from a non-copiable repository into '', do not
+recurse infinitely (issue 2528)
+
+  $ hg clone full.hg ''
+  abort: No such file or directory
+  [255]
+
 test for http://mercurial.selenic.com/bts/issue216
 
 Unbundle incremental bundles into fresh empty in one go




But in reality, we might run into troubles in other areas:
os.path.realpath('') is equivalent to os.path.realpath('.'):
localrepo(path='', create=False) will work on ./.hg/ ... nasty!
Comment 2 Matthew Fernandez 2010-11-30 16:26 UTC
Hmm... what do you mean by "not copiable"? The repo is over ssh so I can't
directly cp it, but I can scp it and hg clone it.

The most sensible solution seems to me to fail consistently, as you're
effectively asking hg to clone into a directory with no name. It seems OK to
clone to ./ though (as if you had not passed a third argument).
Comment 3 HG Bot 2010-12-01 19:00 UTC
Fixed by http://selenic.com/repo/hg/rev/2649be11ab0b
Nicolas Dumazet <nicdumz.commits@gmail.com>
util: do not recurse in makedirs if name is '' (issue2528)

(please test the fix)
Comment 4 Matthew Fernandez 2010-12-01 21:52 UTC
Sorry, I thought the patch had already been reviewed and tested. It does fix
the problem, but "No such file or directory" isn't really the correct error
msg. "hg clone MY_REPO local_dir" should work fine as long as local_dir
*doesn't* exist.
Comment 5 Nicolas Dumazet 2010-12-01 22:40 UTC
hgbot is only a robot, updating issues after pushes, no worries ;)

In my opinion '' is not a valid directory: in reality I can't think of a
straightforward meaning of '' as a directory specification. But we're still
discussing it and how to handle this... Opinions may vary and I should not
be the only one to decide.

There are a lot of places in code where passing '' doesn't really make
sense. For instance, if path is everything but '', makedirs works perfectly.
os.path functions handle it with different standards... 
Maybe some toplevel code could replace '' by '.' after a clear user warning?


In the mean time, aborting is not perfect/could be better, but it is still
better than a nasty opaque infinite recursion ;)
Comment 6 Matthew Fernandez 2010-12-01 22:50 UTC
Yes, I agree it's better than infinite recursion. The only reason I marked
it as cbb was that, purely from a user's perspective, "abort: No such file
or directory" doesn't really indicate what was wrong with the clone they
just attempted. The problem isn't that the path they passed doesn't exist,
but rather that they passed an illegal path.

I agree though that it's ambiguous what the user's intention was in passing "".
Comment 7 Bugzilla 2012-05-12 09:15 UTC

--- Bug imported by bugzilla@serpentine.com 2012-05-12 09:15 EDT  ---

This bug was previously known as _bug_ 2528 at http://mercurial.selenic.com/bts/issue2528
Imported an attachment (id=1487)
Comment 8 Matt Mackall 2014-07-25 17:22 UTC
Bulk close: no activity for >2 years -> WONTFIX
Comment 9 Matt Mackall 2014-07-31 13:22 UTC
Bulk change recent WONTFIX -> new, more descriptive ARCHIVED state (sorry for the spam)