distutils sdist add_defaults does not add data_files

[dripton]

BPO	2279
Nosy	@tarekziade

^{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 = 'https://github.com/tarekziade'
closed_at = <Date 2009-02-16.21:59:11.331>
created_at = <Date 2008-03-12.14:53:31.412>
labels = ['type-bug', 'library']
title = 'distutils sdist add_defaults does not add data_files'
updated_at = <Date 2010-06-03.14:53:13.518>
user = 'https://bugs.python.org/dripton'

bugs.python.org fields:

activity = <Date 2010-06-03.14:53:13.518>
actor = 'eric.araujo'
assignee = 'tarek'
closed = True
closed_date = <Date 2009-02-16.21:59:11.331>
closer = 'tarek'
components = ['Distutils']
creation = <Date 2008-03-12.14:53:31.412>
creator = 'dripton'
dependencies = []
files = []
hgrepos = []
issue_num = 2279
keywords = []
message_count = 14.0
messages = ['63475', '81650', '81888', '81980', '82004', '82213', '82269', '82277', '82297', '82298', '82309', '82313', '82383', '82388']
nosy_count = 3.0
nosy_names = ['dripton', 'gsakkis', 'tarek']
pr_nums = []
priority = 'high'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue2279'
versions = ['Python 2.6', 'Python 3.0', 'Python 3.1', 'Python 2.7']

[dripton]

distutils.sdist.add_defaults adds the Python modules and scripts and C
extensions found in setup.py to the MANIFEST. It does *not* add
data_files mentioned in setup.py to the MANIFEST.

This is non-orthogonal and confusing, because it means that a
MANIFEST.in is required if you have data_files, optional if you do not.
If you have data_files and do not have MANIFEST.in, you get a broken
package but no error.

[gsakkis]

Agreed; package_data are also ignored by sdist. Unfortunately, neither
setuptools seems to work as expected on sdist but at least bdist_egg does.

MANIFEST.in is an ugly hack and should be deprecated; you shouldn't have
to repeat yourself in two files.

[tarekziade]

I'll change add_defaults() so it includes package_data.

I don't think though, that MANIFEST.in is an ugly hack. Imho the best
practice is an explicit MANIFEST.in (and not use the .svn browsing
setuptools do)

[gsakkis]

I didn't mean to imply that automagic discovery based on external
version control software is better than MANIFEST.in; I favor
explicitness here as well. It's just that this information can (and
often has to) be duplicated in setup.py as 'package_data' or
'data_files'. For cases that package_data/data_files are not enough,
setup.py should be extended to handle them, instead of requiring to
write and keep in sync a separate file with its own mini syntax.

[tarekziade]

Right, but if MANIFEST.in is removed, I can see cases where you would
need the same kind of mini-syntax in your setup.py.

For instance, how would you tell sdist to recursively add files located
in a directory (like the current recursive-include feature of MANIFEST.in) ?

[gsakkis]

By an equivalent option in setup() of course. I'm not against the
*functionality* of MANIFEST.in but on that (a) it's a second file you
have to remember to write and maintain in addition to setup.py (b) has
its own ad-hoc syntax instead of python and (c) overlaps in scope with
the package_data and data_files of setup.py.

FWIW I wrote a module that overrides the default build_py and sdist
commands with versions that allow specifying package_data recursively
(while preserving file permissions, unlike the - buggy IMO - behavior of
distutils) so that I can get rid of the MANIFEST.in.

[tarekziade]

Ok I have finished the patch, I'll commit it during the week

FWIW I wrote a module that overrides the default build_py and sdist
commands with versions that allow specifying package_data recursively

Maybe that could be a new feature ?

(while preserving file permissions, unlike the - buggy IMO - behavior of
distutils) so that I can get rid of the MANIFEST.in.

Sounds like a bug to me, could you fill an issue on that ?

[tarekziade]

done in r69692 and r69696.

[gsakkis]

> FWIW I wrote a module that overrides the default build_py and sdist
> commands with versions that allow specifying package_data recursively

Maybe that could be a new feature ?

That would be nice, especially if we want to reimplement MANIFEST.in as
setup() option at some point. My current implementation doesn't extend
the API, so there's no way to specify a subset of files under a
directory like recursive-include; every directory matched by a glob is
copied in whole (recursively):

import os
from distutils.command.build_py import build_py as _build_py

class build_py(_build_py):
    def find_data_files(self, package, src_dir):
        files = []
        for p in _build_py.find_data_files(self, package, src_dir):
            if os.path.isdir(p):
                files.extend(os.path.join(par,f)
                             for par,dirs,files in os.walk(p) 
                             for f in files)
            else:
                files.append(p)
        return files

> (while preserving file permissions, unlike the - buggy IMO - behavior of
> distutils) so that I can get rid of the MANIFEST.in.

Sounds like a bug to me, could you fill an issue on that ?

If it's a bug, it's certainly not accidental; there's a big XXX comment
justifying this choice but I'm not convinced. I posted about it at
http://mail.python.org/pipermail/python-list/2009-January/524263.html;
if you think it's a bug I'll fill an issue.

[gsakkis]

done in r69692 and r69696.

Great, thanks. The data_files part though seems incorrect; for one thing
each item in data_files can be either a (dir,files) tuple or a plain
string, and for two 'dir' is the output (installation) directory, not
the root of 'files'. Here's the relevant part from my module:

if self.distribution.has_data_files():
    for item in self.distribution.data_files:
        if isinstance(item, basestring): # plain file
            self.filelist.append(convert_path(item))
        else: # an (outdir, files) tuple
            outdir,data_files = item
            self.filelist.extend(convert_path(f) for f in data_files)

[tarekziade]

added in r69710. Thx