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

Re: About fusioninventory-agent packaging

On Thu, 28 Jun 2018 16:53:18 +0200, Guillaume Bougard wrote:

> Hello Gregor,

> I finished to review the TODOs and pushed the last commit with unstable in place of UNRELEASED.

Great, thanks.

I just reviewed the package, and we're really to close to an upload,
I think. I left a few minor TODO items in d/changelog again.
> Here are few explanations about the choices I did.

> > > > 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'

> > Thank you, this should help to fix the issue... and add a patch :)
> Well here I tried hardly in different context but I failed to
> reproduce the case.
> Then to avoid losting more time, and as the failing test purpose
> was only to test the rpm command output parsing result which is
> definitively not relevant under debian, I decided to apply a patch
> to simply skip that test under debian.

Fair enough.
Question to others: What's the best way to detect (in a Perl test) if
the test is run on a Debian system?
> > > 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.
> Indeed I failed here to find my way. I even tried to install some
> chroot to do the tests myself, but the
> https://perl-team.pages.debian.net/autopkgtest.html page seems
> completely outdated as few commands fail.

I'll take this to a new thread.
> By the way, fusioninventory-agent being an application,
> FusionInventory::Agent module is not installed in the system perl
> library paths (this was historical and may change later). Then this
> makes the tests failing anyway.

Maybe the smoke tests but the others might be relevant as well.
> So I just decided to remove the TestSuite field and I think
> fusioninventory-agent is not a good candidate for this test.
> I hope this won't compromise the debian release and I understand it
> surely delays the merging in testing.

That's not optimal but I think ok for the time being; but we should
try and add them later.


And as I like autopkgtests I've now implemented them, but in a
separate "autopkgtest" branch, which we can merge into master once
you have a working setup as well.
(And it's still not perfect, and the package is indeed not trivial to
autopkgtest :))
> > > 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.
> I added the needed snippet in d/fusioninventory-agent.postinst &
> d/fusioninventory-agent.postrm. I also added it in the new
> d/fusioninventory-agent.preinst. I hope I did it in the right way.

Almost :)
> > > dpkg-dev has some makefile snippets that can be included in
> > > debian/rules which provide variables that can be used:
> > > Maybe $(DEB_VENDOR) from /usr/share/dpkg/vendor.mk is enough?
> > Thank you, I'll check that, but I also think to see what I should better do
> > in Makefile.PL to also limit the sources changes.
> Here I decided to do all the requested work into Makefile.PL as this is not a debian only problem. So I added a patch I did upstream.
> Then the d/rules file became really lighter.
> This should also fix the package failing to build twice in a row.

Ack, this looks all good.
d/rules is readable again, and the package builds twice.

> I also added few line in d/rules to being able to set the version
> that will be reported by the script. I follow your advice and used
> snippet found in dpkg-dev examples.

Well, you didn't use it, you copied it from there :)
(See my notes in d/changelog)
> Waiting the next TODOs now ;-)

Hehe :)


 .''`.  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: Don McLean: If We Try_2

Attachment: signature.asc
Description: Digital Signature

Reply to: