[generate_changelog] Better err message for wrong tag syntax.

[130s]

For the situation where #142 is an issue, catkin_generate_changelog fails with an unintuitive error. Turned out it's because of the previous git's tag that doesn't follow the catkin's (?) requirement.

BEFORE this PR:

$ catkin_generate_changelog            
Found packages: aaa_msgs, aaa_support
Querying commit information since latest tag...
Updating forthcoming section of changelog files...
- updating 'aaa_support/CHANGELOG.rst'
Traceback (most recent call last):
  File "/usr/bin/catkin_generate_changelog", line 9, in <module>
    load_entry_point('catkin-pkg==0.4.1', 'console_scripts', 'catkin_generate_changelog')()
  File "/usr/lib/python2.7/dist-packages/catkin_pkg/cli/generate_changelog.py", line 125, in main_catching_runtime_error
    main(*args, **kwargs)
  File "/usr/lib/python2.7/dist-packages/catkin_pkg/cli/generate_changelog.py", line 118, in main
    update_changelogs(base_path, packages_with, tag2log_entries, logger=logging, vcs_client=vcs_client, skip_contributors=args.skip_contributors)
  File "/usr/lib/python2.7/dist-packages/catkin_pkg/changelog_generator.py", line 124, in update_changelogs
    data = update_changelog_file(data, pkg_tag2log_entries, vcs_client=vcs_client, skip_contributors=skip_contributors)
  File "/usr/lib/python2.7/dist-packages/catkin_pkg/changelog_generator.py", line 180, in update_changelog_file
    raise RuntimeError('Could not find section "%s"' % next_tag.name)
UnboundLocalError: local variable 'next_tag' referenced before assignment

$ apt-cache policy ros-kinetic-catkin
ros-kinetic-catkin:
  Installed: 0.7.8-0xenial-20171104-171232-0800
  Candidate: 0.7.8-0xenial-20171104-171232-0800
  Version table:
 *** 0.7.8-0xenial-20171104-171232-0800 500
        500 http://packages.ros.org/ros/ubuntu xenial/main amd64 Packages
        100 /var/lib/dpkg/status

AFTER this PR:

$ catkin_generate_changelog 
Found packages: aaa_msgs, aaa_support
Querying commit information since latest tag...
ERROR: Tag name 'v1.0.0_rc1' failing to meet criteria.

[dirk-thomas]

Please provide the command / repo to reproduce the problem.

[130s]

You can reproduce the issue using this step and my sample ros_comm repo. Note that I had to remove all tags to let this issue happen; with other tags I still could generate the changelog. So I don't know exact condiiton where this issue occurs. Still, whenever it happens the better error msg helps.

$ mkdir -p /tmp/catkin_gen_samp/src && cd /tmp/catkin_gen_samp/src/
$ git clone https://github.com/130s/ros_comm -b l/catkingpkg_207
:
$ cd ros_comm/
$ catkin_generate_changelog                          
Found packages: ros_comm
Querying commit information since latest tag...
Updating forthcoming section of changelog files...
- updating 'ros_comm/CHANGELOG.rst'
Traceback (most recent call last):
  File "/usr/bin/catkin_generate_changelog", line 9, in <module>
    load_entry_point('catkin-pkg==0.4.1', 'console_scripts', 'catkin_generate_changelog')()
  File "/usr/lib/python2.7/dist-packages/catkin_pkg/cli/generate_changelog.py", line 125, in main_catching_runtime_error
    main(*args, **kwargs)
  File "/usr/lib/python2.7/dist-packages/catkin_pkg/cli/generate_changelog.py", line 118, in main
    update_changelogs(base_path, packages_with, tag2log_entries, logger=logging, vcs_client=vcs_client, skip_contributors=args.skip_contributors)
  File "/usr/lib/python2.7/dist-packages/catkin_pkg/changelog_generator.py", line 124, in update_changelogs
    data = update_changelog_file(data, pkg_tag2log_entries, vcs_client=vcs_client, skip_contributors=skip_contributors)
  File "/usr/lib/python2.7/dist-packages/catkin_pkg/changelog_generator.py", line 180, in update_changelog_file
    raise RuntimeError('Could not find section "%s"' % next_tag.name)
UnboundLocalError: local variable 'next_tag' referenced before assignment

[bmagyar]

I'm seeing the same trying to generate changelogs for geographic-info.

It can be discussed what should be the action to take on parsing off-policy tags but there is a Python crash to be fixed first.

Simplified code from here:

for next_tag in list(tags)[i:]:
  blabla
if not match:
  if tag.name is None:
    raise RuntimeError('Could not find section "%s"' % next_tag.name)
  else:
    raise RuntimeError('Could neither find section "%s" nor any other section' % tag.name)

If list(tags)[i:] is empty, next_tag will never have a valid value assigned. Please correct me if I'm wrong.

I'm using:

apt-cache policy ros-kinetic-catkin
ros-kinetic-catkin:
  Installed: 0.7.11-0xenial-20180222-175501-0800
  Candidate: 0.7.11-0xenial-20180222-175501-0800
  Version table:
 *** 0.7.11-0xenial-20180222-175501-0800 500
        500 http://packages.ros.org/ros/ubuntu xenial/main amd64 Packages
        100 /var/lib/dpkg/status

[bmagyar]

On the same note, Python3 won't allow for this kind of control loop variable leaking to be used:

Also note that list comprehensions have different semantics: they are closer to syntactic sugar for a generator expression inside a list() constructor, and in particular the loop control variables are no longer leaked into the surrounding scope .

https://docs.python.org/3.0/whatsnew/3.0.html#changed-syntax

[130s]

Please review again as I think I addressed https://github.com/ros-infrastructure/catkin_pkg/pull/207#pullrequestreview-106975352.

[dirk-thomas]

Thank you for addressing the previous comments. Looking at the big picture again I don't think this PR addresses the original problem to be able to generate changelogs for non-standard tags.

Failing with a better error message is certainly good. But the problem UnboundLocalError: local variable 'next_tag' referenced before assignment should probably be addressed in the code part which raises that exception. The variable next_tag isn't defined when the loop has no tags to iterate over.

And to close the original issue the code should successfully generate a changelog for the non-standard tag.

[130s]

Not an excuse but with this PR I didn't intend to address the original suggestion in #142, which is to allow custom tag. In fact I tried to look into it, then I noticed https://github.com/ros-infrastructure/catkin_pkg/issues/142#issuecomment-393502476 so stopped there as I haven't come up with a solution.

Instead, I only intend to workaround the unintuitive error so that users can spend less time debugging their issues.

Failing with a better error message is certainly good. But the problem UnboundLocalError: local variable 'next_tag' referenced before assignment should probably be addressed in the code part which raises that exception. The variable next_tag isn't defined when the loop has no tags to iterate over.

Hope the updated commit addresses this part.

[130s]

I hope I understand the review comments better. Commit updated.

Now the whole catkin_generate_changelog process ends earlier when an invalid tag is found.

$ export PYTHONPATH=~/ROS/ncws/src/ros-infrastructure/catkin_pkg/src:$PYTHONPATH 
$ mkdir -p /tmp/catkin_gen_samp/src && cd /tmp/catkin_gen_samp/src/
$ git clone https://github.com/130s/ros_comm -b l/catkingpkg_207
$ cd ros_comm/
$ git tag
v1.13.6_rc1
$ catkin_generate_changelog               
Found packages: ros_comm
Querying commit information since latest tag...
ERROR: The tag name 'v1.13.6_rc1' doesn't match the version pattern x.y.z

[130s]

A CI job failed saying the following. I can download the file from the URL, so may be just a temporary glitch?

3.5 is not installed; attempting download
Downloading archive: https://s3.amazonaws.com/travis-python-archives/binaries/ubuntu/14.04/x86_64/python-3.5.tar.bz2
$ curl -sSf -o python-3.5.tar.bz2 ${archive_url}
curl: (56) SSL read: error:00000000:lib(0):func(0):reason(0), errno 104
Unable to download 3.5 archive. The archive may not exist. Please consider a different version.

[130s]

Addressed review comments.

[dirk-thomas]

Thank you.