On Thu, 28 Jun 2018 16:53:18 +0200, Guillaume Bougard wrote:
> Hello Gregor,
Salut!
> 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.
Thanks!
> > > > 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 :)
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: Don McLean: If We Try_2
Attachment:
signature.asc
Description: Digital Signature