[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: About fusioninventory-agent packaging



Hi Gregor,

well, I'll really need help regarding your changelog TODOs ^^

I'm taking them back here and make my comments around.

1. d/changelog: there should be only one changelog entry for a new upload, so
    everything after 1:2.3.16-1 should be squashed into this stanza
   d/changelog: as this is the first upload of 2.4, the version should be
    1:2.4-1 (not -3)

I uploaded 2.3.18, 2.3.21 & 2.4, do you mean there should be only one changelog entry for each ?
In fact, there's one for 2.3.18, that should be okay so for this one.
There are 2 entries for 2.3.21, but in fact I still released privately them to some customers. They were published there: http://debian.fusioninventory.org/downloads/
This is the same for 2.4. In fact, I merged my own debian changelog that was itself merged in our github repository.
It could be a problem if debian publishes 2.4-1 and one customer still installed our 2.4-2. It won't be updated until next release.
Btw as 2.4.1 should be release in few days, this won't be a big deal if you don't want I merge my own changelog.

But then do you want only a 2.4 release after the 2.3.16 (with 2.3.18 & 2.3.21 changelog merged in 2.4) ? If yes, I'll have to reset again the repository.

2. bug closing: closing bugs fixed in older versions ("Still fixed in 2.3.15-1")
    should be done via versioned closes in the BTS, not in the changelog of a
    newer upload.

Here I really don't know how to close the bug in BTS. Gonéri is still the defined maintainer on BTS and he missed to close the bug.

3. d/changelog: "Fix litian warnings":
    + typo litian -> lintian
    + mention what changes you made (that's more interesting than which tool
      helped)

Okay I missed the typo sorry. But in fact, and if I remember well, I just fixes the lintian warning found while testing the package build and I noticed everything just after the line... I just removed the entry then as the following lines are telling what was fixed.

4. debian/patches: patches should be unapplied in git, and have proper
    headers (not the autogenerated ones). for quilt:
    https://perl-team.pages.debian.net/howto/quilt.html

Okay, I wasn't aware to not apply the patch in git. I can revert the patch for last 2.4 release, but I can't for 2.3.18 & 2.3.21, unless resetting the repository.
For the patch having propre header, I missed to fix it for 2.3.18 patches (was deleted with 2.3.21 as the problem was fixed upstream). For 2.3.21 & 2.4, I think headers are okay, but of course I could be wrong.
Again, to fix that this means I should reset the repository.

5. the package doesn't build, a test fails:
    #   Failed test 'rpm: parsing'
    #   at t/tasks/inventory/linux/softwares.t line 306.
    # Compared $data->[6]{"INSTALLDATE"}
    #    got : '24/03/2012'
    # expect : '25/03/2012'

I don't have that issue on my own. We had such kind of TZ related issue back in 2016 and it should has been fixed in 2.3.18.  Can you give me more detail about which version is tested and in which context ? Eventually send me back the full output log so I can understand what happened ? Thank you.

6. the package doesn't build twice in a row, as etc/agent.cfg gets modified in
    debian rules. possible fixes: either a proper quilt patch, or making a
    backup of the file and moving it back. for the latter cf. e.g.
    libmodule-extractuse-perl's debian/rules

Yes I was annoyed with that too. I'll try to follow your advice and find a better solution.

7. autopkgtest: the smoke test should run the same tests as during build; cf.
    e.g. debian/tests/pkg-perl/smoke-skip and debian/rules in
    libcatalyst-controller-html-formfu-perl
   autopkgtest: the smoke test fails (should be better with the above
    change); use.t is skipped -> needs debian/tests/pkg-perl/use-name .
    for autopkgtest cf. https://perl-team.pages.debian.net/autopkgtest.html

Okay, I'll follow your advices to fix that.

8. debian/control: Standards-Version: 4.1.4 is the current one
    (in d/changelog you say 4.1.4 :))

Do you mean I forgot to set 4.1.4 in d/control...Weirdly it seems to be the old value 3.9.8 in d/control and I though I set it to 4.1.4...

9. debian/copyright:
    + incomplete, `grep -ir copyright .' shows more
      * for inc/* you can use
        https://perl-team.pages.debian.net/copyright.html#Module%3A%3AInstall
    + years are different from the ones in README.md

Okay I added Module::Install license section in d/copyright.
About the years difference, sorry, the 2.4 was release in 2017 december, and so I agree I should have set it to 2017 in d/copyright.

10. why is the init script removed? is this really necessary?
    if yes, this needs a big fat warning in debian/NEWS

init support has been deprecated upstream and systemd support is now mandatory.
I create a debian/NEWS so to explain the purpose. I need also to tell about
other big changes there after all. Thank you for the suggestion.

11. you're removing conffiles (~ files in /etc), e.g. the initscript but also
     the logrotate file and some cron stuff, maybe more. this requires a bit more: cf.
     dpkg-maintscript-helper(1) (and rm_conffile there)

Okay I'll do.

12. debian/rules: I think there's something to be simplified ... but we can look
     at that later :)
    I'm pretty sure the package doesn't build reproducibly because debian/fix-install.sh
     embeds the output of `uname -r' and `uname -n' which is a Bad Idea™

Yes, I agree. My goal was to make it works, but surely not make it works cleanly. I'll pass some time on that to make it better. I even think I may have to enhance the Makefile.PL directly so every can benefit from the changes I need to do for debian.
To avoid mis-understandings in issue reporting with people from different platforms, we really need to set on which platform was packaged the software.
Do you know what should I do to replace theses uname commands ? Is there any environment variables set by build environment I can simply use ?
I didn't find my way to a good answer into documentation.

Cheers,

Guillaume Bougard 
Ingénieur R&D 
gbougard@teclib.com 

TECLIB' Montpellier 
3 rue Doria, 
34000 Montpellier, France

----- Mail original -----
De: "gregor herrmann" <gregoa@debian.org>
À: "Guillaume Bougard" <gbougard@teclib.com>
Cc: "Ignacio" <ignacio600@gmail.com>, "Debian Perl List" <debian-perl@lists.debian.org>
Envoyé: Vendredi 22 Juin 2018 21:06:36
Objet: Re: About fusioninventory-agent packaging

On Fri, 22 Jun 2018 19:13:59 +0200, Guillaume Bougard wrote:

> I've finished the package update. The 2.4-3 release seems ready for me. I even fixed building issues that I merged upstream.

For those reading along at home:
I've added my review to d/changelog in git and mentioned it on IRC
already (before seeing this mail :))
 
Cheers,
gregor

-- 
 .''`.  https://info.comodo.priv.at -- Debian Developer https://www.debian.org
 : :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D  85FA BB3A 6801 8649 AA06
 `. `'  Member VIBE!AT & SPI Inc. -- Supporter Free Software Foundation Europe
   `-   NP: Tracy Chapman: Speak The Word


Reply to: