Skip to content

Conversation

@npostavs
Copy link
Contributor

@npostavs npostavs commented Nov 26, 2021

Short description of changes

Use dch to generate Debian changelog instead of handrolled shell calls

Context: Fixes an issue?

This is an alternate version of #1767

Does this change need documentation? What needs to be documented and how?

No.

Status of this Pull Request

Working implementation, a sample output is changelog.dch.log. (updated with Nov 29th changes)

What is missing until this pull request can be merged?

Possibly the "initial release" line needs to be removed somehow.

Checklist

  • I've verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want
  • My code follows the style guide
  • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • I've filled all the content above

@softins
Copy link
Member

softins commented Nov 29, 2021

Firstly, thank you @npostavs for addressing the build warnings about the debian/changelog entry errors, which had been bothering me for some time. However, I think the proposed change is incomplete, as it introduces other issues:

  1. The modification to .github/actions_scripts/getChangelog.pl changes the output from MarkDown to a single line per change.

    This is certainly useful for passing each line to dch, but the getChangeLog.pl script is also used in .github/actions_scripts/analyse_git_reference.py to create a MarkDown file containing the version release notes. analyse_git_reference.py is called from .github/workflows/autobuild.yml in the section create_release.

    The new output format will not be compatible with this requirement.

    Maybe instead leave getChangeLog.pl unchanged, and create instead a new script (e.g. getChangeLines.pl?) for use with dch, which could be called from distributions/build-debian-package-auto.sh? Or else provide a parameter to the script?

  2. The debian/changelog produced by the new version of build-debian-package-auto.sh changes the maintainer information from what was used previously. After some experimentation, I found that I preferred the output if I changed the dch lines as follows:

    DEBFULLNAME=GitHubActions DEBEMAIL=noemail@example.com dch --nomultimaint --newversion "${VERSION}" ''
    

    (or some appropriate values - the above matches the old script) and:

    dch --nomultimaint "$entry"
    

@softins
Copy link
Member

softins commented Nov 29, 2021

After further experimentation, this is the current state of my build-debian-package-auto.sh:

#!/bin/sh -e

# Create deb files

cp -r debian ..
cd ..

# get the jamulus version from pro file
VERSION=$(cat Jamulus.pro | grep -oP 'VERSION = \K\w[^\s\\]*')

export DEBFULLNAME=GitHubActions DEBEMAIL=noemail@example.com

# Generate Changelog
echo -n generating changelog
rm -f debian/changelog
dch --create --package jamulus --empty --newversion "${VERSION}" ''
perl .github/actions_scripts/getChangelog.pl ChangeLog "${VERSION}" | while read entry
do
    echo -n .
    dch "$entry"
done
echo

echo "${VERSION} building..."

sed -i "s/é&%JAMVERSION%&è/${VERSION}/g" debian/control

debuild --preserve-env -b -us -uc

It uses --create, --package, --newversion and --empty to create a new debian/changelog file. The use of --empty causes a warning, which the man page says is expected, but it does avoid the spurious "Initial version" log entry.

It exports DEBFULLNAME and DEBEMAIL so they are available to all calls of dch. This also means --nomultimaint is not necessary.

@npostavs
Copy link
Contributor Author

  1. but the getChangeLog.pl script is also used in .github/actions_scripts/analyse_git_reference.py

Ah, I missed that.

Or else provide a parameter to the script?

I went with this option.

After further experimentation, this is the current state of my build-debian-package-auto.sh:

Added to the PR.

@ann0see
Copy link
Member

ann0see commented Dec 16, 2021

@npostavs what is the Next action on this? Review, testing?

@npostavs
Copy link
Contributor Author

Yeah, I believe the code is ready, and I have addressed all of @softin's concerns.

Copy link
Member

@softins softins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the one change required. Everything else looks ok, and I have successfully built a deb.

Copy link
Member

@ann0see ann0see left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor thought. It’s still not optimal, but probably better then now.

Copy link
Member

@softins softins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine now, thank you!

Copy link
Member

@ann0see ann0see left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you also test if the automated change log detection for GH releases still works? Seems ok, but better test it.

Urgh. Didn't read... Should be ok...


my $logversion = "";
my $entry;
while (my $line = <$fh>) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should stop looping through the file if we already found the correct version ($logversion changed to the next version). Then we could also check if a version wasn't found as soon as we reached the EOF and output some error message.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@npostavs could you please insert a break statement to stop looping through unneeded lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yep, done.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!


my $logversion = "";
my $entry;
while (my $line = <$fh>) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@npostavs could you please insert a break statement to stop looping through unneeded lines?

Also exit early once we're done processing the version relevant lines.
Copy link
Member

@ann0see ann0see left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants