Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

distutils should not assume that hardlinks will work #53122

Closed
samtygier mannequin opened this issue Jun 2, 2010 · 24 comments
Closed

distutils should not assume that hardlinks will work #53122

samtygier mannequin opened this issue Jun 2, 2010 · 24 comments
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@samtygier
Copy link
Mannequin

samtygier mannequin commented Jun 2, 2010

BPO 8876
Nosy @malemburg, @birkenfeld, @pitrou, @larryhastings, @benjaminp, @tarekziade, @merwok, @ssbarnea
Files
  • hardlink-traceback.txt
  • distutil-hardlink-trunk.diff
  • copy_file_link_fail.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2014-10-30.18:54:39.408>
    created_at = <Date 2010-06-02.13:38:35.110>
    labels = ['type-bug', 'library']
    title = 'distutils should not assume that hardlinks will work'
    updated_at = <Date 2014-10-30.19:14:46.538>
    user = 'https://bugs.python.org/samtygier'

    bugs.python.org fields:

    activity = <Date 2014-10-30.19:14:46.538>
    actor = 'eric.araujo'
    assignee = 'none'
    closed = True
    closed_date = <Date 2014-10-30.18:54:39.408>
    closer = 'pitrou'
    components = ['Distutils']
    creation = <Date 2010-06-02.13:38:35.110>
    creator = 'samtygier'
    dependencies = []
    files = ['17521', '17522', '37069']
    hgrepos = []
    issue_num = 8876
    keywords = ['patch']
    message_count = 24.0
    messages = ['106881', '106885', '106886', '164543', '164545', '164546', '164554', '186994', '186996', '188970', '208191', '208776', '208792', '211180', '229298', '230211', '230213', '230215', '230242', '230250', '230283', '230285', '230286', '230290']
    nosy_count = 14.0
    nosy_names = ['lemburg', 'georg.brandl', 'pitrou', 'larry', 'benjamin.peterson', 'tarek', 'eric.araujo', 'ssbarnea', 'samtygier', 'python-dev', 'olliewalsh', 'fkochem', 'dfarrell07', 'Matt.Wright']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue8876'
    versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

    @samtygier
    Copy link
    Mannequin Author

    samtygier mannequin commented Jun 2, 2010

    distutils will currently try to use hardlinks if os has a 'link' attribute, however sometimes os.link() will fail, for example the filesystem may not support it (see attached traceback).

    in commands/sdist.py in make_release_tree() there is the test:
    if hasattr(os, 'link'): # can make hard links on this system
    link = 'hard'
    msg = "making hard links in %s..." % base_dir

    'link' is then passed to copy_file() in file_util.py, which trusts that if link == 'hard', then hardlinking will work.

    there has been discussion in the past, but i dont think it has been fixed
    http://thread.gmane.org/gmane.comp.python.distutils.devel/2076

    @samtygier samtygier mannequin assigned tarekziade Jun 2, 2010
    @samtygier samtygier mannequin added the stdlib Python modules in the Lib dir label Jun 2, 2010
    @samtygier
    Copy link
    Mannequin Author

    samtygier mannequin commented Jun 2, 2010

    here is a patch against http://svn.python.org/projects/python/trunk

    it moves the return statements into the individual file copying sections, and takes the final call to _copy_file_contents() out of the else. this allows an error in hardlinking to fall through by passing the exception.

    i have only tested on Linux.

    when running it still prints out
    "hard linking CHANGES -> pyzgoubi-0.4dev"
    because of how the message is generated in line 130. do you think this needs to be changed. do we need a
    "hardlinking failed, falling back to copy"
    message? it might cause unnecessary worry (distutils already suppresses tracebacks to avoid scaring non-python-programmers).

    i have a patch that applies to 2.6 as well, is there much chance of that being accepted?

    @merwok
    Copy link
    Member

    merwok commented Jun 2, 2010

    Distutils is a special case: Many third-party code relies on its undocumented quirks and bugs, so it’s basically frozen. Some non-disruptive bugfixes are accepted, in which case the normal Python rules apply (e.g., no new features in 2.7 which is in beta). Work has been moved to Distutils2, which breaks compatibility in order to fix things and add needed features.

    If Tarek (maintainer of both packages) agrees this can go into Distutils, Benjamin Peterson (release manager) will say whether he agrees or not. Patient a while for Tarek to see this bug report, and thanks for your work :)

    @merwok merwok added the type-bug An unexpected behavior, bug, or error label Sep 30, 2010
    @merwok
    Copy link
    Member

    merwok commented Jul 2, 2012

    AFAICT using hard links only serves to save up a little time and disk space; it seems to me that always copying would solve one or two bugs at a small cost (not so small for large projects, I don’t know). Could this impact setup scripts? Maybe if os.link were monkey-patched and expected to be called. I could ask on distutils-sig.

    In bpo-15205 Ollie noted this: “distutils2 appears to always copy instead of hardlinking resolving all of these issues”. That would be because d2 uses shutil.copytree and distutils.util.copy_file (yes, a function from distutils1 because of some Windows issue :().

    @merwok merwok assigned merwok and unassigned tarekziade Jul 2, 2012
    @merwok
    Copy link
    Member

    merwok commented Jul 2, 2012

    Of course the conservative fix would be to try linking as of now and do a copy when an exception is caught, as was proposed.

    FTR python.org link to the thread mentioned: http://mail.python.org/pipermail/distutils-sig/2005-August/004954.html

    @merwok
    Copy link
    Member

    merwok commented Jul 2, 2012

    samtygier’s patch to implement the fallback-to-copy idea looks good. Not sure if an automated test would be written, but samtygier says it was manually tested.

    @pitrou
    Copy link
    Member

    pitrou commented Jul 2, 2012

    AFAICT using hard links only serves to save up a little time and disk
    space; it seems to me that always copying would solve one or two bugs
    at a small cost

    I agree with this.

    @ssbarnea
    Copy link
    Mannequin

    ssbarnea mannequin commented Apr 15, 2013

    Can we have this merged, it prevents us form using distutil, especially in a continuous integration environment where you do not have control over the build server.

    See: https://drone.io/github.com/pycontribs/tendo/1

    @merwok
    Copy link
    Member

    merwok commented Apr 15, 2013

    I’ll get this in the next bugfix releases.

    @benjaminp
    Copy link
    Contributor

    Not blocking on this old bug.

    @larryhastings
    Copy link
    Contributor

    Has this been fixed?

    @fkochem
    Copy link
    Mannequin

    fkochem mannequin commented Jan 22, 2014

    Éric said that he should be able to get to it soon.:
    https://twitter.com/merwok_/status/425596183176704002

    @fkochem
    Copy link
    Mannequin

    fkochem mannequin commented Jan 22, 2014

    A dirty hack is to include this line at the top of your setup.py:

    del os.link

    @merwok
    Copy link
    Member

    merwok commented Feb 13, 2014

    Sorry for the lax definition of “soon”; I’m back from a busy January and will try to make some time for CPython contributions.

    @MattWright
    Copy link
    Mannequin

    MattWright mannequin commented Oct 14, 2014

    Here's another example of where this is a pain. An emerging workflow is using Docker for a Python environment. However, on OS X, its common to use boot2docker (a lightweight VM). With VirtualBox on OS X, its common to setup a shared folder between the host and boot2docker so one can mount volumes to Docker containers from the host to the container(s) on the VM. So when running something like tox or python setup.py sdist things fail because VirtualBox does not allow hard links on the shared folder filesystem. Changing this would be grand. Otherwise I have the, arguably, ugly del os.link in my setup.py.

    @dfarrell07
    Copy link
    Mannequin

    dfarrell07 mannequin commented Oct 29, 2014

    An emerging workflow is using Docker for a Python environment.

    This applies to Vagrant environments as well (VirtualBox shared folders, tests fail).

    @pitrou
    Copy link
    Member

    pitrou commented Oct 29, 2014

    I agree this deserves fixing, and the patch looks basically ok. It would be nice to add a test, though (by mocking os.link()).

    Éric, are you still here, or should someone else take over?

    @merwok
    Copy link
    Member

    merwok commented Oct 29, 2014

    I can’t make any commitment to core Python at this time. I still read all email and can be available for guidance or questions about obscure parts of distutils.

    (Ideally I would find someone willing to be mentored to take over maintenance.)

    @pitrou
    Copy link
    Member

    pitrou commented Oct 29, 2014

    Updated patch with tests.

    @pitrou
    Copy link
    Member

    pitrou commented Oct 29, 2014

    For the record, the only place where distutils seems to use hard-linking is distutils.command.sdist. The fallback on copying is safe there, since it's done manually if os.link doesn't exist.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 30, 2014

    New changeset d94d8789e924 by Antoine Pitrou in branch '3.4':
    Issue bpo-8876: distutils now falls back to copying files when hard linking doesn't work.
    https://hg.python.org/cpython/rev/d94d8789e924

    New changeset ce484e0840e3 by Antoine Pitrou in branch 'default':
    Issue bpo-8876: distutils now falls back to copying files when hard linking doesn't work.
    https://hg.python.org/cpython/rev/ce484e0840e3

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 30, 2014

    New changeset 263395345aa7 by Antoine Pitrou in branch '2.7':
    Issue bpo-8876: distutils now falls back to copying files when hard linking doesn't work.
    https://hg.python.org/cpython/rev/263395345aa7

    @pitrou
    Copy link
    Member

    pitrou commented Oct 30, 2014

    This should now be fixed. Thanks for your patience :)

    @pitrou pitrou closed this as completed Oct 30, 2014
    @merwok
    Copy link
    Member

    merwok commented Oct 30, 2014

    Thanks for the patches folks!

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants