568990 - Cloning a repository fails because older versions of Mercurial can't clone into an existent folder

Status: VERIFIED FIXED

(Mozilla QA Graveyard :: Mozmill Automation, defect)

Description

[Marc-Aurèle DARCHE]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; fr; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3
Build Identifier: 

$ ./testrun_addons.py --with-untrusted ~/opt/firefox-3.6/firefox
*** Cloning repository to '/tmp/tmpBSvROE.mozmill-tests'
Traceback (most recent call last):
  File "./testrun_addons.py", line 78, in <module>
    main()
  File "./testrun_addons.py", line 74, in main
    addons.run()
  File "/home/firefox/mozmill-automation/libs/testrun.py", line 161, in run
    self.clone_repository()
  File "/home/firefox/mozmill-automation/libs/testrun.py", line 111, in clone_repository
    e.message)
Exception: Failure in setting up the mozmill-tests repository. destination '/tmp/tmpBSvROE.mozmill-tests' already exists

My platform is:
Debian GNU/Linux 5.0.4 (lenny)
Python 2.5.2


Reproducible: Always

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

This is a strange failure which I can't explain at the moment. I create a temporary folder in the temp folder:

self.repository_path = tempfile.mkdtemp(".mozmill-tests")

But the exception which gets raised for you in hg.clone is:

    if os.path.exists(dest):
        if not os.path.isdir(dest):
            raise util.Abort(_("destination '%s' already exists") % dest)
        elif os.listdir(dest):
            raise util.Abort(_("destination '%s' is not empty") % dest) 

But that would mean os.path.isdir() doesn't recognize the folder as directory!?

Reporter, can you please try to run the following script with python and tell me the output:

import os
import tempfile

path = tempfile.mkdtemp(".mozmill-tests")
print os.path.isdir(path)

Comment 2

[Marc-Aurèle DARCHE]

Output is: True

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Then I can't understand why this exception is thrown. Do you have the knowledge to debug the Python script? If not you could at least enter some print statements so we can get closer to the problem.

Comment 4

[Marc-Aurèle DARCHE]

OK, I'll try to debug it.

Comment 5

[Marc-Aurèle DARCHE]

Here it is:

The creation of temporary files and directories work fine on my systems.

The problem appears in the call to mercurial/hg.py clone() method. This clone method has an "if os.path.exists(dest)" check (from at least Mercurial 1.0.1 to the latest version as of this date, that is 1.5.3) that refuses to create a repository in an already existing file/directory.

Because when you do "self.repository_path = tempfile.mkdtemp(".mozmill-tests")" it really does create a file.

To fix the script, replace

    def clone_repository(self, *args, **kwargs):
        """ Clones the repository to a local temporary location. """
        try:
            self.repository_path = tempfile.mkdtemp(".mozmill-tests")

by 

    def clone_repository(self, *args, **kwargs):
        """ Clones the repository to a local temporary location. """
        try:
            self.repository_path = tempfile.mktemp(".mozmill-tests")

with "tempfile.mktemp" that only returns a temp path, without actually creating the file/directory.

But tempfile.mktemp is marked as deprecated in the Python doc, so simply generating your own temp path would feel more sustainable, in case Python drops the support for tempfile.mktemp. But your own temp path generation method would do exactly the same.

Now I wonder how this script could run on your system at all. Any idea?

Comment 6

[Jeff Hammel]

(In reply to comment #5)
> Here it is:
> 
> The creation of temporary files and directories work fine on my systems.
> 
> The problem appears in the call to mercurial/hg.py clone() method. This clone
> method has an "if os.path.exists(dest)" check (from at least Mercurial 1.0.1 to
> the latest version as of this date, that is 1.5.3) that refuses to create a
> repository in an already existing file/directory.
> 
> Because when you do "self.repository_path = tempfile.mkdtemp(".mozmill-tests")"
> it really does create a file.
> 
> To fix the script, replace
> 
>     def clone_repository(self, *args, **kwargs):
>         """ Clones the repository to a local temporary location. """
>         try:
>             self.repository_path = tempfile.mkdtemp(".mozmill-tests")
> 
> by 
> 
>     def clone_repository(self, *args, **kwargs):
>         """ Clones the repository to a local temporary location. """
>         try:
>             self.repository_path = tempfile.mktemp(".mozmill-tests")
> 
> with "tempfile.mktemp" that only returns a temp path, without actually creating
> the file/directory.
> 
> But tempfile.mktemp is marked as deprecated in the Python doc, so simply
> generating your own temp path would feel more sustainable, in case Python drops
> the support for tempfile.mktemp. But your own temp path generation method would
> do exactly the same.
> 
> Now I wonder how this script could run on your system at all. Any idea?

I would also recommend using tempfile.mktemp.  I don't think this is deprecated before py3k.  I would imagine the latter will give us the functionality we want, but if not, then maybe it's worth rolling our own.  I would not got to the point of making a general purpose temporary file generator just on the basis that mktemp will not work at some point in the future.

Comment 7

[Marc-Aurèle DARCHE]

Now with the fix I'm trying to test the only addon presently in the mozmill-tests repository, but no test seems to be started, no firefox appears. Here is the output below.

$ ./testrun_addons.py --with-untrusted ~/opt/firefox-3.6/firefox --addons='[e-mail]'
*** Cloning repository to '/tmp/tmpC-8fXX.mozmill-tests'
*** failed to import extension hgext.inotify: 'module' object has no attribute 'inotify'
requesting all changes
adding changesets
adding manifests
adding file changes
added 460 changesets with 1508 changes to 187 files (+2 heads)
updating working directory
137 files updated, 0 files merged, 0 files removed, 0 files unresolved
*** Updating to branch 'mozilla1.9.2'
rev
*** Removing repository '/tmp/tmpC-8fXX.mozmill-tests'

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #5)
> The problem appears in the call to mercurial/hg.py clone() method. This clone
> method has an "if os.path.exists(dest)" check (from at least Mercurial 1.0.1 to
> the latest version as of this date, that is 1.5.3) that refuses to create a
> repository in an already existing file/directory.

No, see my comment 1. It's only a check if the directory exists, but it also checks if it is an empty folder. The latter is always the case when you are using tempfile.mkdtemp. So I don't see a reason why that part should fail.

I get more the impression that your python installation could be broken. Which exact version of Python are you using?

Comment 9

[Jeff Hammel]

(In reply to comment #8)
> (In reply to comment #5)
> > The problem appears in the call to mercurial/hg.py clone() method. This clone
> > method has an "if os.path.exists(dest)" check (from at least Mercurial 1.0.1 to
> > the latest version as of this date, that is 1.5.3) that refuses to create a
> > repository in an already existing file/directory.
> 
> No, see my comment 1. It's only a check if the directory exists, but it also
> checks if it is an empty folder. The latter is always the case when you are
> using tempfile.mkdtemp. So I don't see a reason why that part should fail.
> 
> I get more the impression that your python installation could be broken. Which
> exact version of Python are you using?

Could this be a difference in behaviour between hg versions installed?

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Could be. How has mercurial be installed? Have you used easy_install or pip? And which version of the mercurial module do you have?

Comment 11

[Marc-Aurèle DARCHE]

First, sorry I haven't checked again your comment #1 before going through the investigation. I should have, sorry, it would have saved us an iteration.

So, I'm running Debian GNU/Linux 5.0.4 (lenny) which ships with Mercurial 1.0.1.
In Mercurial 1.0.1 the existence test is smaller that in the latest version, it only reads, no more detailed check on the dest being a directory:

    if os.path.exists(dest):
        raise util.Abort(_("destination '%s' already exists") % dest)

So my proposition of using tempfile.mktemp is still valid by adding compatibility for the older versions of Mercurial, and now we both should have understood what is really happening with an old and a recent version of Mercurial. If my proposition is accepted, a comment in libs/testrun.py should explain that the sub-optimal use of tempfile.mktemp instead of tempfile.mkdtemp is done to support older versions of Mercurial, otherwise it will be eventually be wept away.

The next release of Debian will ship at least Mercurial 1.5.2 which has the same check of the dest path than Mercurial 1.5.3. But the next release of Debian is not ready yet, some more months to wait.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Thanks for that information. Looks like we have to use that way then. But lemme ask one remaining question. When you say Mercurial 1.0.1 is shipped by default, how is it installed? Have you used setuptools or pip to update the installed version? If not please run "easy_install --update mercurial" or "pip install --upgrade mercurial". It should give you the latest version.

Comment 13

[Marc-Aurèle DARCHE]

When I say Mercurial 1.0.1 is shipped by default, it means that it is shipped as a Debian package archive file, for example in my case mercurial_1.0.1-5.1_i386.deb. The details of the package can be found on http://packages.debian.org/stable/mercurial

Each GNU-Linux distribution has its specific package format. For RedHat and Mandriva it's .rpm, form Debian and Ubuntu it's .deb.

Using the GNU-Linux distribution's own package format is globally preferred as using a side-package format (such as egg for Python or gem for Ruby) since the distribution only knows how to handle its own packages, their dependencies and security fixes. When one started to mix side-packages in a GNU-Linux distribution, one starts to loose the advantages of the consistency of the distribution package repository.

That's why I avoid to install through easy_install (or gems for Ruby) when I can.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Ok, we have an ongoing discussion if we should better remove the dependency of the Python mercurial module in favor of simply calling the system command. M.-A. DARCHE could you please check the result when you clone our repository into a local existing but empty folder under your Debian version? Thanks.

Comment 15

[Marc-Aurèle DARCHE]

Sure. I've done ah "hg pull -u" and I'm sure to have the latest source (your latest commits appear there).

It starts alright, but no test seems to be started, and no firefox window appears. Here is the output below:

$ ./testrun_addons.py --addons='[e-mail]' --with-untrusted ~/opt/firefox-3.6/firefox
*** Cloning repository to '/tmp/tmp1DPAlL.mozmill-tests'
*** failed to import extension hgext.inotify: 'module' object has no attribute 'inotify'
requesting all changes
adding changesets
adding manifests
adding file changes
added 464 changesets with 1522 changes to 188 files (+2 heads)
updating working directory
138 files updated, 0 files merged, 0 files removed, 0 files unresolved
*** Updating to branch 'mozilla1.9.2'
rev
*** Removing repository '/tmp/tmp1DPAlL.mozmill-tests'

Comment 16

[Marc-Aurèle DARCHE]

The main problem now might just be the need for a more complete documentation on the expectations of testrun_addons.py.

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #15)
> Sure. I've done ah "hg pull -u" and I'm sure to have the latest source (your
> latest commits appear there).

I'm not sure what you have done here. So let me explain it in detail. Please run the following steps in the terminal:

$ mkdir test-mozmill
$ hg clone http://hg.mozilla.org/qa/mozmill-automation test-mozmill

Does that command succeed or do you get a failure?

Comment 18

[Marc-Aurèle DARCHE]

Exactly as what you asked, and without trying to understand or interpret anything, here it is:

$ mkdir test-mozmill
$ hg clone http://hg.mozilla.org/qa/mozmill-automation test-mozmill
abort: destination 'test-mozmill' already exists

Comment 19

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Patch v1

Ok, thanks for testing. Looks like that it's always a problem whether if you are using the command line or Python module. So lets make use of the mktemp function even it is deprecated. Seems to be the most valuable way right now.

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Oh, and it also fixes the problem with reporting. I accidentally assigned the parameter to the wrong variable. :/

Comment 21

[Jeff Hammel]

Comment on attachment 450084 [details] [diff] [review]
Patch v1

Looks good.  If mktemp goes away (which I don't think it will until python 3), then I would guess another function will be made available, or writing our own won't be hard.

Comment 22

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Correct.

Landed as:
http://hg.mozilla.org/qa/mozmill-automation/rev/b946dac796fe

M.-A. DARCHE can you please test?

Comment 23

[Marc-Aurèle DARCHE]

(In reply to comment #22)
> Correct.
> 
> Landed as:
> http://hg.mozilla.org/qa/mozmill-automation/rev/b946dac796fe
> 
> M.-A. DARCHE can you please test?

=> No more problems with the Mercurial clone:

$ ./testrun_addons.py --with-untrusted ~/opt/firefox-3.6/firefox

*** Cloning repository to '/tmp/tmp7t6aDs.mozmill-tests'
*** failed to import extension hgext.inotify: 'module' object has no attribute 'inotify'
requesting all changes
adding changesets
adding manifests
adding file changes
added 476 changesets with 1582 changes to 198 files (+2 heads)
updating working directory
144 files updated, 0 files merged, 0 files removed, 0 files unresolved
*** Updating to branch 'mozilla1.9.2'
rev
*** Removing repository '/tmp/tmp7t6aDs.mozmill-tests'

Comment 24

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Thanks. So the remaining issue you are seeing is:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=476885

Comment 25

[Marc-Aurèle DARCHE]

I've got this "failed to import extension hgext.inotify: 'module' object has no attribute 'inotify'" error message all the time and it doesn't prevent me from using Mercurial for everyday development. This shouldn't block me from using mozmill-automation at all.

Comment 26

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Move of Mozmill related project bugs to newly created components. You can
filter out those emails by using "Mozmill-Tests-to-MozillaQA" as criteria.