Repository: pypa/setuptools
Pull request #553 (2016-04-18 19:16:21)

Preserve order of egg_info section in setup.cfg
egg_info is the dictionary with information that is injected
into setup.cfg. edit_config uses RawConfigParser which uses
collections.OrderedDict for all the data. Since we use a
simple dict(), when we loop through items in edit_config, we
see random behavior as a result the fields
tag_svn_revision/tag_date/tag_build are added to the setup.cfg
randomly. Let's add a OrderedDict to make it more predictable.

rbtcollins: If we're going to make this something folk depend on, it really needs a test so that future developers don't break it by mistake.
jaraco: Yes. I also think a test is called for. I'm not sure the implementation as written will work as I believe the keyword arguments will be ordered according to the PYTHONHASHSEED.
dims: @rbtcollins @jaraco i've added a test and sorted the items by key. Seems to work. Please take a look.
jaraco: It seems that the tests aren't capturing the new expectation in egg_info.py. If I apply the following patch to your branch, the tests still pass:

```
diff -r f08b20844fbd setuptools/command/egg_info.py
--- a/setuptools/command/egg_info.py	Mon Apr 18 15:08:49 2016 -0400
+++ b/setuptools/command/egg_info.py	Tue Apr 19 12:20:13 2016 -0400
@@ -2,11 +2,6 @@
 
 Create a distribution's .egg-info directory and contents"""
 
-try:
-    from collections import OrderedDict
-except ImportError:
-    OrderedDict = dict
-
 from distutils.filelist import FileList as _FileList
 from distutils.util import convert_path
 from distutils import log
@@ -72,10 +67,10 @@
 
     def save_version_info(self, filename):
         values = dict(
-            egg_info=OrderedDict(
+            egg_info=dict(
+                tag_svn_revision=0,
+                tag_date=0,
                 tag_build=self.tags(),
-                tag_date=0,
-                tag_svn_revision=0,
             )
         )
         edit_config(filename, values)
```

That leads me to believe the OrderedDict isn't required at all.

Furthermore, this new pull request alters the way that all config files are generated, which may have implications for files other than setup.cfg and sections other than egg_info. Rather than retaining the current order, it specifies a new order (sorted lexicographically).

In my experimentation, RawConfigParser will retain order of any existing options. It's only when new items are added that order is relevant (and they're always appended).

From the perspective of 'edit_config', I think I'd prefer to give control to the caller as to what order the settings are applied, rather than forcing lexicographic ordering.


Could you elaborate on what is the issue you're encountering that's inspiring this change?
dims: Jason, Here's the background. We were trying to verify that old tarball(s) were not tampered with by generating fresh tar.gz(s) and comparing the two. Example output : https://etherpad.openstack.org/p/lJgI00FaLw  
dims: in the end, i'd like to be able to run md5sum on generated tar.gz(s) so we need the files generated to be deterministic.

Patch:
diff --git a/setuptools/command/setopt.py b/setuptools/command/setopt.py
index 7f332be5a..912da7825 100755
--- a/setuptools/command/setopt.py
+++ b/setuptools/command/setopt.py
@@ -2,6 +2,7 @@ from distutils.util import convert_path
 from distutils import log
 from distutils.errors import DistutilsOptionError
 import distutils
+import operator
 import os
 
 from setuptools.extern.six.moves import configparser
@@ -42,7 +43,8 @@ def edit_config(filename, settings, dry_run=False):
     log.debug("Reading configuration from %s", filename)
     opts = configparser.RawConfigParser()
     opts.read([filename])
-    for section, options in settings.items():
+    for section, options in sorted(settings.items(),
+                                   key=operator.itemgetter(0)):
         if options is None:
             log.info("Deleting section [%s] from %s", section, filename)
             opts.remove_section(section)
@@ -50,7 +52,8 @@ def edit_config(filename, settings, dry_run=False):
             if not opts.has_section(section):
                 log.debug("Adding new section [%s] to %s", section, filename)
                 opts.add_section(section)
-            for option, value in options.items():
+            for option, value in sorted(options.items(),
+                                        key=operator.itemgetter(0)):
                 if value is None:
                     log.debug(
                         "Deleting %s.%s from %s",


Test patch:
diff --git a/setuptools/tests/test_egg_info.py b/setuptools/tests/test_egg_info.py
index fd5f26fc2..d37b127e7 100644
--- a/setuptools/tests/test_egg_info.py
+++ b/setuptools/tests/test_egg_info.py
@@ -1,7 +1,16 @@
 import os
 import glob
+import re
 import stat
+import sys
 
+try:
+    from unittest import mock
+except ImportError:
+    import mock
+
+from setuptools.command.egg_info import egg_info
+from setuptools.dist import Distribution
 from setuptools.extern.six.moves import map
 
 import pytest
@@ -59,6 +68,59 @@ class TestEggInfo(object):
             })
             yield env
 
+    def test_egg_info_save_version_info_setup_empty(self, tmpdir_cwd, env):
+        setup_cfg = os.path.join(env.paths['home'], 'setup.cfg')
+        build_files({
+            setup_cfg: DALS("""
+            [egg_info]
+            """),
+        })
+        dist = Distribution()
+        ei = egg_info(dist)
+        ei.initialize_options()
+        ei.save_version_info(setup_cfg)
+
+        with open(setup_cfg, 'r') as f:
+            content = f.read()
+
+            assert '[egg_info]' in content
+            assert 'tag_build =' in content
+            assert 'tag_date = 0' in content
+            assert 'tag_svn_revision = 0' in content
+
+            if sys.version_info[0:2] >= (2, 7):
+                assert re.search('tag_build.*tag_date.*tag_svn_revision',
+                                 content,
+                                 re.MULTILINE | re.DOTALL) is not None
+
+    def test_egg_info_save_version_info_setup_defaults(self, tmpdir_cwd, env):
+        setup_cfg = os.path.join(env.paths['home'], 'setup.cfg')
+        build_files({
+            setup_cfg: DALS("""
+            [egg_info]
+            tag_build =
+            tag_date = 0
+            tag_svn_revision = 0
+            """),
+        })
+        dist = Distribution()
+        ei = egg_info(dist)
+        ei.initialize_options()
+        ei.save_version_info(setup_cfg)
+
+        with open(setup_cfg, 'r') as f:
+            content = f.read()
+
+            assert '[egg_info]' in content
+            assert 'tag_build =' in content
+            assert 'tag_date = 0' in content
+            assert 'tag_svn_revision = 0' in content
+
+            if sys.version_info[0:2] >= (2, 7):
+                assert re.search('tag_build.*tag_date.*tag_svn_revision',
+                                 content,
+                                 re.MULTILINE | re.DOTALL) is not None
+
     def test_egg_base_installed_egg_info(self, tmpdir_cwd, env):
         self._create_project()