[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Marco Sulla]

Marco Sulla  added the comment:

This is IMHO broken.

1. _ensure_list() allows strings, because, documentation says, they are split 
in finalize_options(). But finalize_options() does only split keywords and 
platforms. It does _not_ split classifiers.

2. there's no need that keywords, platforms and classifiers must be a list. 
keywords and platforms can be any iterable, and classifiers can be any non 
text-like iterable. 

Indeed, keywords are written to file using ','.join(), and platforms and 
classifiers are written using DistributionMetadata._write_list(). They both 
accepts any iterable, so I do not understand why such a strict requirement.

--
nosy: +Marco Sulla

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Neil Schemenauer]

Change by Neil Schemenauer :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Berker Peksag]

Berker Peksag  added the comment:

Thank you for fixing this, Neil. Can we close this issue now?

--
priority: release blocker -> normal

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Neil Schemenauer]

Neil Schemenauer  added the comment:


New changeset 8837dd092fe5ad5184889104e8036811ed839f98 by Neil Schemenauer in 
branch 'master':
bpo-19610: Warn if distutils is provided something other than a list to some 
fields (#4685)
https://github.com/python/cpython/commit/8837dd092fe5ad5184889104e8036811ed839f98


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Neil Schemenauer]

Neil Schemenauer  added the comment:

Don't be sorry.  We are all passionate about making Python better.  distutils 
will be better once we gets this sorted out.  Berker deserves credit for seeing 
an issue and developing on a fix for it.  The collaboration between all the 
core developers is making the final fix better than what any one of us could 
make.

The process is working quite well IMHO.  I had a small patch a few days ago 
that I was considering just committing without reviews.  I went through the 
review process though and the patch was better as a result of the review 
feedback I got.

The documentation says that the argument needs to be a list of strings or a 
string.  So, unless we change the documentation, a string must be accepted 
without warnings or errors.  The current PR works pretty well.  Figuring out 
how to best warn is the trickiest bit.  Sometimes it seems like 
DepreciationWarning doesn't work as we would like, since it often gets 
suppressed.  It seems pip is especially "good" at hiding it.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[STINNER Victor]

Change by STINNER Victor :


--
nosy:  -vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Éric Araujo]

Éric Araujo  added the comment:

I am sorry this snowballed.  The intent in my messages here and in my PR review 
was to exchange a late, unfriendly traceback with a clear, early message, but 
only for package authors.  I forgot that a Python 3.7 could execute an invalid 
setup.py from an existing tarball, as Neil pointed with the pip install 
exifread example.  Even if these packages get PRs to build wheels, it’s still 
bad to break existing sdists.  +1 to reverting the patch, +1 to reverting the 
breakage and also fixing the original problem with a warning and explicit 
conversion.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Nick Coghlan]

Nick Coghlan  added the comment:

Prohibiting strings and bytes on the grounds of "Yes they're iterable, but are 
more likely intended as atomic here, so treat them as ambiguous and refuse to 
guess" would be fine. (Although I'll also note the classifier case will already 
fail on upload, since they won't be valid classifiers)

The only part I'm not OK with is upgrading a historically unenforced type 
restriction that only sometimes causes problems into an eagerly enforced one 
that breaks currently working code.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Antoine Pitrou]

Antoine Pitrou  added the comment:

Le 03/12/2017 à 05:57, Neil Schemenauer a écrit :
> 
> I like Nick's idea of calling list() to fix the argument.

I don't think that's a good idea.  Suppose someone is passing a string
by mistake:

setup(...,
  classifiers='Programming Language :: Python :: 3 :: Only',
  )

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Perhaps it is worth to backport warnings to 2.7 in Py3k mode. This could help 
to detect some issues earlier.

In 3.6 fields could be converted to lists silently, without warnings.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Neil Schemenauer]

Neil Schemenauer  added the comment:

I like Nick's idea of calling list() to fix the argument.  I've created a PR 
that implements it.  I also generate a RuntimeWarning since if we document them 
as needing to be lists, we should at least warn for invalid types.  The 
RuntimeWarning will be seen if you call setup.py directly which should have the 
effect of eventually pushing package authors to fix their code.

--
keywords: +needs review -patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Neil Schemenauer]

Change by Neil Schemenauer :


--
pull_requests: +4598
stage: resolved -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Nick Coghlan]

Nick Coghlan  added the comment:

I'd prefer to see this change go in the other direction: instead of enforcing 
eager type checks, we should be unconditionally wrapping the given values in a 
"list(arg)" call, such that more typical iterable duck-typing behaviour applies.

That will transparently fix any code that isn't already passing a list, will 
ensure that internal code can safely assume that the instance *attributes* are 
always lists (no matter what the caller passes in), and means that even when 
the caller is passing in a list, the instance will have its own copy (rather 
than retaining a reference to the caller's copy).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Neil Schemenauer]

Neil Schemenauer  added the comment:

> Thank you, but I don't need a lecture from you. Feel free to propose our 
> solution in the form of pull request instead of acting like a project manager 
> and telling people what to do.

I'm sorry you are offended.  My pull request would consist of the patch being 
reverted.  It is not ready to go in.  If a change breaks a significant amount 
of previously working Python code, it needs very careful consideration and 
should be introduced in a way to minimize breakage and allow people time to fix 
their code.

Repeatably pointing to the documentation and saying that existing code is 
broken because it doesn't respect documented requirements is not okay.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Berker Peksag]

Berker Peksag  added the comment:

> Filed a bug https://github.com/simplejson/simplejson/issues/198 for
> simplejson.

I've opened https://github.com/simplejson/simplejson/pull/197 to make 
CLASSIFIERS a list.

I've also opened https://github.com/ianare/exif-py/pull/80 to create a 
universal wheel for exifread.

> That's not how we develop Python.

Thank you, but I don't need a lecture from you. Feel free to propose your 
solution in the form of pull request instead of acting like a project manager 
and telling people what to do.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Neil Schemenauer]

Neil Schemenauer  added the comment:

> Classifiers were always documented as lists (msg214915) and passing a 
> non-list type was raised a cryptic exception message as already reported in 
> my first message

That doesn't matter.  You can't break a bunch of packages in a 3.x release with 
no warning just because people are doing something against what the 
documentation says.  That's not how we develop Python. How is a user of a 3rd 
party package going to fix this?  They have to ask the 3rd party package author 
to fix their setup.py code.  Until then, they cannot use Python 3.7.  This 
patch needs to be reverted and reworked, IMHO.

I trying installing the top packages listed on https://pythonwheels.com .  A 
number of them fail because of this change, see attached text file.

--
Added file: https://bugs.python.org/file47312/pip-errors.txt

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Filed a bug https://github.com/simplejson/simplejson/issues/198 for simplejson.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

In simplejson classifiers is a result of filter(). This is a list in Python 2 
and an iterator in Python 3. It can be uploaded using Python 2.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Ned Deily]

Change by Ned Deily :


--
nosy: +ncoghlan

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Berker Peksag]

Berker Peksag  added the comment:

(Sorry, I messed up fields in the issue.)

--
nosy: +ned.deily
priority: normal -> release blocker

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Berker Peksag]

Berker Peksag  added the comment:

Classifiers were always documented as lists (msg214915) and passing a non-list 
type was raised a cryptic exception message as already reported in my first 
message, https://github.com/pypa/setuptools/issues/1163 and 
https://reinbach.com/blog/setuptools-classifiers-upload-python3-5/

Any usage of classifiers=(...,) is already broken in Python 3 in some way (see 
the setup.py I attached or 
https://reinbach.com/blog/setuptools-classifiers-upload-python3-5/ for a quick 
reproducer) since they can't upload a new release.

Also, this is only an issue when sdist is the only way to install the project. 
exifread only provides a wheel for Python 2. I cloned it and add

[wheel]
universal = 1

then I created a universal wheel and tried to install it in Python 3.7.0a2+:

Processing 
/home/berker/projects/test/exif-py/dist/ExifRead-2.1.2-py2.py3-none-any.whl
Installing collected packages: ExifRead
Successfully installed ExifRead-2.1.2

--
nosy:  -ned.deily
priority: release blocker -> normal

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Neil Schemenauer]

Neil Schemenauer  added the comment:

I tried building the top packages from python3wos.appspot.com.  Only 
simplejson-3.13.2.tar.gz fails to build due to this change.  However, given 
that it is the top downloaded module, I think think making a change to Python 
that makes it uninstallable by "pip" is a bad idea.  There needs to be a 
transition process.  I'm setting priority to "release blocker".  People can 
downgrade if they disagree with me.

I tried changing the TypeError raises with Deprecation warnings.  That doesn't 
have any effect because DeprecationWarning is filtered by default.  Enabling it 
still has no effect because apparently pip filters it out.

So, I think some other way to warn people would need to be implemented.  E.g. 
have distutils print to stderr.  Change pip to warn as well.

--
nosy: +ned.deily
priority: normal -> release blocker

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Neil Schemenauer]

Neil Schemenauer  added the comment:

I don't see a good reason to add this check.  I would guess there could be lots 
of 3rd party packages that are no uninstallable on Python 3.7.  E.g. 

python3 -m pip install exifread
...
TypeError: 'classifiers' should be a 'list', not 'tuple'

If people are determined to add extra type checking, make it a warning for 3.7 
so setup.py files can be fixed.

--
nosy: +nascheme
resolution: fixed -> 
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Berker Peksag]

Berker Peksag  added the comment:


New changeset dcaed6b2d954786eb5369ec2e8dfdeefe3cdc6ae by Berker Peksag in 
branch 'master':
bpo-19610: setup() now raises TypeError for invalid types (GH-4519)
https://github.com/python/cpython/commit/dcaed6b2d954786eb5369ec2e8dfdeefe3cdc6ae


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Berker Peksag]

Change by Berker Peksag :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Berker Peksag]

Berker Peksag  added the comment:

Éric and Henk-Jaap: I've now opened PR 4519.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Berker Peksag]

Change by Berker Peksag :


--
pull_requests: +4456

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Berker Peksag]

Berker Peksag  added the comment:

Thanks for the ping and for the review! I will open a PR this week.

--
versions:  -Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Éric Araujo]

Éric Araujo  added the comment:

Latest patch seems good.  Berker, would you have the time to adapt for 3.7 and 
submit as a PR?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Henk-Jaap Wagenaar]

Henk-Jaap Wagenaar  added the comment:

This is still present, and also silently affects Python 2.7 as evidenced here: 
https://github.com/pypa/setuptools/issues/1163

I am happy to adapt Berker Peksags patch to a PR, if he is?

--
nosy: +Henk-Jaap Wagenaar
versions: +Python 3.6, Python 3.7 -Python 3.5

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Berker Peksag]

Berker Peksag added the comment:

Éric, could you please take a look at issue19610_v4.diff? I'd like to commit 
the patch this weekend. Thanks!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Berker Peksag]

Berker Peksag added the comment:

Thanks for the review, Éric. New patch attached.

> When running a setup.py that uses a tuple for classifiers, is the error 
> message in the terminal user-friendly, or do we get a full traceback?

A full traceback:

Traceback (most recent call last):
  File "setup.py", line 37, in 
platforms=('Windows', 'Any'),
  File "/home/berker/projects/cpython/default/Lib/distutils/core.py", line 108, 
in setup
_setup_distribution = dist = klass(attrs)
  File "/home/berker/projects/cpython/default/Lib/distutils/dist.py", line 253, 
in __init__
getattr(self.metadata, "set_" + key)(val)
  File "/home/berker/projects/cpython/default/Lib/distutils/dist.py", line 
1212, in set_platforms
raise TypeError(msg % type(value).__name__)
TypeError: 'platforms' should be a 'list', not 'tuple'

--
Added file: http://bugs.python.org/file38862/issue19610_v4.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue19610] Give clear error messages for invalid types used for setup.py params (e.g. tuple for classifiers)

[Éric Araujo]

Éric Araujo added the comment:

Thanks, reviewed.

When running a setup.py that uses a tuple for classifiers, is the error message 
in the terminal user-friendly, or do we get a full traceback?

--
title: setup.py does not allow a tuple for classifiers -> Give clear error 
messages for invalid types used for setup.py params (e.g. tuple for classifiers)

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com