[pkg-go] Call for review - containerd and dependencies

Fri Mar 2 03:38:44 UTC 2018

Hi,

I addressed all your comments, thanks very much for the feedback. More
details bellow.

On 02/28/2018 04:05 PM, Michael Stapelberg wrote:
>
>
>     #### containerd
>     <https://salsa.debian.org/elboulangero-guest/containerd
>     <https://salsa.debian.org/elboulangero-guest/containerd>>
>     - As for testing the thing really, I didn't go far yet, I just
>     launched
>       the binary, the log messages that appeared looked healthy enough,
>       and that's all for now.
>
>
> Some of the comments below apply (unnecessary-team-upload, DEP-3
> headers for patches).

Fixed.

> Also, this requires golang-github-containerd-console-dev to build,
> which is not in the archive and not in this list of packages?

golang-github-containerd-console is available at
<https://salsa.debian.org/go-team/packages/golang-github-containerd-console>

It was part of the very first packages I made, that's why it's in
`salsa.debian.org/go-team/packages`.

The last time I heard from it it Debian FTP Masters, it said:

  Your package has been put into the NEW queue, which requires manual action
  from the ftpteam to process. The upload was otherwise valid (it had a good
  OpenPGP signature and file hashes are valid), so please be patient.

>  
>
>
>     #### golang-github-docker-go-events
>     <https://salsa.debian.org/elboulangero-guest/golang-github-docker-go-events
>     <https://salsa.debian.org/elboulangero-guest/golang-github-docker-go-events>>
>
>
> Can you address these please?
> P: golang-github-docker-go-events source:
> file-contains-trailing-whitespace debian/control (line 25)
> W: golang-github-docker-go-events source: unnecessary-team-upload

Fixed.

> I: golang-github-docker-go-events source: vcs-field-uses-insecure-uri
> vcs-git
> git://anonscm.debian.org/pkg-go/packages/golang-github-docker-go-events.git
> <http://anonscm.debian.org/pkg-go/packages/golang-github-docker-go-events.git>

I don't have this warning, neither from lintian Stretch, neither from
lintian backported from Buster. What do I miss ?

Anyway, I compared with other packages and fixed it.

>  
>
>
>     #### golang-github-docker-go-metrics
>     <https://salsa.debian.org/elboulangero-guest/golang-github-docker-go-metrics
>     <https://salsa.debian.org/elboulangero-guest/golang-github-docker-go-metrics>>
>
>
> Can’t build this until golang-github-prometheus-client-golang is
> fixed. Some of the lintian warnings above seem to apply here, too, though.

Lintian warnings fixed.

>  
>
>
>     #### golang-github-prometheus-client-golang
>     <https://salsa.debian.org/elboulangero-guest/golang-github-prometheus-client-golang
>     <https://salsa.debian.org/elboulangero-guest/golang-github-prometheus-client-golang>>
>
>
> This package cannot be cloned with gbp as-is:
>
> % gbp clone
> https://salsa.debian.org/elboulangero-guest/golang-github-prometheus-client-golang
> gbp:info: Cloning from
> 'https://salsa.debian.org/elboulangero-guest/golang-github-prometheus-client-golang'
> gbp:error: Git command failed: Error running git checkout: error:
> pathspec 'master' did not match any file(s) known to git.
>
> I think you’ll need to set debian-branch in debian/gbp.conf.

OK, I just changed the default branch in the GitLab settings, and set it
to `debian/sid`, which is the branch I worked on, and which comes with a
proper gbp.conf. The default branch before was
`debian/jessie-backports`, and it doesn't have a gpb.conf.

So now gbp cone works.

This package has no `master` branch, only a `debian/jessie-backports`
and `debian/sid`. I think it's what DEP-14 recommends so I prefer not to
change anything here, even more because I'm bumping someone else package.

>
>     #### continuity
>     <https://salsa.debian.org/elboulangero-guest/continuity
>     <https://salsa.debian.org/elboulangero-guest/continuity>>
>     <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=890983
>     <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=890983>>
>     - I ran the series of test described at
>       <https://github.com/containerd/continuity
>     <https://github.com/containerd/continuity>>,
>       everything went fine.
>
>
> It would be good to add a manpage for the continuity binary.
> Otherwise, the package looks fine.

Agreed, well, I'm running out of time on that project, so I skip for
now, if it's ok with you.

>  
>
>
>     #### golang-gogottrpc
>     <https://salsa.debian.org/elboulangero-guest/golang-gogottrpc
>     <https://salsa.debian.org/elboulangero-guest/golang-gogottrpc>>
>     <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=890958
>     <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=890958>>
>     - Patch and issues submitted upstream.
>
>
> Can you add DEP-3 tags to the patches please?
> See http://dep.debian.net/deps/dep3/. No need to go overboard, just
> the last-updated/author/description/forwarded ones.

Fixed.

>
> Also, there seems to be a lintian false-positive: it reports
> statically-linked-binary despite the Build-Depends on golang-go. Can
> you file a lintian bug about that please?

Ok, I'm doing that right now.

BTW, in this package I use the following Build-Depends for `golang-go`:

               golang-go (>= 2:1.9~) | golang-1.9-go,

Using the epoch `2` in the version here. Is it actually needed ? I'm not
100% sure, but I think I saw other packages that don't mention epoch and
just do something like `>1.9`, which is more concise. I'm also not sure
about the tilde at then end, I just copy-pasted that from somewhere :)

>  
>
>
>     #### golang-github-containerd-btrfs
>     <https://salsa.debian.org/elboulangero-guest/golang-github-containerd-btrfs
>     <https://salsa.debian.org/elboulangero-guest/golang-github-containerd-btrfs>>
>     <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=890989
>     <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=890989>>
>     - I disabled the test to avoid installing a btrfs-test binary. I don't
>       think it makes sense to create an additional binary package
>       just for shipping this test.
>
>
> Sounds good. Maybe change cmd to cmd/btrfs-test so that we’ll notice
> newly introduced binaries upon updates? Otherwise, the package looks good.

Indeed, good catch ! Done.

>  
>
>
>     #### golang-github-containerd-typeurl
>     <https://salsa.debian.org/elboulangero-guest/golang-github-containerd-typeurl
>     <https://salsa.debian.org/elboulangero-guest/golang-github-containerd-typeurl>>
>     <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=891181
>     <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=891181>>
>     - Patch submitted upstream
>
>
> Please add DEP-3 tags to the patch. Looks good otherwise.
>  

Done.

>
>     #### golang-github-dmcgowan-go-tar
>     <https://salsa.debian.org/elboulangero-guest/golang-github-dmcgowan-go-tar
>     <https://salsa.debian.org/elboulangero-guest/golang-github-dmcgowan-go-tar>>
>     <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=890960
>     <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=890960>>
>     - This one was a tricky one, please read the commit messages for
>     details.
>
>
> Does the package which needs this require support for sparse tar
> files? If not, I think we can do away with this package in favor of
> patching the consumer to use archive/tar instead. Doing a diff -ur
> shows that this package is identical to archive/tar as of Go 1.10,
> except for the sparse support, which is scheduled to land in Go 1.11.

You're right, and there's no need for sparse support. I patched as you
suggested to use archive/tar.

Best regards,

  Arnaud

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.alioth.debian.org/pipermail/pkg-go-maintainers/attachments/20180302/dee051b3/attachment-0001.html>