Re: Bug#869926: RFS: oprofile/1.2.0-1 [ITP]

[Andrey Rahmatullin]

On Sat, Oct 14, 2017 at 11:15:53PM -0300, Roberto Oliveira wrote:
> > > Thanks Roberto.
> > >
> > > wRar, do you still any concern about this package?
> > Nope, uploaded. Thank you Roberto.
> 
> Thanks for uploading the package!
> 
> I saw in ftp-master [1] that the package was built but it is only
> showing for amd64, however the package architecture is linux-any.
While the package is in NEW it's not built anywhere.

-- 
WBR, wRAR


signature.asc
Description: PGP signature

Bug#869926: RFS: oprofile/1.2.0-1 [ITP]

[Roberto Oliveira]

On Mon, 18 Sep 2017 23:59:27 +0500 Andrey Rahmatullin  wrote:
> On Mon, Sep 18, 2017 at 02:14:17PM -0300, Breno Leitao wrote:
> > Thanks Roberto.
> >
> > wRar, do you still any concern about this package?
> Nope, uploaded. Thank you Roberto.

Thanks for uploading the package!

I saw in ftp-master [1] that the package was built but it is only
showing for amd64, however the package architecture is linux-any.

Shouldn't it been build for other architectures, like ppc64le?  Is
something I missed in the package?


Regards,
Roberto Oliveira
--
[1] - https://ftp-master.debian.org/new/oprofile_1.2.0-1.html

Bug#869926: RFS: oprofile/1.2.0-1 [ITP]

[Breno Leitao]

On Sat, 9 Sep 2017 20:20:03 -0300 Roberto Oliveira
> > In that case please override this warning and write a comment describing
> > the reason.
> Fixed.
> 
> > libopagent1 should be Section: libs.
> Fixed.

Thanks Roberto.

wRar, do you still any concern about this package?

Bug#869926: RFS: oprofile/1.2.0-1 [ITP]

[Roberto Oliveira]

On Sat, 9 Sep 2017 00:00:30 +0500 Andrey Rahmatullin  wrote:
> On Fri, Sep 08, 2017 at 03:48:10PM -0300, Roberto Oliveira wrote:
> > > W: oprofile-jit: package-has-unnecessary-activation-of-ldconfig-trigger
> > oprofile-jit installs a file under /etc/ld.so.conf.d/ that allows
> > oprofile finds the jvmti_oprofile when profiling Jited codes.
> > So I think we need to activate the ldconfig trigger so ldconfig
> > reloads it cache.
> In that case please override this warning and write a comment describing
> the reason.
Fixed.

> libopagent1 should be Section: libs.
Fixed.

> --
> WBR, wRAR

Bug#869926: RFS: oprofile/1.2.0-1 [ITP]

[Andrey Rahmatullin]

On Fri, Sep 08, 2017 at 03:48:10PM -0300, Roberto Oliveira wrote:
> > W: oprofile-jit: package-has-unnecessary-activation-of-ldconfig-trigger
> oprofile-jit installs a file under /etc/ld.so.conf.d/ that allows
> oprofile finds the jvmti_oprofile when profiling Jited codes.
> So I think we need to activate the ldconfig trigger so ldconfig
> reloads it cache.
In that case please override this warning and write a comment describing
the reason.

libopagent1 should be Section: libs.

-- 
WBR, wRAR


signature.asc
Description: PGP signature

Bug#869926: RFS: oprofile/1.2.0-1 [ITP]

[Roberto Oliveira]

Hi Andrey,


On Thu, 7 Sep 2017 22:33:48 +0500 Andrey Rahmatullin  wrote:
> dpkg-gencontrol: warning: Depends field of package libopagent-dev: unknown 
> substitution variable ${shlibs:Depends}
Fixed.

> I: oprofile source: duplicate-short-description libopagent1 libopagent-dev
Fixed.

> W: oprofile-jit: package-has-unnecessary-activation-of-ldconfig-trigger
oprofile-jit installs a file under /etc/ld.so.conf.d/ that allows
oprofile finds the jvmti_oprofile when profiling Jited codes.
So I think we need to activate the ldconfig trigger so ldconfig
reloads it cache.

> W: libopagent-dev: wrong-section-according-to-package-name libopagent-dev => 
> libdevel
Fixed

>
> There is also
> E: oprofile-jit: package-modifies-ld.so-search-path 
> etc/ld.so.conf.d/oprofile-jit.conf
> but it seems there is no good solution to this.
>
>
> --
> WBR, wRAR

Bug#869926: RFS: oprofile/1.2.0-1 [ITP]

[Andrey Rahmatullin]

dpkg-gencontrol: warning: Depends field of package libopagent-dev: unknown 
substitution variable ${shlibs:Depends}

I: oprofile source: duplicate-short-description libopagent1 libopagent-dev
W: oprofile-jit: package-has-unnecessary-activation-of-ldconfig-trigger
W: libopagent-dev: wrong-section-according-to-package-name libopagent-dev => 
libdevel

There is also
E: oprofile-jit: package-modifies-ld.so-search-path 
etc/ld.so.conf.d/oprofile-jit.conf
but it seems there is no good solution to this.


-- 
WBR, wRAR


signature.asc
Description: PGP signature

Bug#869926: RFS: oprofile/1.2.0-1 [ITP]

[Roberto Oliveira]

Hi Andrey,


On Sat, 2 Sep 2017 22:44:24 +0500 Andrey Rahmatullin  wrote:
> libopagent.so should be in -dev.
Fixed.

> I think /etc/ld.so.conf.d/oprofile-jit.conf should be in oprofile-jit.
Fixed.

> I think you had a .symbols file for libopagent1, you should use it again.
Fixed. I'm using it again.

> You should use debian/tmp instead of . in find commands in d/rules.
Fixed.

> Your .html files are size 0, because you haven't adjusted B-D.
Fixed the B-D. The .html files are now ok.

>
> --
> WBR, wRAR

Bug#869926: RFS: oprofile/1.2.0-1 [ITP]

[Andrey Rahmatullin]

libopagent.so should be in -dev.
I think /etc/ld.so.conf.d/oprofile-jit.conf should be in oprofile-jit.
I think you had a .symbols file for libopagent1, you should use it again.
You should use debian/tmp instead of . in find commands in d/rules.
Your .html files are size 0, because you haven't adjusted B-D.

-- 
WBR, wRAR


signature.asc
Description: PGP signature

Bug#869926: RFS: oprofile/1.2.0-1 [ITP]

[Roberto Oliveira]

Hi Andrey,

I found something wrong in the last version I sent, so I just sent a
new version.

So in case you already did a "dget -x", please, do it again.


Regards,
Roberto Oliveira.

Bug#869926: RFS: oprofile/1.2.0-1 [ITP]

[Roberto Oliveira]

Hi Andrey,


On Thu, 31 Aug 2017 22:52:12 +0500 Andrey Rahmatullin  wrote:
> Please upgrade to the current Standards-Version.
Fixed.

> Why do you use "rm `find . -name "*.la"`" instead of "find . -name "*.la" 
> -delete"?
Fixed

> Why do you ship static versions of the plugins?
Fixed. Don't need them, just need the .so files

> README_PACKAGERS says "It is recommended to run ldconfig in %post and
> %postun to add/remove the /oprofile path from the standard library
> search paths.", have you checked the plugins work without adding that to
> the search paths?
Fixed. I created a file in /etc/ld.so.conf.d/ so operf with JITed code
works doing
something like:
$ operf java -Xcomp -agentlib:jvmti_oprofile Hello

I also did the changes in packages, as we talked in IRC, providing 4 packages:
oprofile, oprofile-jit, libopagent1 and libopagent-dev

> include/sstream should be documented in debian/copyright.
Fixed

> doc/*.html should ideally be rebuilt during the build process, you can
> probably achieve that by adding them to debian/clean and checking that
> they are built properly.
Fixed. I added debian/clean file

>
> --
> WBR, wRAR

Bug#869926: RFS: oprofile/1.2.0-1 [ITP]

[Andrey Rahmatullin]

Please upgrade to the current Standards-Version.
Why do you use "rm `find . -name "*.la"`" instead of "find . -name "*.la" 
-delete"?
Why do you ship static versions of the plugins?
README_PACKAGERS says "It is recommended to run ldconfig in %post and
%postun to add/remove the /oprofile path from the standard library
search paths.", have you checked the plugins work without adding that to
the search paths?
include/sstream should be documented in debian/copyright.
doc/*.html should ideally be rebuilt during the build process, you can
probably achieve that by adding them to debian/clean and checking that
they are built properly.

-- 
WBR, wRAR


signature.asc
Description: PGP signature

Bug#869926: RFS: oprofile/1.2.0-1 [ITP]

[Roberto Oliveira]

Hi Andrey,

On Tue, 29 Aug 2017 23:52:48 +0500 Andrey Rahmatullin  wrote:
> On Thu, Aug 24, 2017 at 04:45:56PM -0300, Roberto Oliveira wrote:
> > > I still see a lot of old junk in debian/control:
> > > - did you write that Build-Depends? if no, please write them from scratch,
> > >   making sure they are complete and necessary.
> > I Checked all the dependencies and now just the required dependencies are
> > in control file.
> dh-autoreconf is not needed and lintian tells that.
Fixed.

> libtool and g++ are already pulled by build-essential and debhelper.
Fixed.

> g++ (>>3.3.1) is always true for many years.
> Are you sure zlib1g-dev is needed? The packages don't depend on zlib.
I thought it was but I did a second check and seems it isn't needed. Fixed.

> > > Is libjvmti_oprofile intended to be a public shared library? It looks like
> > > a plugin.
> > Yes, it is a plugin, I moved it to the correct location.
> If both libs are not public shared libs you shouldn't package them as if
> they are. This includes .symbols, development files, the package names etc.
Fixed. As both are Oprofile plugins and don't bring extra deps, I'm
shipping them
in Oprofile package.

> Why do you create the user with --home /var/lib/oprofile --no-create-home?
> Does it need the home dir or no? What is /var/lib/oprofile?
Oprofile needs the 'oprofile' user and group to enable profiling JITed
code but it
doesn't need the home directory.

I removed the --home option but Lintian complained about:
maintainer-script-should-not-use-adduser-system-without-home

and suggested:
--no-create-home --home /nonexistent

I applied Litian's suggestion but if it is not correct, I can change.

> --with autoreconf is the default with debhelper 10.
Fixed.

> Why does oprofile depend on libopagent1?
Fixed. I'm now shipping libopagent1 in Oprofile as it is a plugin.


> --
> WBR, wRAR

Bug#869926: RFS: oprofile/1.2.0-1 [ITP]

[Andrey Rahmatullin]

On Thu, Aug 24, 2017 at 04:45:56PM -0300, Roberto Oliveira wrote:
> > I still see a lot of old junk in debian/control:
> > - did you write that Build-Depends? if no, please write them from scratch,
> >   making sure they are complete and necessary.
> I Checked all the dependencies and now just the required dependencies are
> in control file.
dh-autoreconf is not needed and lintian tells that.
libtool and g++ are already pulled by build-essential and debhelper.
g++ (>>3.3.1) is always true for many years.
Are you sure zlib1g-dev is needed? The packages don't depend on zlib.

> > Is libjvmti_oprofile intended to be a public shared library? It looks like
> > a plugin.
> Yes, it is a plugin, I moved it to the correct location.
If both libs are not public shared libs you shouldn't package them as if
they are. This includes .symbols, development files, the package names etc.

Why do you create the user with --home /var/lib/oprofile --no-create-home?
Does it need the home dir or no? What is /var/lib/oprofile?
--with autoreconf is the default with debhelper 10.
Why does oprofile depend on libopagent1?

-- 
WBR, wRAR


signature.asc
Description: PGP signature

Bug#869926: RFS: oprofile/1.2.0-1 [ITP]

[Roberto Oliveira]

Hi Andrey,

I think I did all the changes you suggested, can you take a look on it
again, please?


> Control: tags -1 + moreinfo
>
> I still see a lot of old junk in debian/control:
> - did you write that Build-Depends? if no, please write them from scratch,
>   making sure they are complete and necessary.
I Checked all the dependencies and now just the required dependencies are
in control file.

> - why Conflicts: oprofile-modules*? There are no such packages in last
>   releases.
Fixed.

> - Replaces: oprofile-common? Suggests: oprofile-gui? Replaces: oprofile
>   (<< 0.9.6-1ubuntu1)? The same.
Fixed.

> - oprofile gets a Depends: binutils via libbfd-2.29-system.so, it
>   shouldn't also Recommend it.
Fixed.

> You don't need .shlibs when you use .symbols.
Fixed.

> Why does the oprofile user need /bin/bash as a shell?
It doesn't need it, fixed.

> Why do you need the debconf snippet in postrm?
Actually don't need this step in purge as it doesn't store any configuration and
it doesn't use the debconf. As postrm was just doing that, I removed it.

> What is /usr/doc/oprofile which is removed in prerm?
It seems a deprecated file and oprofile doesn't install it. As prerm was just
doing that, I removed it.

> Please don't mix debhelper files with and without package prefixes (this
> is about maintainer script files).
> Instead of editing .la you should delete it.
Fixed.

> Instead of manually installing to debian/oprofile and manually moving to
> other package dirs you should use dh_install to move from debian/tmp to
> package dirs.
Fixed.

> Please upgrade to the current Standards-Version.
Fixed.

> Please switch to the debhelper compat level 10.
Fixed.

> Is libjvmti_oprofile intended to be a public shared library? It looks like
> a plugin.
Yes, it is a plugin, I moved it to the correct location.


> --
> WBR, wRAR

Bug#869926: RFS: oprofile/1.2.0-1 [ITP]

[Andrey Rahmatullin]

Control: tags -1 + moreinfo

I still see a lot of old junk in debian/control:
- did you write that Build-Depends? if no, please write them from scratch,
  making sure they are complete and necessary.
- why Conflicts: oprofile-modules*? There are no such packages in last
  releases.
- Replaces: oprofile-common? Suggests: oprofile-gui? Replaces: oprofile
  (<< 0.9.6-1ubuntu1)? The same.
- oprofile gets a Depends: binutils via libbfd-2.29-system.so, it
  shouldn't also Recommend it.

You don't need .shlibs when you use .symbols.
Why does the oprofile user need /bin/bash as a shell?
Why do you need the debconf snippet in postrm?
What is /usr/doc/oprofile which is removed in prerm?
Please don't mix debhelper files with and without package prefixes (this
is about maintainer script files).
Instead of editing .la you should delete it.
Instead of manually installing to debian/oprofile and manually moving to
other package dirs you should use dh_install to move from debian/tmp to
package dirs.
Please upgrade to the current Standards-Version.
Please switch to the debhelper compat level 10.
Why are the dev files for the libs shipped in the main subpackage?
Is libjvmti_oprofile intended to be a public shared library? It looks like
a plugin.

-- 
WBR, wRAR


signature.asc
Description: PGP signature

Bug#869926: RFS: oprofile/1.2.0-1 [ITP]

[Roberto Oliveira]

Thanks for reviewing it Andrey!

I tried to apply your suggestions in the package and also made some other
improvements. Hope it looks better now.

Bug#869926: RFS: oprofile/1.2.0-1 [ITP]

[Andrey Rahmatullin]

Why is this not using dh(1)?
It's also hardcoding /usr/lib and not the multiarch path in d/rules.
Why is changelog-move-to-git-log.patch needed?

There are a lot of strange and ancient things kept from the previous
Debian package, yet d/changelog doesn't have the old entries. OTOH
d/copyright doesn't list even the current maintainer or the current year.


-- 
WBR, wRAR


signature.asc
Description: PGP signature

Bug#869926: RFS: oprofile/1.2.0-1 [ITP]

[Roberto Oliveira]

Package: sponsorship-requests
Severity: wishlist

Dear mentors,

I am looking for a sponsor for my package "oprofile"

Package name: oprofile
Version : 1.2.0-1
URL : http://oprofile.sourceforge.net/
License : GPL2
Section : devel

It builds those binary packages:
libjvmti-oprofile0 - system-wide profiler for Linux systems (Java
runtime library)
libopagent1 - system-wide profiler for Linux systems (opagent runtime library)
oprofile   - system-wide profiler for Linux systems

To access further information about this package, please visit the
following URL:
https://mentors.debian.net/package/oprofile


Alternatively, one can download the package with dget using this command:

dget -x 
https://mentors.debian.net/debian/pool/main/o/oprofile/oprofile_1.2.0-1.dsc


Regards,
Roberto Oliveira