Bug#874718: debchange doesn't allow empty changelog entry

2017-09-10 Thread Osamu Aoki
Hi

Good news mostly but some concern on pylint, now.

On Sun, Sep 10, 2017 at 12:00:08AM +0100, Adam D. Barratt wrote:
> Control: retitle -1 dch "" should not create maintainer header
> Control: tags -1 + patch

Title: I agree.
Patch: I committed a slightly different patch.

Your proposal:
> diff --git a/scripts/debchange.pl b/scripts/debchange.pl
> index b44b9633..444bb399 100755
> --- a/scripts/debchange.pl
> +++ b/scripts/debchange.pl
> @@ -1411,7 +1411,7 @@ if (($opt_r || $opt_a || $merge) && ! $opt_create) {
>  
>  if (! $opt_r) {
> # Add a multi-maintainer header...
> -   if ($multimaint) {
> +   if ($multimaint and not $EMPTY_TEXT) {
> # ...unless there already is one for this maintainer.
> if (!defined $maintline) {
> print O "\n  [ $MAINTAINER ]\n";

I agree this works for the case I discussed.  But looking at the code
after it, I decided to make this logic a bit longer to be ion the safe
side.

  df9e22fb ("debchange: update changelog entry correctly", 2017-09-10)

> (an alternative would be having the preceding checks skip the block
> that potentially sets $multimaint to 1 if $EMPTY_TEXT is set)

This may be an option if you think through.  But such complicated logic
is prone for breakage.  I just "or"ed all cases following to cover
additional entry is created.

My proposal:
-   if ($multimaint) {
+   if ($multimaint and
+   (@closes_text or $TEXT or $opt_news or !$EMPTY_TEXT)) {
# ...unless there already is one for this maintainer.
if (!defined $maintline) {
print O "\n  [ $MAINTAINER ]\n";

So far I see no warning related to my changes any more.

pylink is still causing warning (python3.5 and python3.6) over crashed
and skipped testVgq:

| python3.6 setup.py test
| running test
| running egg_info
| creating devscripts.egg-info
| writing devscripts.egg-info/PKG-INFO
| writing dependency_links to devscripts.egg-info/dependency_links.txt
| writing top-level names to devscripts.egg-info/top_level.txt
| writing manifest file 'devscripts.egg-info/SOURCES.txt'
| reading manifest file 'devscripts.egg-info/SOURCES.txt'
| writing manifest file 'devscripts.egg-info/SOURCES.txt'
| running build_ext
| test_debdiff-apply (devscripts.test.test_help.HelpTestCase) ... 
/usr/lib/python3.6/unittest/case.py:605: ResourceWarning: unclosed file 
<_io.BufferedReader name=4>
|   testMethod()
| /usr/lib/python3.6/unittest/case.py:605: ResourceWarning: unclosed file 
<_io.BufferedReader name=6>
|   testMethod()
| ok
| test_reproducible-check (devscripts.test.test_help.HelpTestCase) ... ok
| test_sadt (devscripts.test.test_help.HelpTestCase) ... ok
| test_suspicious-source (devscripts.test.test_help.HelpTestCase) ... ok
| test_wrap-and-sort (devscripts.test.test_help.HelpTestCase) ... ok
| testArgs (devscripts.test.test_logger.LoggerTestCase) ... ok
| testCommand (devscripts.test.test_logger.LoggerTestCase) ... ok
| testNoArgs (devscripts.test.test_logger.LoggerTestCase) ... ok
| test_pylint (devscripts.test.test_pylint.PylintTestCase)
| Test: Run pylint on Python source code ... skipped 'pylint crashed :/'
| 
| --
| Ran 9 tests in 0.475s
| 
| OK (skipped=1)
| python3.5 setup.py test
| running test
| running egg_info
| writing devscripts.egg-info/PKG-INFO
| writing top-level names to devscripts.egg-info/top_level.txt
| writing dependency_links to devscripts.egg-info/dependency_links.txt
| reading manifest file 'devscripts.egg-info/SOURCES.txt'
| writing manifest file 'devscripts.egg-info/SOURCES.txt'
| running build_ext
| test_debdiff-apply (devscripts.test.test_help.HelpTestCase) ... 
/usr/lib/python3.5/unittest/case.py:605: ResourceWarning: unclosed file 
<_io.BufferedReader name=6>
|   testMethod()
| /usr/lib/python3.5/unittest/case.py:605: ResourceWarning: unclosed file 
<_io.BufferedReader name=4>
|   testMethod()
| ok
| test_reproducible-check (devscripts.test.test_help.HelpTestCase) ... ok
| test_sadt (devscripts.test.test_help.HelpTestCase) ... ok
| test_suspicious-source (devscripts.test.test_help.HelpTestCase) ... ok
| test_wrap-and-sort (devscripts.test.test_help.HelpTestCase) ... ok
| testArgs (devscripts.test.test_logger.LoggerTestCase) ... ok
| testCommand (devscripts.test.test_logger.LoggerTestCase) ... ok
| testNoArgs (devscripts.test.test_logger.LoggerTestCase) ... ok
| test_pylint (devscripts.test.test_pylint.PylintTestCase)
| Test: Run pylint on Python source code ... skipped 'pylint crashed :/'
| 
| --
| Ran 9 tests in 0.467s
| 
| OK (skipped=1)

Regards,

Osamu



Bug#874718: debchange doesn't allow empty changelog entry

2017-09-09 Thread Adam D. Barratt
Control: retitle -1 dch "" should not create maintainer header
Control: tags -1 + patch

On Sun, 2017-09-10 at 07:37 +0900, Osamu Aoki wrote:
> control: retitle -1 dch -u "" creates undesired changelog entry
> 
> Hi,
> 
> On Sat, Sep 09, 2017 at 10:22:14AM +0100, Adam D. Barratt wrote:
[...]
On Sat, 2017-09-09 at 15:08 +0900, Osamu Aoki wrote:
> > > If "" is the text to be added to the changelog, debchange skips
> > > making line
> > > with "*".  This seems to intentional design decision of debchange
> > > which I have
> > > no idea why.  (Code logic around it is a bit complicated.  So I
> > > may
> > > be wrong)
> > 
> > [...]
> > > If no objection, I will try to update debchange.
> > 
> > The changelog says why:
> > 
> > + Allow an explicit empty changelog entry to be passed on the
> > command line
> >   to allow non-interactive changes to the distribution and
> > urgency without
> >   adding a changelog entry (Closes: #442267)
> 
> Thanks for a pointer. 
> 
> > Is what you're looking for a valid changelog stanza that doesn't
> > contain an actual message? If so, would "dch ' '" suffice? (It
> > doesn't
> > add a space after the "*", and I can't remember if that's
> > intentional,
> > but it _does_ produce a changelog that dpkg-parsechangelog is happy
> > with.)
> 
> OK I didn't realize white space trick.  I was using null string.
> So now I know the solution for uupdate but still see issues with
> debchange.
> 
> This null string as argument is supposed to avoid interactive session
> like the case without text while not adding any extra line like space
> as
> text.
> 
>  $ dch -u high ""
> 
> This should simply update urgency.  But it doesn't do so.
[...]
> +  [ Osamu Aoki ]
> +

Right, yes, that looks like a bug indeed. I was about to suggest adding
--nomultimaint to your invocation in order to suppress it, but realised
that won't work - and is even documented as such. :-(

>From a quick test, I think this will work:

diff --git a/scripts/debchange.pl b/scripts/debchange.pl
index b44b9633..444bb399 100755
--- a/scripts/debchange.pl
+++ b/scripts/debchange.pl
@@ -1411,7 +1411,7 @@ if (($opt_r || $opt_a || $merge) && ! $opt_create) {
 
 if (! $opt_r) {
# Add a multi-maintainer header...
-   if ($multimaint) {
+   if ($multimaint and not $EMPTY_TEXT) {
# ...unless there already is one for this maintainer.
if (!defined $maintline) {
print O "\n  [ $MAINTAINER ]\n";

(an alternative would be having the preceding checks skip the block
that potentially sets $multimaint to 1 if $EMPTY_TEXT is set)

Regards,

Adam



Bug#874718: debchange doesn't allow empty changelog entry

2017-09-09 Thread Osamu Aoki
control: retitle -1 dch -u "" creates undesired changelog entry

Hi,

On Sat, Sep 09, 2017 at 10:22:14AM +0100, Adam D. Barratt wrote:
> [You appear to have filed this bug 3 times. Maybe you want to close
> #874717 and #874716?]

I guess I need to fix my mailer set up.
(This mail may be tripled.)
I will close duplicates.
 
> On Sat, 2017-09-09 at 15:08 +0900, Osamu Aoki wrote:
> > If "" is the text to be added to the changelog, debchange skips
> > making line
> > with "*".  This seems to intentional design decision of debchange
> > which I have
> > no idea why.  (Code logic around it is a bit complicated.  So I may
> > be wrong)
> [...]
> > If no objection, I will try to update debchange.
> 
> The changelog says why:
> 
> + Allow an explicit empty changelog entry to be passed on the command line
>   to allow non-interactive changes to the distribution and urgency without
>   adding a changelog entry (Closes: #442267)

Thanks for a pointer. 

> Is what you're looking for a valid changelog stanza that doesn't
> contain an actual message? If so, would "dch ' '" suffice? (It doesn't
> add a space after the "*", and I can't remember if that's intentional,
> but it _does_ produce a changelog that dpkg-parsechangelog is happy
> with.)

OK I didn't realize white space trick.  I was using null string.
So now I know the solution for uupdate but still see issues with
debchange.

This null string as argument is supposed to avoid interactive session
like the case without text while not adding any extra line like space as
text.

 $ dch -u high ""

This should simply update urgency.  But it doesn't do so.

Please look into attached tared git repository with test case results.
Look at branch osamu-null-u.  All but test01 cases create bad changelog
entry.  The following type of addition should not happen.

+  [ Osamu Aoki ]
+

Regards,

Osamu



dch-tests.tar.xz
Description: application/xz


Bug#874718: debchange doesn't allow empty changelog entry

2017-09-09 Thread Adam D. Barratt
[You appear to have filed this bug 3 times. Maybe you want to close
#874717 and #874716?]

On Sat, 2017-09-09 at 15:08 +0900, Osamu Aoki wrote:
> If "" is the text to be added to the changelog, debchange skips
> making line
> with "*".  This seems to intentional design decision of debchange
> which I have
> no idea why.  (Code logic around it is a bit complicated.  So I may
> be wrong)
[...]
> If no objection, I will try to update debchange.

The changelog says why:

+ Allow an explicit empty changelog entry to be passed on the command line
  to allow non-interactive changes to the distribution and urgency without
  adding a changelog entry (Closes: #442267)

Is what you're looking for a valid changelog stanza that doesn't
contain an actual message? If so, would "dch ' '" suffice? (It doesn't
add a space after the "*", and I can't remember if that's intentional,
but it _does_ produce a changelog that dpkg-parsechangelog is happy
with.)

Regards,

Adam