edgeai-apps-utils: add support to build edgeai-apps-utils package#21
edgeai-apps-utils: add support to build edgeai-apps-utils package#21jsuhaas22 wants to merge 1 commit intoTexasInstruments:masterfrom
Conversation
| dh $@ | ||
|
|
||
| override_dh_auto_configure: | ||
| mkdir build/ && cd build/ && cmake .. |
There was a problem hiding this comment.
Current debhelper should be able to build cmake projects automatically, these overrides shouldn't be needed.
| @@ -0,0 +1 @@ | |||
| version=3 | |||
There was a problem hiding this comment.
Empty watch files don't help, just remove this file.
|
|
||
| mkdir -p /opt | ||
|
|
||
| git clone https://git.ti.com/git/edgeai/edgeai-apps-utils /opt/edgeai-apps-utils |
There was a problem hiding this comment.
Nak, just copy the source to somewhere more standard, like /usr/src or /usr/share. Have it as part of the package content, no .postinst downloading/magic please, packages should install the same without network access.
| Upstream-Contact: Texas Instruments | ||
| Source: https://git.ti.com/cgit/edgeai/edgeai-apps-utils | ||
|
|
||
| Files: include/edgeai_dl_color_convert_armv8_utils.h |
There was a problem hiding this comment.
I know this file is auto-generated, but you should fix it up a little. All files have the same license, use wildcard * to collect them all. Then the License is not "UNKNOWN" it is a TI BSD-3-clause. See how I did the same fix for k3conf:
| Homepage: https://git.ti.com/cgit/edgeai/edgeai-apps-utils | ||
| Rules-Requires-Root: no | ||
|
|
||
| Package: libedgeai-apps-utils0.1.0 |
There was a problem hiding this comment.
Shouldn't have versions in the package name unless we expect to have multiple version installed at the same time.
| @@ -0,0 +1,5 @@ | |||
| edgeai-apps-utils (0.1+git20240205+8e84be376-1) UNRELEASED; urgency=low | |||
There was a problem hiding this comment.
UNRELEASED causes issues with some tools, every time these files are updated in the main branch they would be built and "released", so you can just set this to unstable or bookworm.
2abc4e6 to
439adab
Compare
|
Thanks @glneo for the review. I have done the changes. Not sure if I got the copyright file right. |
|
Looks much better, thanks! |
Signed-off-by: Suhaas Joshi <s-joshi@ti.com>
439adab to
4fad8d5
Compare
|
Removed the source code from the package. |
| @@ -0,0 +1,6 @@ | |||
| #!/usr/bin/make -f | |||
|
|
|||
| export SOC=am62x | |||
There was a problem hiding this comment.
So here is a problem, for some reason they made part of this program SoC specific, instead of just doing runtime detection of the platform. This means they need a different binary compiled for each and every SoC. And you would then need a different package for each supported SoC. That is bad software design. They might get away with it in Yocto where the whole system is rebuild for a given machine, but not normal distros.
Let's fix this at the source, go let the edgeai-apps-utils folks know this won't work. They need fix their compile-time hard-coding and instead do runtime detection, just like we do in k3conf. They can use that as an example even.
There was a problem hiding this comment.
Makes sense.
I talked to the edgeai team. They have agreed to do it, but they said that it will require some work. They said that it may not be possible in the near-future, but they'll try to squeeze it in 9.2 release.
|
@jsuhaas22 , @jonaswood01 , @cshilwant this PR is still in draft state. Can you align and take it forward towards integration closure path |
No description provided.