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

Mon Mar 12 19:17:17 UTC 2018

On Fri, Mar 2, 2018 at 4:38 AM, Arnaud <arnaud.rebillout at collabora.com>
wrote:

> 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>
>> - 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>
> <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`
> <http://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.
>
>
Yeah, I see that mail, but I can’t see the package in
https://ftp-master.debian.org/new.822. Might be worth sending an email to
ftp-master about this discrepancy.

>
>
>>
>> #### 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
>
>
> I don't have this warning, neither from lintian Stretch, neither from
> lintian backported from Buster. What do I miss ?
>

Are you enabling all the options I have enabled? See
https://github.com/stapelberg/configfiles/blob/master/lintianrc

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

Thanks, the package indeed builds now.

Could you send an email to tincho@ about reviewing/merging these changes
please? I think your update of the upstream branch is different from how
tincho uses the upstream branch.

>
>
>
> #### continuity
>> <https://salsa.debian.org/elboulangero-guest/continuity>
>> <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=890983>
>> - I ran the series of test described at
>>   <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://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 :)
>

The epoch is required; leaving it out results in a trivially fulfilled
dependency:

% dpkg --compare-versions '2:1.10' '>=' '1.9'  && echo yes || echo no
yes
% dpkg --compare-versions '2:1.8' '>=' '1.9'  && echo yes || echo no
yes

>
>
>
>
>>
>> #### golang-github-containerd-btrfs
>> <https://salsa.debian.org/elboulangero-guest/golang-github-
>> containerd-btrfs>
>> <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://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://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
>
>

-- 
Best regards,
Michael
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.alioth.debian.org/pipermail/pkg-go-maintainers/attachments/20180312/c354d8e8/attachment-0001.html>