collective / zestreleaser.towncrier

GNU General Public License v2.0
1 stars 1 forks source link

towncrier integration into zestreleaser breaks #6

Closed gyst closed 5 years ago

gyst commented 5 years ago

Ubuntu 16.04 in a docker container. Running bin/towncrier --version unreleased works fine. But then:

app@thor(ploneintranet):~$ bin/fullrelease 
INFO: Starting prerelease.
Run pyroma on the package before tagging? (Y/n)? 
INFO: ------------------------------
INFO: Checking /app
INFO: Found ploneintranet
INFO: ------------------------------
INFO: Setuptools and Distribute support running tests. By specifying a test suite, it's easy to find and run tests both for automated tools and humans.
INFO: ------------------------------
INFO: Final rating: 9/10
INFO: Cottage Cheese
INFO: ------------------------------
Do you want to run check-manifest? (Y/n)? 
lists of files in version control and sdist match
WARNING: Found more than one file, picked the shortest one to change: CHANGES.rst, docs/about/history.rst
Enter version [1.3.15b8]: 
Traceback (most recent call last):
  File "bin/fullrelease", line 387, in <module>
    sys.exit(zest.releaser.fullrelease.main())
  File "/var/tmp/eggs/zest.releaser-6.15.1-py2.7.egg/zest/releaser/fullrelease.py", line 23, in main
    prereleaser.run()
  File "/var/tmp/eggs/zest.releaser-6.15.1-py2.7.egg/zest/releaser/baserelease.py", line 390, in run
    self._run_hooks('middle')
  File "/var/tmp/eggs/zest.releaser-6.15.1-py2.7.egg/zest/releaser/baserelease.py", line 385, in _run_hooks
    utils.run_hooks(self.setup_cfg, which_releaser, when, self.data)
  File "/var/tmp/eggs/zest.releaser-6.15.1-py2.7.egg/zest/releaser/utils.py", line 679, in run_hooks
    run_entry_points(which_releaser, when, data)
  File "/var/tmp/eggs/zest.releaser-6.15.1-py2.7.egg/zest/releaser/utils.py", line 695, in run_entry_points
    plugin(data)
  File "/var/tmp/eggs/zestreleaser.towncrier-1.0.0b3-py2.7.egg/zestreleaser/towncrier/__init__.py", line 97, in call_towncrier
    'Running command to update news: %s', utils.format_command(cmd))
  File "/var/tmp/eggs/zest.releaser-6.15.1-py2.7.egg/zest/releaser/utils.py", line 724, in format_command
    if " " in arg:
TypeError: argument of type 'NoneType' is not iterable
gyst commented 5 years ago

Stepping this bug, it seems that running check-manifest as part of fullrelease borks the sys.argv (which becomes: ['setup.py']. Skipping pyroma and check-manifest bypasses this bug.

Also: this bug only manifests if there's none of the fallback options available. If I . bin/activate before running fullrelease, it will find towncrier not via sys.argv but via distutils.spawn.find_executable('towncrier'). Which IMO masks the bug, but at least it's a workaround so my release process is not blocked.

mauritsvanrees commented 5 years ago

I have looked before, but haven't commented yet. Looking at it again, I don't understand how this can go wrong. It apparently goes wrong in the call_towncrier function:

def call_towncrier(data):
    """Entrypoint: run towncrier when available and configured."""
    if not check_towncrier(data):
        return
    path = _towncrier_executable()
    cmd = [path, '--version', data['new_version'], '--yes']
    logger.info(
        'Running command to update news: %s', utils.format_command(cmd))
    ...

The utils.format_command line breaks if any of the items in cmd is None. Theoretically data['new_version'] could be the one, but this seems unlikely, given that . bin/activate helps, which has nothing to do with new_version.

So presumably path is None, but I don't see how this is possible:

Put differently, the breaking code path can only be reached when check_towncrier returns True, and _towncrier_executable return False. But check_towncrier is basically doing return _towncrier_executable(), except that it has a few more ways to return False.

According to your traceback, the code is doing the impossible...

If you are still interested in debugging this, it could help to get extra verbose output:

bin/fullrelease -v
mauritsvanrees commented 5 years ago

I have released zestreleaser.towncrier 1.0.0 which changes a few of the involved functions. This may or may not help.

gyst commented 5 years ago

The 1.0.0 release does not fix this issue. Like I said, sys.argv gets nuked in the middle, if check-manifest is run.

This may be obscured on your system by towncrier being on your PATH. On my system it's not in my PATH - my bin/fullrelease script finds it by virtue of adding the virtualenv buildout bin dir to it's own system path.

Here's a trace with check-manifest:

app@orion(ploneintranet):~$ bin/fullrelease 
INFO: Starting prerelease.
> /var/tmp/eggs/zestreleaser.towncrier-1.0.0-py2.7.egg/zestreleaser/towncrier/__init__.py(23)_towncrier_executable()
-> releaser_path = os.path.abspath(sys.argv[0])
(Pdb) pp sys.argv
['bin/fullrelease']
(Pdb) c
Run pyroma on the package before tagging? (Y/n)? 
INFO: ------------------------------
INFO: Checking /app
INFO: Found ploneintranet
INFO: ------------------------------
INFO: Setuptools and Distribute support running tests. By specifying a test suite, it's easy to find and run tests both for automated tools and humans.
INFO: ------------------------------
INFO: Final rating: 9/10
INFO: Cottage Cheese
INFO: ------------------------------
Do you want to run check-manifest? (Y/n)? 
lists of files in version control and sdist match
WARNING: Found more than one file, picked the shortest one to change: CHANGES.rst, docs/about/history.rst
Enter version [1.4.2]: 1.4.99.ccc1
> /var/tmp/eggs/zestreleaser.towncrier-1.0.0-py2.7.egg/zestreleaser/towncrier/__init__.py(23)_towncrier_executable()
-> releaser_path = os.path.abspath(sys.argv[0])
(Pdb) pp sys.argv
['setup.py']
(Pdb) c
> /var/tmp/eggs/zestreleaser.towncrier-1.0.0-py2.7.egg/zestreleaser/towncrier/__init__.py(98)call_towncrier()
-> cmd = [path, '--version', data['new_version'], '--yes']
(Pdb) pp path
None
(Pdb) c
Traceback (most recent call last):
  File "bin/fullrelease", line 383, in <module>
    sys.exit(zest.releaser.fullrelease.main())
  File "/var/tmp/eggs/zest.releaser-6.15.1-py2.7.egg/zest/releaser/fullrelease.py", line 23, in main
    prereleaser.run()
  File "/var/tmp/eggs/zest.releaser-6.15.1-py2.7.egg/zest/releaser/baserelease.py", line 390, in run
    self._run_hooks('middle')
  File "/var/tmp/eggs/zest.releaser-6.15.1-py2.7.egg/zest/releaser/baserelease.py", line 385, in _run_hooks
    utils.run_hooks(self.setup_cfg, which_releaser, when, self.data)
  File "/var/tmp/eggs/zest.releaser-6.15.1-py2.7.egg/zest/releaser/utils.py", line 679, in run_hooks
    run_entry_points(which_releaser, when, data)
  File "/var/tmp/eggs/zest.releaser-6.15.1-py2.7.egg/zest/releaser/utils.py", line 695, in run_entry_points
    plugin(data)
  File "/var/tmp/eggs/zestreleaser.towncrier-1.0.0-py2.7.egg/zestreleaser/towncrier/__init__.py", line 98, in call_towncrier
    cmd = [path, '--version', data['new_version'], '--yes']
  File "/var/tmp/eggs/zest.releaser-6.15.1-py2.7.egg/zest/releaser/utils.py", line 724, in format_command
    if " " in arg:
TypeError: argument of type 'NoneType' is not iterable

Here's a trace without check-manifest:

app@orion(ploneintranet):~$ bin/fullrelease 
INFO: Starting prerelease.
> /var/tmp/eggs/zestreleaser.towncrier-1.0.0-py2.7.egg/zestreleaser/towncrier/__init__.py(23)_towncrier_executable()
-> releaser_path = os.path.abspath(sys.argv[0])
(Pdb) pp sys.argv
['bin/fullrelease']
(Pdb) c
Run pyroma on the package before tagging? (Y/n)? n
Do you want to run check-manifest? (Y/n)? n
WARNING: Found more than one file, picked the shortest one to change: CHANGES.rst, docs/about/history.rst
Enter version [1.4.2]: 1.4.99.ccc1
> /var/tmp/eggs/zestreleaser.towncrier-1.0.0-py2.7.egg/zestreleaser/towncrier/__init__.py(23)_towncrier_executable()
-> releaser_path = os.path.abspath(sys.argv[0])
(Pdb) pp sys.argv
['bin/fullrelease']
(Pdb) c
> /var/tmp/eggs/zestreleaser.towncrier-1.0.0-py2.7.egg/zestreleaser/towncrier/__init__.py(98)call_towncrier()
-> cmd = [path, '--version', data['new_version'], '--yes']
(Pdb) pp path
'/app/bin/towncrier'
(Pdb) c
INFO: Running command to update news: /app/bin/towncrier --version 1.4.99.ccc1 --yes
Loading template...
Finding news fragments...
Rendering news fragments...
Writing to newsfile...
Staging newsfile...
Removing news fragments...
Removing the following files:
/app/news/1988.technical
/app/news/2438.changed
/app/news/1988.changed
/app/news/2435.fixed
/app/news/2441.fixed
Done!
mauritsvanrees commented 5 years ago

I managed to reproduce it like this:

check-manifest is innocent. pyroma is the one that messes up the sys.argv, in this line. It tries to save and restore the original sys.argv, but it does not work. I will create an issue and PR for that.

Meanwhile, I will check if a workaround in zestreleaser.towncrier is possible.

mauritsvanrees commented 5 years ago

So there are several options that may work for you:

  1. Don't let zest.releaser call pyroma.
  2. Try pyroma PR #36 to prevent the sys.argv mangling.
  3. Try zestreleaser.towncrier PR #12, which detects the wrong sys.argv and instead tries to find towncrier in the same directory as sys.executable.
mauritsvanrees commented 5 years ago

I think this is solved. I have high hopes that zestreleaser.towncrier 1.2.0 works for everyone.