Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.debian.bugs.dist > #952296 > unrolled thread

Bug#925911: RFS: lopsub-1.0.2 [ITP]

Back to article view | Back to linux.debian.bugs.dist

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Bug#925911: RFS: lopsub-1.0.2 [ITP] Adam Borowski <kilobyte@angband.pl> - 2019-03-31 00:10 +0100
    Bug#925911: RFS: lopsub-1.0.2 [ITP] Andre Noll <maan@tuebingen.mpg.de> - 2019-04-01 01:40 +0200
      Bug#925911: RFS: lopsub-1.0.2 [ITP] Adam Borowski <kilobyte@angband.pl> - 2019-04-01 12:40 +0200
      Bug#925911: RFS: lopsub-1.0.2 [ITP] Andre Noll <maan@tuebingen.mpg.de> - 2019-04-21 17:30 +0200
        Bug#925911: RFS: lopsub-1.0.2 [ITP] Adam Borowski <kilobyte@angband.pl> - 2019-04-21 22:30 +0200
          Bug#925911: RFS: lopsub-1.0.2 [ITP] Andre Noll <maan@tuebingen.mpg.de> - 2019-04-22 17:40 +0200
            Bug#925911: RFS: lopsub-1.0.2 [ITP] Andre Noll <maan@tuebingen.mpg.de> - 2019-04-29 15:10 +0200

#952296 — Bug#925911: RFS: lopsub-1.0.2 [ITP]

On Thu, Mar 28, 2019 at 11:41:45AM +0100, Andre Noll wrote:
>  * Package name    : lopsub
>    Version         : 1.0.2

Such a version means a native package; only software written specifically
for Debian which makes no sense outside it should use the native format.
Even if you're both upstream and the packager, a non-native format is
helpful in case someone other than you would upload a fix.

This library is certainly not something for Debian only, thus please append
"-1" to the version, and set debian/source/format to "3.0 (quilt)" (which,
despite the name, doesn't require actually using quilt).

>    Upstream Author : Andre Noll <maan@tuebingen.mpg.de>
>  * URL             : http://people.tuebingen.mpg.de/maan/lopsub/
>  * License         : (L)GPLv3

It would be nice to mention more prominently what parts are GPLv3-ed and
which are LGPLv3-ed.

>    Section         : libdevel
> 
> lopsup - The Long Option Parser for Subcommands

> However, there are still some warnings from lintian
> I don't know how to deal with.

I'd recommend running "lintian -i" which gives a long descriptive message
for every warning, including hints how to fix.

I see a static library installed by the package.  Those shouldn't generally
be used unless you're doing something special -- static linking makes
security and bugfix updates extremely tedious.  Libraries should also be
split into separate binary packages, with lib${name}dev providing
development files while lib${name}${so} the runtime lib.

I'll stop this round of review here; I haven't took a look at the actual
functionality yet.


Meow!
-- 
⢀⣴⠾⠻⢶⣦⠀
⣾⠁⢠⠒⠀⣿⡁ Did ya know that typing "test -j8" instead of "ctest -j8"
⢿⡄⠘⠷⠚⠋⠀ will make your testsuite pass much faster, and fix bugs?
⠈⠳⣄⠀⠀⠀⠀

[toc] | [next] | [standalone]

#952478

[Multipart message — attachments visible in raw view] — view raw

On Sat, Mar 30, 23:58, Adam Borowski wrote
> On Thu, Mar 28, 2019 at 11:41:45AM +0100, Andre Noll wrote:
> >  * Package name    : lopsub
> >    Version         : 1.0.2
> 
> Such a version means a native package; only software written specifically
> for Debian which makes no sense outside it should use the native format.
> Even if you're both upstream and the packager, a non-native format is
> helpful in case someone other than you would upload a fix.
> 
> This library is certainly not something for Debian only, thus please append
> "-1" to the version, and set debian/source/format to "3.0 (quilt)" (which,
> despite the name, doesn't require actually using quilt).

Sure, I just wasn't aware of this convention, and that "3.0 (quilt)"
is the right choice for a package like lopsub. One question: With this
change applied, dpkg-buildpackage wants the upstream tarball in the
parent directory. I guess I'm supposed to run git archive here to
create it as part of the before-build hook, but I couldn't figure
out where this hook is defined.

> >    Upstream Author : Andre Noll <maan@tuebingen.mpg.de>
> >  * URL             : http://people.tuebingen.mpg.de/maan/lopsub/
> >  * License         : (L)GPLv3
> 
> It would be nice to mention more prominently what parts are GPLv3-ed and
> which are LGPLv3-ed.

The library is LGPLv3 while the lopsubgen executable is GPLv3. The
source code generated by lopsubgen is licensed with a simple
all-permissive license. See lopsub.7 for more information. I've
changed debian/copyright to make this distinction stand out a bit more.

> I'd recommend running "lintian -i" which gives a long descriptive message
> for every warning, including hints how to fix.

Point. This was a very helpful hint indeed. With the long descriptions,
it was easy to squash the remaining few warnings.

> I see a static library installed by the package.  Those shouldn't generally
> be used unless you're doing something special -- static linking makes
> security and bugfix updates extremely tedious.  Libraries should also be
> split into separate binary packages, with lib${name}dev providing
> development files while lib${name}${so} the runtime lib.

Yes, I had this concern as well. I'll change the build system to
create a shared library instead and split the binary package as you
suggest. I'll follow up with another reply when it's ready.

Thanks a bunch for the review
Andre
-- 
Max Planck Institute for Developmental Biology
Max-Planck-Ring 5, 72076 Tübingen, Germany. Phone: (+49) 7071 601 829
http://people.tuebingen.mpg.de/maan/

[toc] | [prev] | [next] | [standalone]

#952523

On Mon, Apr 01, 2019 at 12:57:51AM +0200, Andre Noll wrote:
> On Sat, Mar 30, 23:58, Adam Borowski wrote
> > On Thu, Mar 28, 2019 at 11:41:45AM +0100, Andre Noll wrote:
> > >  * Package name    : lopsub
> > >    Version         : 1.0.2

> > This library is certainly not something for Debian only, thus please append
> > "-1" to the version, and set debian/source/format to "3.0 (quilt)" (which,
> > despite the name, doesn't require actually using quilt).
> 
> Sure, I just wasn't aware of this convention, and that "3.0 (quilt)"
> is the right choice for a package like lopsub. One question: With this
> change applied, dpkg-buildpackage wants the upstream tarball in the
> parent directory. I guess I'm supposed to run git archive here to
> create it as part of the before-build hook, but I couldn't figure
> out where this hook is defined.

There are many workflows for this, but my personal favourite is to skip any
such wrappers and rely on uscan (ie, debian/watch) only.  As most project
will want a watch file anyway, this reduces the amount of work.

Alas, the vast majority of projects use a git host that does provide tarballs
for a tag, thus I have never used uscan with mode=git (as downloading a
tarball is still more likely to produce a reproducible result), and thus
can't give you ready copypasta to use.

> > I see a static library installed by the package.  Those shouldn't generally
> > be used unless you're doing something special -- static linking makes
> > security and bugfix updates extremely tedious.  Libraries should also be
> > split into separate binary packages, with lib${name}dev providing
> > development files while lib${name}${so} the runtime lib.
> 
> Yes, I had this concern as well. I'll change the build system to
> create a shared library instead and split the binary package as you
> suggest. I'll follow up with another reply when it's ready.

Sounds good.


Meow!
-- 
⢀⣴⠾⠻⢶⣦⠀
⣾⠁⢠⠒⠀⣿⡁ Did ya know that typing "test -j8" instead of "ctest -j8"
⢿⡄⠘⠷⠚⠋⠀ will make your testsuite pass much faster, and fix bugs?
⠈⠳⣄⠀⠀⠀⠀

[toc] | [prev] | [next] | [standalone]

#955494

[Multipart message — attachments visible in raw view] — view raw

On Sun, Apr 14, 15:41, Adam Borowski wrote

> On Sat, Apr 06, 2019 at 09:52:53PM +0200, Andre Noll wrote:
> > > > I see a static library installed by the package.  Those shouldn't generally
> > > > be used unless you're doing something special -- static linking makes
> > > > security and bugfix updates extremely tedious.  Libraries should also be
> > > > split into separate binary packages, with lib${name}dev providing
> > > > development files while lib${name}${so} the runtime lib.
> 
> I'm afraid that the shared library doesn't work: liblopsub1 in /usr/lib
> has nothing but an infinite symlink loop from liblopsub.so.unnamed_version
> back to itself.

It worked when compiling from git, but the generated source tarball
was broken because the version.gen.sh script relied on git. This has
been fixed now.

> Besides a proper soname, the file should also go to a multiarch dir (ie,
> /usr/lib/x86_64-linux-gnu on amd64, /usr/lib/aarch64-linux-gnu on arm64,
> etc -- most build systems get it right without your action).

Done.

> Also, I wonder why you include the soname in the -dev package's name. 
> That's allowed but usually not what you want -- an ABI break will also
> require changing every depending package instead of a simple recompile.
> Unless you change the API every time there's a soname bump, it'd be
> better to have the -dev package be unversioned.  But this is up to you.

That's just because I misread section 8.1 of the Debian Policy Manual.
I've renamed the -dev package to liblopsub-dev.

Please find the updated repository at the usual location. As before,
you have to check out the "pu" branch:

        git clone git://git.tuebingen.mpg.de/lopsub.git
        cd lopsub
        git checkout origin/pu
        git archive --prefix liblopsub1-1.0.2/ @ | xz > ../liblopsub1_1.0.2.orig.tar.xz
        dpkg-buildpackage

If there are further issues, just let me know.

Thanks
Andre
-- 
Max Planck Institute for Developmental Biology
Max-Planck-Ring 5, 72076 Tübingen, Germany. Phone: (+49) 7071 601 829
http://people.tuebingen.mpg.de/maan/

[toc] | [prev] | [next] | [standalone]

#955522

On Sun, Apr 21, 2019 at 05:11:05PM +0200, Andre Noll wrote:
> That's just because I misread section 8.1 of the Debian Policy Manual.
> I've renamed the -dev package to liblopsub-dev.

Not sure if you'd want the _source_ package to have a simple soname-less
name as well; I would but that's up to you -- that'd be nicer and make
having only-one-version transitions easier; on the other hand a
soname-encoded source name is better when there's a need for multiple
coinstallable versions.

Your choice; current state is ok.

> If there are further issues, just let me know.

Just cosmetic stuff:

* installation instructions don't really belong in the man page -- if you
  can read it, you've already managed to install the package.

* please copy the description for liblopsub1 to liblopsub-dev; it currently
  says just "This package contains the development environment for the
  lopsub library."  It's pointless to require the user to check the other
  package -- other libraries alter merely the last part.  Also, it's -dev
  what users pull by hand.

* is there a reason for shipping the static library?  Static linking is
  frowned upon in a distribution -- whenever the library gets updated,
  every reverse dependency has to be recompiled; this is especially nasty
  for security updates.

* (bonus) The nicely documented process for building the example looks
  like something that could be turned into an autopkgtest.  Unlike
  build-time tests, autopkgtests are run against installed packages,
  the way an user would.  That's of course extra effort, by no means
  required -- but, extra testing is always good.

But in general, the package already seems to be in a releasable state. 
Could you please change "UNRELEASED" to "unstable" so it can be uploaded?


Meow!
-- 
⢀⣴⠾⠻⢶⣦⠀
⣾⠁⢠⠒⠀⣿⡁ Did ya know that typing "test -j8" instead of "ctest -j8"
⢿⡄⠘⠷⠚⠋⠀ will make your testsuite pass much faster, and fix bugs?
⠈⠳⣄⠀⠀⠀⠀

[toc] | [prev] | [next] | [standalone]

#955598

[Multipart message — attachments visible in raw view] — view raw

On Sun, Apr 21, 22:25, Adam Borowski wrote
> On Sun, Apr 21, 2019 at 05:11:05PM +0200, Andre Noll wrote:
> > That's just because I misread section 8.1 of the Debian Policy Manual.
> > I've renamed the -dev package to liblopsub-dev.
> 
> Not sure if you'd want the _source_ package to have a simple soname-less
> name as well; I would but that's up to you -- that'd be nicer and make
> having only-one-version transitions easier; on the other hand a
> soname-encoded source name is better when there's a need for multiple
> coinstallable versions.

There are no plans for multiple incompatible versions. Instead, the
plan is to keep the ABI compatible forever, using symbol versioning
if necessary. Therefore I've changed the name of the source package
from liblopsub1 to liblopsub.

> * installation instructions don't really belong in the man page -- if you
>   can read it, you've already managed to install the package.

Right. OTOH, the instructions might be helpful for the web page, which
is just the html version of lopsub.7. I've moved the instructions to
the README file, and adjusted lopsub.7 slightly.

> * please copy the description for liblopsub1 to liblopsub-dev; it currently
>   says just "This package contains the development environment for the
>   lopsub library."  It's pointless to require the user to check the other
>   package -- other libraries alter merely the last part.  Also, it's -dev
>   what users pull by hand.

Done.

> * is there a reason for shipping the static library?  Static linking is
>   frowned upon in a distribution -- whenever the library gets updated,
>   every reverse dependency has to be recompiled; this is especially nasty
>   for security updates.

The main reason I've kept it is that the Debian Policy Manual says

	The static library (libraryname.a) is usually provided in addition
	to the shared version.

But yes, nobody needs the static library, so I've removed it from
the build.

> * (bonus) The nicely documented process for building the example looks
>   like something that could be turned into an autopkgtest.  Unlike
>   build-time tests, autopkgtests are run against installed packages,
>   the way an user would.  That's of course extra effort, by no means
>   required -- but, extra testing is always good.

I guess I need some help here.

	https://people.debian.org/~mpitt/autopkgtest/README.package-tests.html

describes the format of debian/tests/control, but does not say
how to set up the test environment. Since the tests do not need to
run as root, and do not access the network either, a simple chroot
environment should be fine. But how do I set up such an environment
for use with autopkgtest?

> But in general, the package already seems to be in a releasable state. 
> Could you please change "UNRELEASED" to "unstable" so it can be uploaded?

Done. Please have a final look. If everything is fine, I can merge
the various topic branches to master (so that master becomes what is
pu now), and tag the result as v1.0.3.

Thanks
Andre
-- 
Max Planck Institute for Developmental Biology
Max-Planck-Ring 5, 72076 Tübingen, Germany. Phone: (+49) 7071 601 829
http://people.tuebingen.mpg.de/maan/

[toc] | [prev] | [next] | [standalone]

#956498

[Multipart message — attachments visible in raw view] — view raw

On Mon, Apr 22, 17:30, Andre Noll wrote
> > But in general, the package already seems to be in a releasable state. 
> > Could you please change "UNRELEASED" to "unstable" so it can be uploaded?
> 
> Done. Please have a final look. If everything is fine, I can merge
> the various topic branches to master (so that master becomes what is
> pu now), and tag the result as v1.0.3.

Ping
Andre
-- 
Max Planck Institute for Developmental Biology
Max-Planck-Ring 5, 72076 Tübingen, Germany. Phone: (+49) 7071 601 829
http://people.tuebingen.mpg.de/maan/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.debian.bugs.dist

csiph-web