Please merge nut 2.7.2-4 (main) from debian (unstable)

Bug #1522346 reported by Louis Bouchard
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
nut (Ubuntu)
Fix Released
Wishlist
Louis Bouchard

Bug Description

merge bug

Tags: patch
Louis Bouchard (louis)
Changed in nut (Ubuntu):
status: New → In Progress
importance: Undecided → Wishlist
assignee: nobody → Louis Bouchard (louis-bouchard)
Revision history for this message
Louis Bouchard (louis) wrote :

debian upstream to merged 2.7.2-4ubuntu1 debdiff

Revision history for this message
Louis Bouchard (louis) wrote :

ubuntu wily to Xenial debdiff

Changed in nut (Ubuntu):
status: In Progress → Confirmed
assignee: Louis Bouchard (louis-bouchard) → nobody
tags: added: patch
Revision history for this message
Robie Basak (racb) wrote :

I'll take this review (in progress).

Revision history for this message
Robie Basak (racb) wrote :

Hi Louis,

Thank you for working on this merge. Good job identifying some changes that
could be dropped.

However I think this work could do with some improvement before I am happy to
upload it.

Overall it looks like it might be OK, but I'm concerned that the changelog is
misleading. There are changes that are being carried forward that are not
mentioned in your Remaining Changes section, and conversely there are also
changes that are claimed to have been carried forward but are partially
missing. Details below:

udev changes in debian/rules. Was this dropped? In your merge the delta still
seems to be there, and it still looks relevant. So why does the changelog say
that this is dropped?

Some but not all changes made in 2.7.1-1ubuntu5 seem to have disappeared. Are
they still needed? Why is this part of the delta not mentioned in the
changelog?

What about these parts of the previous delta, for which I don't see any
information in your merge changelog about whether these were carried forward or
dropped?

      * Disable systemd unit; it does not check nut.conf whether nut is
        configured, and thus fails to start (and the package install). The init.d
        script works well enough for the time being. (LP: #1313231)

      * debian/tests/test-nut.py: in the CVE_2012_2944 test, give nut at most 5
        seconds to shut down, instead of expecting it to be shut down immediately
        after sending the killall. (LP: #1291378)

      * debian/patches/0006-ups-conf-reorder.patch: Move maxretry setting
        above Examples section, closer to the "outside of a driver definition"
        comment. (LP: #1405822)

Please could you address these questions above? In particular I'd like the changelog to be accurate in explaining what is being carried forward, what is being dropped and any additional changes made (I think there are none in this case). Otherwise it's tough both to review now and for a future merge.

Minor: I'd put the extra Ubuntu delta lines for debian/patches/series after all
the Debian ones, unless there's a good reason to insert them into the middle.

Steve Langasek (vorlon)
Changed in nut (Ubuntu):
assignee: nobody → Steve Langasek (vorlon)
Revision history for this message
Steve Langasek (vorlon) wrote :

Louis, I second Robie's comments. Here is an interdiff between your merge, and what I think the merge should be. This addresses Robie's comments about the changelog having incomplete documentation of the remaining delta; it also includes the following changes relative to your merge diff:

 - nut-{cgi,server}.postinst: the existing delta with Debian is bad form, adduser is designed to be called unconditionally. Not strictly speaking part of the merge, but something I spotted along the way that I think it's important to fix. Along the way, I fixed up the changelog explanation for why this is still needed (since we no longer do upgrades from hardy, if that were the only reason we could drop the delta).
 - debian/patches/backport-fix-lp753661.patch has been a pointless diff for several upstream releases. The patch still applies, but upstream fixed the issue differently (using rtrim() ).
 - as Robie noted, you say the udev delta is dropped, but the delta to debian/rules still exists. I've dropped the delta, with more explanation in the changelog to why it can be dropped.
 - the changes from 2.7.7-1ubuntu5 are restored, as still required.

Please have a look at my delta, and let me know if you agree with these changes. If you see anything you disagree with, let's discuss. If you do agree with these changes, please make sure you understand the source of these differences on your side. Then I can sponsor the upload if we're in agreement.

BTW, one thing I notice in the Ubuntu delta that I didn't yet address in the merge is that the apport hooks are installed in wrong paths. There is no source package named nut-client or nut-server, installing those files is a no-op. Also not a question of the merge, but I would gladly take a patch fixing this as part of sponsoring the upload.

Revision history for this message
Louis Bouchard (louis) wrote : Re: [Bug 1522346] Re: Please merge nut 2.7.2-4 (main) from debian (unstable)
Download full text (5.0 KiB)

Hello Robie, Steve,

I will address both of your updates sequentially :

Le 14/01/2016 19:13, Robie Basak a écrit :
> Hi Louis,
>
> Thank you for working on this merge. Good job identifying some changes that
> could be dropped.
>
> However I think this work could do with some improvement before I am happy to
> upload it.
>
> Overall it looks like it might be OK, but I'm concerned that the changelog is
> misleading. There are changes that are being carried forward that are not
> mentioned in your Remaining Changes section, and conversely there are also
> changes that are claimed to have been carried forward but are partially
> missing. Details below:
>
> udev changes in debian/rules. Was this dropped? In your merge the delta still
> seems to be there, and it still looks relevant. So why does the changelog say
> that this is dropped?

Oh dear! I mistakenly thought that the change was coming from upstream as there
is no mention in grab-merge's REPORT file of any conflict on debian/rules. I
will revert the changelog entry accordingly.

I definitively will stop to use this tool for merges. Not only it makes my life
miserable by having to systematically revert to pristine source packages for
comparaison, but it seems to silently changes files without any indication.

So far, grab-merge has been a source of problems for me. I had to restart the
rsyslog merge twice because of mismatch and build problems.

I may need your help to get my head around it, but I will give a second look at
your git workflow that I bookmarked earlier.
>
> Some but not all changes made in 2.7.1-1ubuntu5 seem to have disappeared. Are
> they still needed? Why is this part of the delta not mentioned in the
> changelog?
>

Indeed, the .ubuntu5 modification that you uploaded got dropped. I was mislead
to believe that it was an upstream change added and not dropped from our part. I
will reintroduce it.

I also added a mention of dropped changes in debian/tests/test-nut.py that are
now superseded upstream but had no mention in the previous changelog.

> What about these parts of the previous delta, for which I don't see any
> information in your merge changelog about whether these were carried forward or
> dropped?
>
> * Disable systemd unit; it does not check nut.conf whether nut is
> configured, and thus fails to start (and the package install). The init.d
> script works well enough for the time being. (LP: #1313231)
>
> * debian/tests/test-nut.py: in the CVE_2012_2944 test, give nut at most 5
> seconds to shut down, instead of expecting it to be shut down immediately
> after sending the killall. (LP: #1291378)
>
> * debian/patches/0006-ups-conf-reorder.patch: Move maxretry setting
> above Examples section, closer to the "outside of a driver definition"
> comment. (LP: #1405822)
>

My changelog mention the following :

    * Ubuntu bugs fixed upstream:

      LP: #1405822, #1291378, #1313231

Those are the three bugs that you mention above. I do agree that I should have
carried forward the previous changelog description and state that the bug had
been fixed upstream. I'll carry that change in a new debdi...

Read more...

Revision history for this message
Louis Bouchard (louis) wrote :
Revision history for this message
Louis Bouchard (louis) wrote :
Louis Bouchard (louis)
Changed in nut (Ubuntu):
assignee: Steve Langasek (vorlon) → Louis Bouchard (louis-bouchard)
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package nut - 2.7.2-4ubuntu1

---------------
nut (2.7.2-4ubuntu1) xenial; urgency=low

  * Merge with Debian; remaining changes (LP: #1522346) :
    - debian/control:
      + remove Build-Depends on libpowerman0-dev
      + remove nut-powerman-pdu
    - debian/nut-powerman-pdu.{install,manpages}: dropped for now
    - debian/{nut-cgi,nut-server}.postinst: add nut to the group
      unconditonally, for compatibility with UPSes connected by serial port.
    - debian/source_nut.py, debian/{nut,nut-server,nut-client}.install,
      debian/rules: Install apport hooks for all top-level nut packages.
    - Patch libtool.m4 and configure to support ppc64le.
    - Disable systemd unit; it does not check nut.conf whether nut is
      configured, and thus fails to start (and the package install). The
      init.d script works well enough for the time being.
    - debian/tests/test-nut.py: in the CVE_2012_2944 test, give nut at most
      + 5 seconds to shut down, instead of expecting it to be shut down
        immediately after sending the killall.
      + Additional indication on how to run tests for oneiric, precise to
        vivid and wily+
    - debian/patches/0006-ups-conf-reorder.patch: Move maxretry setting
      above Examples section.
    - debian/tests/control : Fix dep8 test failures:
       + Drop python-unit from dep8 dependencies since the python-test
         package has now been removed. The unittest module has shipped with
         Python since 2.1.
       + Add dep8 test dependency on python, since Python 2 is required and
         is not necessarily installed by default any more.

   * Dropped changes:
     - Fix linking libupsclient : Superseded upstream
     - debian/patches/backport-fix-lp753661.patch: extraneous end-of-line now
       fixed differently upstream.
     - debian/rules: Use udev version for Ubuntu; delta no longer needed as
       both versions are earlier than the earliest supported udev in Ubuntu.

  [ Steve Langasek ]
  * debian/nut-{cgi,server}.postinst: fix 'adduser' logic to be
    unconditional, since adduser is guaranteed to be idempotent.

 -- Louis Bouchard <email address hidden> Thu, 03 Dec 2015 12:37:13 +0100

Changed in nut (Ubuntu):
status: Confirmed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.