Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Please make the generated requires.txt files reproducible #458

Closed
ghost opened this issue Nov 6, 2015 · 8 comments
Closed

Please make the generated requires.txt files reproducible #458

ghost opened this issue Nov 6, 2015 · 8 comments

Comments

@ghost
Copy link

ghost commented Nov 6, 2015

Originally reported by: lambyuk (Bitbucket: lambyuk, GitHub: Unknown)


Whilst working on the "reproducible builds" effort [0], we noticed that
python-setuptools generates non-deterministic output when creating
requires.txt files:

│   │   │   │  [all]
│   │   │   │ -fake-factory==0.5.2
│   │   │   │ -pytz
│   │   │   │  numpy>=1.9.0
│   │   │   │  pytz
│   │   │   │ +pytz
│   │   │   │  django>=1.7
│   │   │   │ +fake-factory==0.5.2


Patch attached.

 [0] https://wiki.debian.org/ReproducibleBuilds

Filed in Debian as https://bugs.debian.org/804249


@ghost
Copy link
Author

ghost commented Nov 15, 2015

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


I agree with the sentiment here, but I'm unsure about the patch. I know order of resolution is important, so sorting lexicographically could violate other expectations (or goals). I'd prefer if setuptools could capture the user's order and retain that. I haven't had a chance to investigate this in more detail, so I don't know what implications that retention would have. Would you be willing to explore that possibility?

@ghost
Copy link
Author

ghost commented Nov 15, 2015

Original comment by lambyuk (Bitbucket: lambyuk, GitHub: Unknown):


Three points:

First, I doubt it makes any difference whatsoever.

Second, sorting lexographically and possibly introducing unexpected expectations would be at least no worse than the status quo which is—as the bug title implies—a non-determinstic output. Nobody can really be relying on the current behaviour as it's inherently unreliable!

Lastly, nobody should be relying on the current behaviour without a reams of comments. Just far too bizarre and/or the software is broken in some other, horrible, way.

@anthraxx
Copy link

@jaraco any updates on this? debian is already applying a custom patch to this issue. currently this is blocking reproducible python builds for our distro. I hope we can get some movement here after 2 years

@jaraco
Copy link
Member

jaraco commented Nov 25, 2017

Apologies for the delay. It’s the unfortunate situation that passing time affords little benefit for open Setuptools tickets (with the exception of regressions). Pull Requests get the lion’s share of attention.

But I’ll see if I can knock this one out.

@jaraco
Copy link
Member

jaraco commented Nov 25, 2017

I'm trying to understand the issue. By using the patch as a starting point, I'm looking at how dist.install_requires is generated, and it seems to be taking the sequence of requirements directly from the package declaration, and if that declaration was deterministically ordered, so also would be the requires.txt file.

I'll see if I can replicate the finding as reported, but my suspicion is it happens only if the package being built uses a non-deterministically ordered sequence (set or dict) for their install_requires declaration.

@jaraco
Copy link
Member

jaraco commented Nov 25, 2017

Although i pushed that commit with the 'fix' (application of the patch from the bitbucket ticket), I've stripped that commit and I'm pushing the tests separately. Using c461d51, I've determined that the issue exists only when the requirements were declared by the user in a non-deterministically-ordered sequence:

$ tox --hashseed 1983024029 -- -v -k 'install_requires and deterministic and not in_setup_cfg'
python develop-inst-nodeps: /Users/jaraco/Dropbox/code/main/setuptools
python installed: apipkg==1.4,contextlib2==0.5.5,execnet==1.5.0,flake8==3.5.0,mccabe==0.6.1,mock==2.0.0,path.py==10.5,pbr==3.1.1,py==1.5.2,pycodestyle==2.3.1,pyflakes==1.6.0,pytest==3.2.5,pytest-fixture-config==1.2.11,pytest-flake8==0.9.1,pytest-shutil==1.2.11,pytest-virtualenv==1.2.11,six==1.11.0,virtualenv==15.1.0
python runtests: PYTHONHASHSEED='1983024029'
python runtests: commands[0] | py.test -v -k install_requires and deterministic and not in_setup_cfg
====================================== test session starts =======================================
platform darwin -- Python 3.6.3, pytest-3.2.5, py-1.5.2, pluggy-0.4.0 -- /Users/jaraco/Dropbox/code/main/setuptools/.tox/python/bin/python3
cachedir: .cache
rootdir: /Users/jaraco/Dropbox/code/main/setuptools, inifile: pytest.ini
plugins: virtualenv-1.2.11, shutil-1.2.11, flake8-0.9.1
collected 390 items / 2 skipped                                                                   

setuptools/tests/test_egg_info.py::TestEggInfo::test_requires[install_requires_deterministic] PASSED
setuptools/tests/test_egg_info.py::TestEggInfo::test_requires[install_requires_set_deterministic] FAILED
==================================== short test summary info =====================================
SKIP [2] /Users/jaraco/Dropbox/code/main/setuptools/setuptools/tests/test_msvc.py:18: could not import 'distutils.msvc9compiler'

============================================ FAILURES ============================================
_________________ TestEggInfo.test_requires[install_requires_set_deterministic] __________________

self = <setuptools.tests.test_egg_info.TestEggInfo object at 0x10d9369b0>
tmpdir_cwd = local('/Users/jaraco/Dropbox/code/main/setuptools')
env = '/var/folders/c6/v7hnmq453xb6p2dbz1gqc6rr0000gn/T/setuptools-test.ro7wax9k'
requires = 'install_requires={"fake-factory==0.5.2", "pytz"}', use_setup_cfg = False
expected_requires = 'fake-factory==0.5.2\npytz\n', install_cmd_kwargs = {}

    @RequiresTestHelper.parametrize(
        # Format of a test:
        #
        # id
        # install_cmd_kwargs [optional]
        #
        # requires block (when used in setup.py)
        #
        # requires block (when used in setup.cfg)
        #
        # expected contents of requires.txt
    
        '''
            install_requires_deterministic
    
            install_requires=["fake-factory==0.5.2", "pytz"]
    
            [options]
            install_requires =
                fake-factory==0.5.2
                pytz
    
            fake-factory==0.5.2
            pytz
            ''',
    
        '''
            install_requires_set_deterministic
    
            install_requires={{"fake-factory==0.5.2", "pytz"}}
    
            [options]
            install_requires =
                fake-factory==0.5.2
                pytz
    
            fake-factory==0.5.2
            pytz
            ''',
    
        '''
            install_requires_with_marker
    
            install_requires=["barbazquux;{mismatch_marker}"],
    
            [options]
            install_requires =
                barbazquux; {mismatch_marker}
    
            [:{mismatch_marker_alternate}]
            barbazquux
            ''',
    
        '''
            install_requires_with_extra
            {'cmd': ['egg_info']}
    
            install_requires=["barbazquux [test]"],
    
            [options]
            install_requires =
                barbazquux [test]
    
            barbazquux[test]
            ''',
    
        '''
            install_requires_with_extra_and_marker
    
            install_requires=["barbazquux [test]; {mismatch_marker}"],
    
            [options]
            install_requires =
                barbazquux [test]; {mismatch_marker}
    
            [:{mismatch_marker_alternate}]
            barbazquux[test]
            ''',
    
        '''
            setup_requires_with_markers
    
            setup_requires=["barbazquux;{mismatch_marker}"],
    
            [options]
            setup_requires =
                barbazquux; {mismatch_marker}
    
            ''',
    
        '''
            tests_require_with_markers
            {'cmd': ['test'], 'output': "Ran 0 tests in"}
    
            tests_require=["barbazquux;{mismatch_marker}"],
    
            [options]
            tests_require =
                barbazquux; {mismatch_marker}
    
            ''',
    
        '''
            extras_require_with_extra
            {'cmd': ['egg_info']}
    
            extras_require={{"extra": ["barbazquux [test]"]}},
    
            [options.extras_require]
            extra = barbazquux [test]
    
            [extra]
            barbazquux[test]
            ''',
    
        '''
            extras_require_with_extra_and_marker_in_req
    
            extras_require={{"extra": ["barbazquux [test]; {mismatch_marker}"]}},
    
            [options.extras_require]
            extra =
                barbazquux [test]; {mismatch_marker}
    
            [extra]
    
            [extra:{mismatch_marker_alternate}]
            barbazquux[test]
            ''',
    
        # FIXME: ConfigParser does not allow : in key names!
        '''
            extras_require_with_marker
    
            extras_require={{":{mismatch_marker}": ["barbazquux"]}},
    
            @xfail
            [options.extras_require]
            :{mismatch_marker} = barbazquux
    
            [:{mismatch_marker}]
            barbazquux
            ''',
    
        '''
            extras_require_with_marker_in_req
    
            extras_require={{"extra": ["barbazquux; {mismatch_marker}"]}},
    
            [options.extras_require]
            extra =
                barbazquux; {mismatch_marker}
    
            [extra]
    
            [extra:{mismatch_marker_alternate}]
            barbazquux
            ''',
    
        '''
            extras_require_with_empty_section
    
            extras_require={{"empty": []}},
    
            [options.extras_require]
            empty =
    
            [empty]
            ''',
        # Format arguments.
        invalid_marker=invalid_marker,
        mismatch_marker=mismatch_marker,
        mismatch_marker_alternate=mismatch_marker_alternate,
    )
    def test_requires(
            self, tmpdir_cwd, env, requires, use_setup_cfg,
            expected_requires, install_cmd_kwargs):
        self._setup_script_with_requires(requires, use_setup_cfg)
        self._run_install_command(tmpdir_cwd, env, **install_cmd_kwargs)
        egg_info_dir = os.path.join('.', 'foo.egg-info')
        requires_txt = os.path.join(egg_info_dir, 'requires.txt')
        if os.path.exists(requires_txt):
            with open(requires_txt) as fp:
                install_requires = fp.read()
        else:
            install_requires = ''
>       assert install_requires.lstrip() == expected_requires
E       AssertionError: assert 'pytz\nfake-factory==0.5.2\n' == 'fake-factory==0.5.2\npytz\n'
E         + fake-factory==0.5.2
E           pytz
E         - fake-factory==0.5.2

/Users/jaraco/Dropbox/code/main/setuptools/setuptools/tests/test_egg_info.py:407: AssertionError
====================================== 388 tests deselected ======================================
================= 1 failed, 1 passed, 2 skipped, 388 deselected in 15.34 seconds =================
ERROR: InvocationError: '/Users/jaraco/Dropbox/code/main/setuptools/.tox/python/bin/py.test -v -k install_requires and deterministic and not in_setup_cfg'
____________________________________________ summary _____________________________________________
ERROR:   python: commands failed

Therefore, the fix should be to either disallow such declarations or to only re-order the declaration when it's not provided in a deterministic sequence.

@jaraco
Copy link
Member

jaraco commented Nov 25, 2017

According to the docs, install_requires takes a string or list of strings, so the packages that are supplying sets are invalid.

jaraco added a commit that referenced this issue Nov 25, 2017
… non-deterministic order, they may appear in the metadata in non-deterministic order. Ref #458.
@jaraco jaraco closed this as completed in f012485 Nov 25, 2017
@jaraco
Copy link
Member

jaraco commented Nov 25, 2017

Although I considered first releasing a deprecation for this behavior, since the documentation has always stated that install_requires must be list or string, I'm going ahead with simply disallowing the behavior. My instinct is there probably aren't many projects that are using sets for declaring dependencies and if there are they can be readily updated. If we find that's not the case, I may relax this constraint, so please report back here if this change causes issues that can't be easily remedied by updating the offending packages, installing from wheels, or downgrading setuptools (as a last resort).

oesgalha added a commit to oesgalha/python-mcollective that referenced this issue Nov 28, 2017
setuptools > 38.0 don't support unordered sequences in the install_requires param
The set sequence is passed as a list now

See: pypa/setuptools#458
gyp pushed a commit to gyp/pygogo that referenced this issue Mar 1, 2018
Since closing pypa/setuptools#458, setuptools
no longer accepts unordered sets for a dependency lists, which makes any
setup.py call fail with the following error, breaking the installation:

  error in pygogo setup command: 'install_requires' must be a string or list of strings containing valid project/version requirement specifiers; Unordered types are not allowed

Since we don't care about the order at this point, let's just force it to
be a list.

Signed-off-by: Peter Gyongyosi <peter.gyongyosi@balabit.com>
reubano pushed a commit to reubano/pygogo that referenced this issue Apr 2, 2018
Since closing pypa/setuptools#458, setuptools
no longer accepts unordered sets for a dependency lists, which makes any
setup.py call fail with the following error, breaking the installation:

  error in pygogo setup command: 'install_requires' must be a string or list of strings containing valid project/version requirement specifiers; Unordered types are not allowed

Since we don't care about the order at this point, let's just force it to
be a list.

Signed-off-by: Peter Gyongyosi <peter.gyongyosi@balabit.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants