This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: distutils2: extension section uses bad environment marker separator
Type: behavior Stage: resolved
Components: Distutils2 Versions: Python 3.3
process
Status: closed Resolution: out of date
Dependencies: Superseder:
Assigned To: eric.araujo Nosy List: alexis, eli.collins, eric.araujo, tarek
Priority: normal Keywords: patch

Created on 2011-06-28 00:12 by eli.collins, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
cpython_issue12424.patch eli.collins, 2011-06-28 00:16 review
9170231ebf14.diff eli.collins, 2011-09-27 21:29 review
Repositories containing patches
http://bitbucket.org/ecollins/cpython#issue-12424-dev
Messages (6)
msg139339 - (view) Author: Eli Collins (eli.collins) * Date: 2011-06-28 00:12
The _pop_values() function in packaging.config uses "--" as the environment marker separator when parsing extension sections. This doesn't match PEP 345, or how the metadata section is currently parsed, both of which use ";" instead. Also, "--" is frequently found in compiler option strings, which will probably cause conflicts in the future. 

The attached patch changes _pop_values() and related unit tests to use ";" as the separator, as well as tries to clarify some of _pop_values()'s internal comments.
msg144557 - (view) Author: Eli Collins (eli.collins) * Date: 2011-09-27 21:32
I've attached a second revision of my patch to fix this issue (9170231ebf14.diff).

Per Eric Araujo's advice, this patch changes _pop_values() to use ";;" as the environment marker separator for all fields in setupcfg's extension sections. I've also updated the documentation and related unittests.
msg148118 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-11-22 15:08
Does your latest patch address my second review?
msg148351 - (view) Author: Eli Collins (eli.collins) * Date: 2011-11-25 19:22
The second patchset (9170231ebf14.diff) should implement all the changes you suggested in your second review (dated 2011-09-05).

---

The only non-addressed item in your second review was a request for clarification on a potential error I noticed (and described somewhat poorly). On later examination, I realized I was mistaken, so didn't want to trouble anyone further with it. 

(If you want details... I noticed that if a line ends with ";;", _pop_values() will call interpret() with an empty string. I initially though that would cause an error, but later realized it just means interpret() would return False, causing the line to be ignored, which is probably fine for that border case - especially since packaging.metadata has the same behavior for lines ending with ";").
msg148406 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-11-26 14:17
> if a line ends with ";;", _pop_values() will call interpret() with an empty string. [...]
> it just means interpret() would return False, causing the line to be ignored, which is probably
> fine for that border case

Hm, I’d rather call that a bug.  _pop_values should not call interpret, or interpret('') should return True (there is no condition that fails), or _pop_values should raise an exception (pro: errors should never pass silently, con: it’s not an error if we define that the empty string is a no-op, and it’s more user-friendly to do that).  I’m inclined to let interpret(''), do you think it’s sensible?
msg148408 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-11-26 14:35
Latest patch looks good.

Looking at the examples with ';;' or even '--', I think that their role is non-obvious.  Even though the PEP and docs explained them, there is value in using intuitive (“guessable”) markup.  So if the constraints are that a marker delimiter should be non-ambiguous and short, why not using something like '#IF#' or '#condition:'?  (This will probably require agreement on the fellowship mailing list.)
History
Date User Action Args
2022-04-11 14:57:19adminsetgithub: 56633
2014-03-13 02:38:43eric.araujosetstatus: open -> closed
resolution: out of date
stage: resolved
2011-11-26 14:35:19eric.araujosetmessages: + msg148408
2011-11-26 14:17:27eric.araujosetassignee: tarek -> eric.araujo
messages: + msg148406
2011-11-25 19:22:59eli.collinssetmessages: + msg148351
2011-11-22 15:08:04eric.araujosetmessages: + msg148118
2011-09-27 21:32:12eli.collinssetmessages: + msg144557
2011-09-27 21:29:13eli.collinssetfiles: + 9170231ebf14.diff
2011-09-27 21:28:21eli.collinssethgrepos: + hgrepo70
2011-06-28 00:16:04eli.collinssetfiles: + cpython_issue12424.patch
keywords: + patch
2011-06-28 00:12:46eli.collinscreate