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 sdist add_defaults does not add data_files #46532

Closed
dripton mannequin opened this issue Mar 12, 2008 · 14 comments
Closed

distutils sdist add_defaults does not add data_files #46532

dripton mannequin opened this issue Mar 12, 2008 · 14 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@dripton
Copy link
Mannequin

dripton mannequin commented Mar 12, 2008

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
Copy link
Mannequin Author

dripton mannequin commented Mar 12, 2008

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.

@dripton dripton mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Mar 12, 2008
@tarekziade tarekziade mannequin self-assigned this Feb 6, 2009
@gsakkis
Copy link
Mannequin

gsakkis mannequin commented Feb 11, 2009

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
Copy link
Mannequin

tarekziade mannequin commented Feb 13, 2009

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
Copy link
Mannequin

gsakkis mannequin commented Feb 14, 2009

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
Copy link
Mannequin

tarekziade mannequin commented Feb 14, 2009

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
Copy link
Mannequin

gsakkis mannequin commented Feb 16, 2009

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
Copy link
Mannequin

tarekziade mannequin commented Feb 16, 2009

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
Copy link
Mannequin

tarekziade mannequin commented Feb 16, 2009

done in r69692 and r69696.

@tarekziade tarekziade mannequin closed this as completed Feb 16, 2009
@gsakkis
Copy link
Mannequin

gsakkis mannequin commented Feb 17, 2009

> 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
Copy link
Mannequin

gsakkis mannequin commented Feb 17, 2009

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
Copy link
Mannequin

tarekziade mannequin commented Feb 17, 2009

added in r69710. Thx

@tarekziade
Copy link
Mannequin

tarekziade mannequin commented Feb 17, 2009

2009/2/17 George Sakkis <report@bugs.python.org>:

> 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):

Please could you add a feature request ?

We will need to discuss it there.

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

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.

Please do so, I am focusing on the Distutils-SIG ML , so I missed it

I don't know yet what is a proper way to adress this, but the bug tracker seem
apprioriate for this.

----------
versions: +Python 2.6, Python 3.0


Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue2279\>



Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/ziade.tarek%40gmail.com

@gsakkis
Copy link
Mannequin

gsakkis mannequin commented Feb 17, 2009

Opened bpo-5300 and bpo-5302 for the mentioned issues.

Btw in your patch, I believe os.path.join(dirname, f) should be
replaced by f alone; dirname refers to the dir under the
installation directory, not the source.

@tarekziade
Copy link
Mannequin

tarekziade mannequin commented Feb 17, 2009

Opened bpo-5300 and bpo-5302 for the mentioned issues.

ok thanks

Btw in your patch, I believe os.path.join(dirname, f) should be
replaced by f alone; dirname refers to the dir under the
installation directory, not the source.

Right, thanks George. I'v also added you in the ACKS for your contribution
on this (r69724)

@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

0 participants