<div dir="ltr"><div>Once you feel your packages are ready for review (possibly this reply prompts some changes?), please send an up-to-date list of git URLs to what you’d like reviewed. Thanks!</div><div><br></div><div>Replies inline:</div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Feb 23, 2018 at 11:40 AM, Arnaud <span dir="ltr"><<a href="mailto:arnaud.rebillout@collabora.com" target="_blank">arnaud.rebillout@collabora.<wbr>com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF"><span>
<br>
<br>
<div class="gmail-m_8831067010898082083m_-5598104649873758548moz-cite-prefix">On 02/23/2018 05:10 PM, Michael
Stapelberg wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">I can’t have a look at the individual packages
right now</div>
</blockquote>
<br></span>
No rush !<span><br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
#### containerd<br>
<<a href="https://salsa.debian.org/elboulangero-guest/containerd" rel="noreferrer" target="_blank">https://salsa.debian.org/elbo<wbr>ulangero-guest/containerd</a>><br>
- There's a lintian warning about
package-contains-vcs-control-f<wbr>ile, but<br>
until<br>
now I didn't find the right way to remove a file from
installation. I<br>
guess<br>
`override_dh_auto_install` is the way to go. Any hint
welcome here.<br>
</blockquote>
<div><br>
</div>
<div>Which file specifically is affected? The Files-Excluded
directive in debian/copyright is a good way to exclude
files.</div>
</div>
</div>
</div>
</blockquote>
<br></span>
There's a `docs/.gitignore`, it's in a subdirectory. Files-Excluded
will remove it from the orig tarball, right ? Is it really suitable
here ?<br></div></blockquote><div><br></div><div>Correct, and up to you (see the caveat which tincho expressed). Bug #812721 discusses a gbp import-ref command which would honor Files-Excluded, but we’re not quite there yet. So, if you want to stick to orig tarballs for the time being, Files-Excluded will work.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF">
<br>
For the moment the only package in which I exclude a file from
installation is done this way:<br>
<br>
override_dh_auto_install:<br>
dh_auto_install<br>
find $(CURDIR)/debian -name '.tool' -type d -prune -exec rm -r
'{}' +<br></div></blockquote><div><br></div><div>nit: use find’s -delete action?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF">
<br>
Not really elegant, and it took me two hours to come up with that,
after trying and failing every other way possible...<span><br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div> <br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
#### golang-github-containerd-btrfs<br>
<<a href="https://salsa.debian.org/elboulangero-guest/golang-github-containerd-btrfs" rel="noreferrer" target="_blank">https://salsa.debian.org/elbo<wbr>ulangero-guest/golang-github-c<wbr>ontainerd-btrfs</a>><br>
<<a href="https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=890989" rel="noreferrer" target="_blank">https://bugs.debian.org/cgi-b<wbr>in/bugreport.cgi?bug=890989</a>><br>
- I disabled the test to avoid installing a btrfs-test
binary, plus I think the<br>
test failed for some reason, I'm not sure to remember
though...<br>
</blockquote>
<div><br>
</div>
<div>To avoid installing a binary, can you use
the DH_GOLANG_EXCLUDES option please? See <a href="https://manpages.debian.org/testing/dh-golang/Debian::Debhelper::Buildsystem::golang.3pm.en.html" target="_blank">https://manpages.debian.or<wbr>g/testing/dh-golang/Debian::De<wbr>bhelper::Buildsystem::golang.<wbr>3pm.en.html</a></div>
<div><br>
</div>
<div>Also, please add a comment stating why precisely the
tests are disabled, and what needs to change so that we
can re-enable them.</div>
</div>
</div>
</div>
</blockquote>
<br></span>
Ok I'll try harder :)<br>
<br>
I noticed that DH_GOLANG_EXCLUDES will not save me from a test which
fails though. For example, when I was working with
containerd/cgroups, the test commmand invoked by dh looks like that:<br>
<br>
go test -vet=off -v -p 8 <a href="http://github.com/containerd/cgroups" target="_blank">github.com/containerd/cgroups</a><br>
<br>
DH_GOLANG_EXCLUDES is matched against <a href="http://github.com/containerd/cgroups" target="_blank">github.com/containerd/cgroups</a>,
so if it contains a file inside the cgroups directory, it's not
excluded from the files being tested.<br></div></blockquote><div><br></div><div>DH_GOLANG_EXCLUDES, like the other directives, operates on Go packages, not individual source code files. All files within <a href="http://github.com/containerd/cgroups">github.com/containerd/cgroups</a> belong to the cgroups package, so you can only exclude it in its entirety, which is not desired in this case.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF">
<br>
It means that if I want to prevent a file from the test, I still
need to patch it with `// +build ignore`, if I understand properly.</div></blockquote><div><br></div><div>Yes, if the whole file should be ignored. In case only individual tests are affected, prefer using t.Skipf please: <a href="https://golang.org/pkg/testing/#T.Skipf">https://golang.org/pkg/testing/#T.Skipf</a></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF"><span><br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
I see that most binaries come with the lintian warning<br>
`statically-linked-binary`, I guess it's just the way it
works in the<br>
go world. Should I just add a lintian override ?<br>
</blockquote>
<div><br>
</div>
<div>lintian does not emit the statically-linked-binary tag
for Go packages. Can you point me to a specific example
where you see it please?</div>
</div>
</div>
</div>
</blockquote>
<br></span>
Yep, easy.<br>
<br>
apt-get download gogoprotobuf<br>
lintian gogoprotobuf_*_amd64.deb<br></div></blockquote><div><br></div><div>Ah, I see what’s wrong here: you are supposed to run lintian on a .changes file, so that it can inspect both the binary packages (*.deb) and the source package (*.dsc). Indeed, when building gogoprotobuf and running “lintian golang-gogoprotobuf_0.5-1_amd64.changes”, I don’t get any warnings about statically linked binaries.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF">
<br>
Is this warning related to the field `Built-Using:
${misc:Built-Using}` for binary packages ? After reading the Debian
Policy Manual, I seem to understand that this field is needed for go
binary packages, am I correct ?<br></div></blockquote><div><br></div><div>I don’t see a connection between the two fields. lintian examines the Build-Depends field for detecting Go packages. Specifically, a package is a Go package if golang-go or golang-any are present in Build-Depends.</div><div><br></div><div>Aside from that, yes, Built-Using must be set when building Go binary packages, and must be unset for Go library packages. I have recently added lintian warnings which cover these cases, but they’re not yet in a released lintian version.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF">
<br>
However even with this field, there is still the warning in the
`containerd` binary package that I built here, but only for *some*
of the binaries in the package, not all of them.<span><br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
In the `-dev` packages, is `${shlibs:Depends}` needed ?
During the builds<br>
I see `unknown substitution variable ${shlibs:Depends}`
passing by.<br>
</blockquote>
<div><br>
</div>
<div>The messages implies it’s not required. I’m not
entirely sure, but would suggest removing it for now.</div>
</div>
</div>
</div>
</blockquote>
<br></span>
Ok. I think it should be also changed in dh-make-golang, should I
look there and issue a PR, or do you prefer not to touch it for now
?</div></blockquote><div><br></div><div>Please feel free to send a PR! :)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF"><span><br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
In the `-dev` packages, do we really need to copy-paste
all the<br>
dependencies from the source package ?<br>
</blockquote>
<div><br>
</div>
<div>Yes. They differ in some cases, for example when code
generation needs more dependencies than the actual
compilation/test (in which case the extra dependencies
show up only in Build-Depends, not Depends).</div>
</div>
</div>
</div>
</blockquote>
<br></span>
Ok...<br>
<br>
Thanks !<span class="gmail-m_8831067010898082083HOEnZb"><font color="#888888"><br>
<br>
Arnaud<br>
</font></span></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail-m_8831067010898082083gmail_signature">Best regards,<br>Michael</div>
</div></div>