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