[Debian/Build] Build ml-api on Ubuntu 26.04 (gcc fallback, C++17) - #692
[Debian/Build] Build ml-api on Ubuntu 26.04 (gcc fallback, C++17)#692myungjoo wants to merge 2 commits into
Conversation
The compiler alternatives stop at gcc-9, which no longer exists on Ubuntu 24.10 and later; Ubuntu 26.04 (resolute) ships gcc-13/14/15 only. Every Launchpad recipe build of ml-api for those series therefore stops at the build-dependency stage. Append the unversioned "gcc" as the last alternative so apt falls back to the archive default, exactly as nnstreamer and nnstreamer-edge already do. The older alternatives stay first, so 22.04 and 24.04 resolve exactly as before. The nnstreamer PPA now publishes nnstreamer, nnstreamer-edge, ssat, tensorflow2-lite and edgetpu for 26.04, so this is the only change ml-api needs to build there. No functional change; the packaging build was verified in an ubuntu:26.04 container against ppa:nnstreamer/ppa. Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
[Automated review by a separate Claude agent, relayed to this PR by the author's account] Reviewed both commits (7ffa297, 492e55e) against a fresh checkout of the branch. Verdict: approve. No blocking findings; one minor commit-message nit. What was checked
Findings (ranked)
Nothing else. The change is the minimum needed and mirrors what nnstreamer (#4895) and nnstreamer-edge (#256) already merged. |
GoogleTest 1.17, which Ubuntu 26.04 ships, refuses anything older than C++17 with "#error C++ versions less than C++17 are not supported", so every gtest binary of ml-api fails to compile there while the libraries themselves build fine. Raise the meson default from c++14 to c++17. The Android build already uses -std=c++17 and nnstreamer itself has been on c++17 for a long time, so this only aligns the Ubuntu build with them. Every compiler a current series can resolve from the packaging, gcc-9 and later, supports it. Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
492e55e to
792c886
Compare
|
[Relayed on behalf of the author by a Claude agent] Thanks. Finding 1 is fixed: the second commit's message now says "every compiler a current series can resolve from the packaging, gcc-9 and later, supports it" instead of the wrong gcc-7 claim (commit message only, no content change; force-pushed to the same branch). The gcc-5/6 alternatives are left as they are in this PR since no series that still has them is affected. Finding 2 is understood: 26.04 coverage comes from the Launchpad recipe once Resolute is added there, which is the next step after this merges. |
The compiler alternatives stop at gcc-13, which Ubuntu 26.04 (resolute) no longer ships, so the Launchpad recipe build of nntrainer for that series stops at the build-dependency stage. Append the unversioned "gcc" as the last alternative so apt falls back to the archive default, as nnstreamer and nnstreamer-edge already do (ml-api gets the same in nnstreamer/api#692). The versioned alternatives stay first, so 22.04 and 24.04 resolve exactly as before. Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
[Automated re-review by a separate Claude agent, relayed to this PR by the author's account] Re-reviewed after the force-push that rewords commit 792c886's message (7ffa297 unchanged). Verified directly against Verdict: approve. The prior minor finding is fixed, and no new issues were found on independent re-verification. Prior finding re-checked
Independent verification performed this pass
NoteThe PR is still marked Nothing else to add; this remains the minimal change needed and continues to mirror the fallback pattern already merged in nnstreamer and nnstreamer-edge. |
myungjoo-bot
left a comment
There was a problem hiding this comment.
Automated review (transcribed from an AI review agent's report; please verify before acting).
Summary: The PR appends the unversioned gcc as the last Build-Depends alternative in debian/control and raises meson's cpp_std from c++14 to c++17 so the gtest binaries compile against GoogleTest 1.17 on Ubuntu 26.04. Verified: Debian alternative syntax and archive facts via the Launchpad API (jammy/noble still publish gcc-9 in universe; oracular, plucky, questing and resolute do not; resolute's default gcc is 4:15.2.0 and libgtest-dev is 1.17.0); no other file pins a C++ standard (Android Application.mk already uses -std=c++17, the Tizen spec strips -std=gnu++11 so meson's value wins, no cpp_args / override_options elsewhere); the only C++ sources are the nine tests/capi/*.cc files and none use C++17-removed constructs; the Tizen GBS x86_64 lane with unit_test 1 built the PR head 792c886 with GCC 14.2.0, compiled all C++ objects, linked all nine gtest binaries and ran them; merge-tree against main is clean; both commits carry Signed-off-by with accurate messages. The sibling changes nnstreamer/nnstreamer#4895 and nnstreamer/nnstreamer-edge#256 apply the byte-identical | gcc suffix, and nnstreamer's meson.build is already at cpp_std=c++17. Approving; the items below are non-blocking.
- [Low] The Ubuntu pdebuild path is not CI-verified for this PR because the workflow is disabled —
.github/workflows/pdebuild.yml:3-5declarespull_request: branches: [main], but the Actions API reports the workflow asdisabled_inactivity(last run: scheduled, 2026-08-09 one7fedae), and no pdebuild run exists for792c886. The earlier automated comments on this PR saying "this PR's jammy pdebuild job will confirm it" are therefore not accurate. For a 2-line change this is acceptable given the GBS lane compiled every gtest binary undercpp_std=c++17and the author reports a fulldpkg-buildpackagein anubuntu:26.04container, but please re-enable the workflow (gh workflow enable pdebuild.yml --repo nnstreamer/api) and ideally re-trigger it on this PR so the jammy path (gcc-11 + gtest 1.11 +-Werror) is covered too.daily-build-gbs.ymlandnns_api_testhub.ymlare alsodisabled_inactivity. - [Low] The compiler alternative list is vestigial; no inconsistency, but the commit text slightly overstates its effect —
debian/rulesnever setsCC/CXXand there is nog++alternative, so meson pickscc/c++frombuild-essential. On jammy/noble apt installsgcc-9first as the commit says, but the actual compiler is gcc-11 / gcc-13, and gcc-15 on resolute. The fallback is safe and mirrors nnstreamer / nnstreamer-edge exactly, so no change requested. Optional follow-up across all three repos: replace thegcc-9 | ... | gcc-5 (>= 5.4) | gccgroup with plaingcc, g++(or rely onbuild-essential). - [Low] Docs / changelog: nothing to update. No README or Documentation file mentions the C++ standard or compiler version, and
debian/changelogis bumped only at release commits.
No back-door or suspicious behavior found: the diff is exactly two lines in two build files, with no scripts, URLs, dependencies, or workflow changes.
|
Libraries we use are requiring C++17 these days :( |
Why
ppa:nnstreamer/ppanow publishes nnstreamer, nnstreamer-edge, ssat, tensorflow2-lite and edgetpu for Ubuntu 26.04 (resolute), butml-apicannot follow: the Launchpad recipennstreamer-ml-api-dailystops at the build-dependency stage on 24.10 and later because every compiler alternative indebian/control(gcc-9 | gcc-8 | ... | gcc-5) has been dropped from the archive. Once past that, all four gtest binaries fail to compile on 26.04 because GoogleTest 1.17 refuses anything older than C++17.What
debian/control: append the unversionedgccas the last compiler alternative, the same fallback nnstreamer (#4895) and nnstreamer-edge ([Build] Compile and package on Ubuntu 26.04 (gcc 15) nnstreamer-edge#256) already carry. 22.04 and 24.04 still resolvegcc-9first, so nothing changes there.meson.build:cpp_std=c++14->c++17. The Android build already passes-std=c++17and nnstreamer has been on C++17 for years.How it was verified
Full packaging build in an
ubuntu:26.04container withppa:nnstreamer/ppaenabled,mk-build-depsfrom the modified control, thendpkg-buildpackage -us -uc -b(gcc 15.2.0, meson 1.10, openjdk 11 from the archive). Without the second commit the build stops within
unittest_capi_inference,unittest_capi_inference_single,unittest_capi_inference_latencyandunittest_capi_datatype_consistency. With both commits the build completes,override_dh_auto_test(packaging/run_unittests.sh) runs, and all eight.debs are produced.No functional change, so no new test case; the existing unit tests are what the C++17 change makes buildable again on 26.04.
Part of the 26.04 PPA work tracked in nnstreamer/nnstreamer#4902. After this merges, the Launchpad recipe needs Resolute added and a build requested (I can do that).
🤖 Generated with Claude Code