[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#860804: RFS: highwayhash/0~20170419-g1f4a24f-1 [ITP] -- tensorflow dependency library



Hi Adam,

> It does look uploadable, yeah, even though there's a bunch of issues.  It's
> up to you whether you want to get it good first or to upload present state
> then improve it incrementally.  Please say what you prefer.

I updated the package again with the following changes:

 * changed Multi-Arch of the lib package to same
 * detect architecture in rules to privide HH_AARCH64 and HH_POWER
flag to upstream makefile,
    which is possible to fix the arm64 and ppc64el builds. (oops, I
have no such machines to test)
 * added manpage highwayhash.3 (modified based on your snippet)
 * added autopkgtest script (using modified version of your sanity test code)

http://debomatic-amd64.debian.net/distribution#experimental/highwayhash/0~20170419-g1f4a24f-1/buildlog
https://mentors.debian.net/debian/pool/main/h/highwayhash/highwayhash_0~20170419-g1f4a24f-1.dsc

I think this package can be uploaded now. Further fix will be added
incrementally.

> The bad news is, it succeeded only on amd64.
>
> On other architectures, the closest it came on x32 (a non-release arch) --
> builds ok, fails only at dpkg-gensymbols due to symbol mismatches.
> One warning: left shift count >= width of type [-Wshift-count-overflow]
> sounds like it might be broken, though[1].
>
> On arm64 it fails with:
> g++: error: unrecognized command line option '-mavx2'
>
> On armhf there's both the shift width issue, and:
> error: #error "Port"
>
> On i386, all of the above, plus:
> error: '_mm_cvtsi64_si128' was not declared in this scope

The HH_POWER and HH_AARCH64 flags are possible to fix the ppc64{,el} build and
arm64 build. However, I'm not clear on upstream's attitude towards
32-bit architectures.

Actually I think 32-bit architectures are not my priority too, since
upstreams of my
deep learning packages don't put much attention on 32-bit architectures.

So I think this package can be uploaded now.

> I assume you neither have access to porterboxes nor an array of hardware at
> home (and qemu can be tricky), so it might be easiest for you to abuse the
> buildds or someone who can test.

I have only an amd64 machine, so I plan to abuse the buildd.

> Other issues:
>
> Please add:
> Multi-Arch: same
> to libhighwayhash0's section in the control file.  You install stuff to the
> proper paths so there's no reason for it to be not coinstallable.

Done.

> Some simple docs would be nice -- RTFSing the headers is not pretty.
> This could work:

Thank you for the manpage snippet, I've completed the manpage and
added it to the -dev package.

> Manually mucking with optimization variant selection provides only a very
> minor speedup, so I wouldn't bother describing those.  Especially that with
> gcc-6 per-ISA variants can be bound at library load time -- if the library
> uses that functionality it could eliminate the overhead altogether.  That's
> a matter for the upstream, though.

I have no intention to make a exhaustive list of interfaces in the
manpage, neither.
Four examples from upstream README should be enough for the manpage use.
(see highwayhash.3)

> Lemme share a sanity test I used:

The snippet is modified and used as autopkgtest :-)

Thank you for the suggestions which make the package in a better quality!


Reply to: