classification
Title: distutils build_scripts and install_data commands need 2to3 support
Type: enhancement Stage:
Components: Distutils Versions: Python 3.0
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: barry Nosy List: barry, benjamin.peterson, larsimmisch, loewis, mhammond, theller
Priority: release blocker Keywords: patch

Created on 2008-10-08 00:55 by mhammond, last changed 2008-12-01 04:39 by loewis. This issue is now closed.

Files
File name Uploaded Description Edit
build_scripts.diff loewis, 2008-11-30 18:31
Messages (13)
msg74507 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2008-10-08 00:55
The distutils commands 'build_scripts' and 'install_data' both may end
up installing .py files.  Such .py file should be able tobe run through
lib2to3 in the same way supported by build_py.

pywin32 has ended up with something like:

    class my_build_scripts(build_scripts):
        def copy_file(self, src, dest):
            dest, copied = build_scripts.copy_file(self, src, dest)
            # 2to3
            if not self.dry_run and copied:
                refactor_filenames([dest])
            return dest, copied

where 'refactor_filenames' is (basically) the lib2to3 block from
build_py moved to a utility function.
msg74795 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-10-15 07:12
Here is a patch. It refactors the 2to3 support into a function in util,
and adds build_scripts support.

For build_scripts, the approach is slightly different from the one
proposed by Mark: I modify copy_scripts to collect the modified files,
and return them along with outfiles. Modifying copy_file is
insufficient, as it doesn't support the case of adjust.

I'm skeptical about adding build_data support, as it's not obvious what
files would need conversion. Users should adjust their build_data
commands to invoke distutils.util.run_2to3. I would appreciate a
confirmation that this function has a useful API.

In this version, I don't pass any options to the RefactoringTool
anymore, and I arrange to delete the backup files.
msg74820 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-10-15 21:33
If you'd like, I can add support for skipping backup files to 2to3.
msg74821 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-10-15 21:34
> If you'd like, I can add support for skipping backup files to 2to3.

That would be useful, I think.
msg74976 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-10-19 14:31
Ok. I've refactored RefactoringTool so that the write_file method
doesn't make a backup file by default.
msg75059 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2008-10-22 03:36
> I'm skeptical about adding build_data support, as it's not 
> obvious what files would need conversion.

All .py files should be converted.  I can't think why a project would
use this 2to3 capability for scripts and packages, but not .py files in
data files.  pywin32 uses data files to install 'demo' scripts.

Would the motivation become clearer if we modified
Demo/distutils/test2to3 to install a README.txt and a demo .py file
(where the demo .py file should not be installed in the global 'scripts'
directory, but instead in a 'demos' sub-directory of the package)?

> Users should adjust their build_data commands to invoke
> distutils.util.run_2to3.

True - but the same justification could be used to avoid adding support
to build_scripts.  I guess the question is 'how common is it that .py
files will be included in data_files?'  My experience is that it is
common enough to warrant support (but obviously others experiences may
be different)

> I would appreciate a confirmation that this function has a useful API.

I haven't actually applied the patch and modified pywin32 to use it, but
the API certainly looks useful from reading the patch.  +1

Thanks.
msg75060 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2008-10-22 03:40
Thinking more about data_files, I'd agree that blindly converting all
.py files and nothing more isn't as useful for install_data as the other
commands.  It might make more sense to allow data_files to specify a
list of patterns that will be matched to perform the conversion, with
the default being all .py files.
msg75061 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-10-22 05:23
> All .py files should be converted.

I disagree. For example, people decided that all .py files in Tools
should get converted, converting Tools/msi in the process. Now,
Tools/msi is intended to be run in an *older* Python version than the
one being packaged; I currently use 2.4 to do the packaging. It took me
some time to undo this conversion.

It's convenient if users can easily express which files they want to
convert, but it must be a selective, explicit choice.

> I can't think why a project would
> use this 2to3 capability for scripts and packages, but not .py files in
> data files.

See above.

> Would the motivation become clearer if we modified
> Demo/distutils/test2to3 to install a README.txt and a demo .py file
> (where the demo .py file should not be installed in the global 'scripts'
> directory, but instead in a 'demos' sub-directory of the package)?

I understand the motivation fully (I think). I just question whether it
is general. Packages that have this need (to 2to3-convert data files)
would have to come up with their own command subclass still, which
hopefully is very easy with what we provide.

IMO, this is a case where we should err on the conservative side. If we
don't provide the command in 3.0, people will write their own. If your
assumption is correct that this is a common need, we can still add the
command into 3.1.
msg76275 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-11-23 21:05
I'd like to see this in 3.0. It will help people porting to 3.0 better.
msg76289 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-11-24 01:08
Review:

1. Since RefactoringTool no longer writes backup files, write_file
doesn't need to be overridden.

2. Don't worry about **kwds on log_error. It's not used at the moment.

3. It might be nice to let users pick which fixers they do or do not
want. Using only needed fixers can boost performance immensely.
msg76650 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-11-30 18:31
Thanks for the review. Here is a revised version, addressing comments 1
and 2.

Comment 3 is address by exposing all arguments to RefactoringTool as
class variables in a new class Mixin2to3, from which build_py and
build_scripts inherit.
msg76660 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-11-30 19:59
Ok. Looks good.
msg76674 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2008-12-01 04:39
Committed as r67464.
History
Date User Action Args
2008-12-01 04:39:51loewissetstatus: open -> closed
resolution: fixed
messages: + msg76674
2008-11-30 19:59:13benjamin.petersonsetkeywords: - needs review
messages: + msg76660
2008-11-30 18:31:39loewissetfiles: - build_scripts.diff
2008-11-30 18:31:27loewissetfiles: + build_scripts.diff
messages: + msg76650
2008-11-27 23:43:56larsimmischsetnosy: + larsimmisch
2008-11-24 01:08:39benjamin.petersonsetmessages: + msg76289
2008-11-23 21:05:24loewissetpriority: release blocker
assignee: barry
messages: + msg76275
nosy: + barry
2008-10-22 05:24:01loewissetmessages: + msg75061
2008-10-22 03:40:56mhammondsetmessages: + msg75060
2008-10-22 03:36:41mhammondsetmessages: + msg75059
2008-10-19 14:31:03benjamin.petersonsetmessages: + msg74976
2008-10-15 21:34:09loewissetmessages: + msg74821
2008-10-15 21:33:21benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg74820
2008-10-15 07:14:16loewissetfiles: + build_scripts.diff
2008-10-15 07:14:02loewissetfiles: - build_scripts.diff
2008-10-15 07:13:25loewissetfiles: + build_scripts.diff
2008-10-15 07:12:59loewissetfiles: - build_scripts.diff
2008-10-15 07:12:29loewissetkeywords: + patch, needs review
files: + build_scripts.diff
messages: + msg74795
2008-10-09 18:14:42thellersetnosy: + theller
2008-10-08 00:55:46mhammondcreate