| msg63475 - (view) |
Author: David Ripton (dripton) |
Date: 2008-03-12 14:53 |
| 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. |
|
|
| msg81650 - (view) |
Author: George Sakkis (gsakkis) |
Date: 2009-02-11 16:46 |
| 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. |
|
|
| msg81888 - (view) |
Author: Tarek Ziadé (tarek) *  |
Date: 2009-02-13 08:58 |
| 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) |
|
|
| msg81980 - (view) |
Author: George Sakkis (gsakkis) |
Date: 2009-02-14 00:06 |
| 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. |
|
|
| msg82004 - (view) |
Author: Tarek Ziadé (tarek) * |
Date: 2009-02-14 10:24 |
| 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) ? |
|
|
| msg82213 - (view) |
Author: George Sakkis (gsakkis) |
Date: 2009-02-16 03:18 |
| 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. |
|
|
| msg82269 - (view) |
Author: Tarek Ziadé (tarek) * |
Date: 2009-02-16 20:36 |
| 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 ? |
|
|
| msg82277 - (view) |
Author: Tarek Ziadé (tarek) * |
Date: 2009-02-16 21:59 |
| done in r69692 and r69696. |
|
|
| msg82297 - (view) |
Author: George Sakkis (gsakkis) |
Date: 2009-02-17 03:31 |
| > > 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. |
|
|
| msg82298 - (view) |
Author: George Sakkis (gsakkis) |
Date: 2009-02-17 03:52 |
| > 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) |
|
|
| msg82309 - (view) |
Author: Tarek Ziadé (tarek) * |
Date: 2009-02-17 09:44 |
| added in r69710. Thx |
|
|
| msg82313 - (view) |
Author: Tarek Ziadé (tarek) * |
Date: 2009-02-17 10:08 |
| 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 > > |
|
|
| msg82383 - (view) |
Author: George Sakkis (gsakkis) |
Date: 2009-02-17 22:45 |
| Opened #5300 and #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. |
|
|
| msg82388 - (view) |
Author: Tarek Ziadé (tarek) * |
Date: 2009-02-17 23:08 |
| > Opened #5300 and #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) |
|
|