[07:46] <mchehab> sailus: I was thinking about the ISP... I don't think it is a good idea to put at the document that 3 people will handle it.... [07:47] <mchehab> I mean, the goal of this document is to point important things for the ones that will be submitting patches [07:47] <mchehab> it is not about how we'll handle stuff internally [07:48] <mchehab> from such PoV, all the developer needs to know is to whom he'll contact if something gets wrong [07:49] <mchehab> s/to whom/who/ [07:51] <mchehab> besides that, Patchwork has a field to indicate when a patch got delegated to someone else [07:51] <mchehab> so, all we need to document is who will be the "entry point" for ISP patches [07:55] <mchehab> we can add a note there saying that, during a patch analisys, other media maintainers may be handling the patch. On such cases, patchwork "delegate" field will indicate who is responsible for reviewing the patch. [07:57] <hverkuil> mailinglist is backed up yet again. No new emails since yesterday evening. [08:07] <mchehab> gah [08:07] <mchehab> v4l-utils Coverity build broke. Did someone added some new ./configure parameter that it is not auto-detected? [08:08] <mchehab> or added some new build dependence? [08:09] <hverkuil> Broke how? There is an issue with gettext version in configure.ac: it was accidentally part of another commit, but I reverted the configure.ac change yesterday. [08:11] <mchehab> Your request for analysis of v4l-utils is failed. [08:11] <mchehab> Analysis status: Failure [08:11] <mchehab> Please fix the error and upload the build again. [08:11] <mchehab> Error details: [08:11] <mchehab> The build uploaded has not been fully compiled. Please fix all compilation errors for accurate analysis. [08:12] <hverkuil> Was that build done with or without commit cbbb3ee34ee7ad3bceec7b6a69547d464cc9bee0 ? [08:14] <mchehab> 2021-03-02T07:04:18.947825Z|cov-build|5262|info|> [WARNING] No files were emitted. This may be due to a problem with your configuration [08:14] <mchehab> 2021-03-02T07:04:18.947825Z|cov-build|5262|info|> or because no files were actually compiled by your build command. [08:14] <mchehab> 2021-03-02T07:04:18.947825Z|cov-build|5262|info|> Please make sure you have configured the compilers actually used in the compilation. [08:14] <mchehab> 08:03:11 Checking out Revision cbbb3ee34ee7ad3bceec7b6a69547d464cc9bee0 (refs/remotes/origin/master) [08:15] <mchehab> let me add a make clean at the script [08:16] <mchehab> I'm suspecting that, without a make clean, the build may fail [08:19] <mchehab> v4l2-compliance-32 -static -m32 -DNO_LIBV4L2 -I../.. -I../../include -I../../utils/common -DGIT_SHA=cbbb3ee34ee7 -DGIT_COMMIT_CNT=-4736 [08:19] <mchehab> out of curiosity: what that "-4736" means? [08:20] <mchehab> GIT_COMMIT_CNT=-4736 [08:20] <hverkuil> number of commits. Increases by 1 for every commit. [08:20] <mchehab> shoudn't it be a positive number? [08:21] <hverkuil> It's appended to a string: v4l2-ctl 1.21.0-4734 [08:21] <mchehab> ah [08:22] <mchehab> 09:23:46 2021-03-02T08:23:20.152036Z|cov-build|3432|info|> [WARNING] Emitted 63 C/C++ compilation units (66%) successfully [08:22] <mchehab> 09:23:46 2021-03-02T08:23:20.152036Z|cov-build|3432|info|> [08:22] <mchehab> 09:23:46 2021-03-02T08:23:20.152036Z|cov-build|3432|info|> 406 C/C++ compilation units (86%) are ready for analysis [08:23] <mchehab> ok, it seems it will build... yet, 86% is too close to the lower limit for it to be accepted [08:24] <mchehab> https://scan.coverity.com/download [08:24] <mchehab> IMPORTANT - Your build will be rejected if at least 85% units of code are not compiled. [08:25] <hverkuil> bootstrap.sh will modify AM_GNU_GETTEXT_VERSION in configure.ac to show the current version number of the installed gettext tools. This is a pain since when you do a commit -a it will also pick up that changed configure.ac. But then it will only build on systems that have that version or up installed. [08:26] <hverkuil> what's the configure line you use for coverity? [08:27] <hverkuil> Also, a lot of stuff in contrib isn't build automatically AFAIK. [08:31] <mchehab> ./bootstrap.sh [08:31] <mchehab> ./configure --enable-gconv --enable-v4l2-compliance-32 --enable-v4l2-ctl-32 --enable-bpf [08:31] <mchehab> make clean [08:31] <mchehab> PATH=/opt/cov-analysis-linux64/bin:$PATH [08:31] <mchehab> cov-build --dir cov-int make [08:31] <mchehab> tar czvf v4l_utils.tgz cov-int [08:31] <mchehab> upload_cv.sh v4l_utils [08:31] <mchehab> tail cov-int/build-log.txt [08:32] <mchehab> hverkuil: on another project (ZBar), I dropped AM_GNU_GETTEXT_VERSION [08:32] <mchehab> it produces a harmless warning [08:33] <hverkuil> Hmm, I think AM_GNU_GETTEXT_REQUIRE_VERSION might be the right name to use. That isn't changed. [08:34] <mchehab> those are used to decide if autopoint will run or not [08:35] <mchehab> I guess that, if we do: AM_GNU_GETTEXT([external]) [08:36] <mchehab> then it should work without needing to place a version there [08:36] <mchehab> https://www.gnu.org/software/gnulib/manual/html_node/gettextize-and-autopoint.html [08:37] <mchehab> explains how the gettext version detection works [08:38] <mchehab> autoreconf will automatically call autopoint, so, despite the warning, everything should work [08:39] <hverkuil> https://www.spinics.net/lists/ac/msg13686.html [08:41] <mchehab> btw, Gregor had some issues with the current configure.ac... he pinged me last month [08:42] <mchehab> > > with newer gettext (> 0.19) I'm facing the problem of a failing build: [08:42] <mchehab> > > [08:42] <mchehab> > > *** error: gettext infrastructure mismatch: using aMakefile.in.in <http://Makefile.in.in> from gettext version 0.20 but the autoconf macros are from gettext version 0.19 [08:43] <mchehab> maybe due to the bug reported at the sphincs post you pointed [08:50] <hverkuil> Based on this: https://www.gnu.org/software/gettext/manual/html_node/autopoint-Invocation.html I would say that using AM_GNU_GETTEXT_REQUIRE_VERSION instead of AM_GNU_GETTEXT_VERSION is the right approach. [08:51] <mchehab> if you use require, things will break if the version is not identical [08:51] <hverkuil> No, it's defined as that version or up. Not that version exactly. [08:53] <mchehab> I guess we tested using this already [08:55] <mchehab> yet, we could try it... [08:56] <mchehab> but the best would be to set a container with an older version of gettext and see how it behaves there, before pushing such change upstrteam [08:56] <hverkuil> Nah, doesn't work when I try that in a clean cloned repo. [08:56] <hverkuil> Just dropping AM_GNU_GETTEXT_VERSION appears to do the job. [08:57] <mchehab> it seems we tried this too: [08:57] <mchehab> ce9f9d8f882b configure.ac: remove gettext version [08:57] <mchehab> and then: [08:57] <mchehab> b335ef5696dc configure.ac: Add AM_GNU_GETTEXT_VERSION([0.19.8]) [09:01] <hverkuil> Or we restore configure.ac in bootstrap.sh: i.e. make a copy before the ./gettextize commands and restore it afterwards. [09:01] <mchehab> dirty, but will do the job [09:02] <mchehab> you'll need to add a trap, in case some error happens when autopoint/reconf is called [09:03] <mchehab> or if someone aborts with break [09:08] <hverkuil> diff --git a/bootstrap.sh b/bootstrap.sh [09:08] <hverkuil> index 0e9eb2d4..52350d86 100755 [09:08] <hverkuil> --- a/bootstrap.sh [09:08] <hverkuil> +++ b/bootstrap.sh [09:08] <hverkuil> @@ -13,6 +13,9 @@ if [ "GETTEXTIZE" != "" ]; then [09:08] <hverkuil> sed "s,read dummy < /dev/tty,," < $GETTEXTIZE > ./gettextize [09:08] <hverkuil> chmod 755 ./gettextize [09:08] <hverkuil> + cp configure.ac configure.ac.orig [09:08] <hverkuil> + trap "mv configure.ac.orig configure.ac" EXIT [09:08] <hverkuil> + [09:08] <hverkuil> echo "Generating locale v4l-utils-po build files for gettext version $VER" [09:08] <hverkuil> ./gettextize --force --copy --no-changelog --po-dir=v4l-utils-po >/dev/null [09:09] <mchehab> that should do the job... [09:09] <mchehab> I would add a "set -e" at the beginning of the script [09:10] <mchehab> as it currently assumes that no error will ever happen while running it ;-) [09:34] <hverkuil> It turns out that you have to add AM_GNU_GETTEXT_REQUIRE_VERSION in addition to AM_GNU_GETTEXT_VERSION in order to support gettext 0.21: [09:34] <hverkuil> https://gitlab.gnome.org/GNOME/gcr/-/commit/3c2d5803b1149a0d2ea1669fac8874f903e30c22 [09:34] <hverkuil> Without it make will fail: [09:34] <hverkuil> $ make -j [09:34] <hverkuil> make --no-print-directory all-recursive [09:34] <hverkuil> Making all in v4l-utils-po [09:34] <hverkuil> *** error: gettext infrastructure mismatch: using a Makefile.in.in from gettext version 0.20 but the autoconf macros are from gettext version 0.19 [09:35] <mchehab> no need. I'm using it here: [09:35] <mchehab> $ gettext --version [09:35] <mchehab> gettext (GNU gettext-runtime) 0.21 [09:35] <mchehab> on Fedora 33 [09:36] <hverkuil> Have you tried a clean clone? [09:36] <hverkuil> it only fails for me with a clean clone, then bootstrap.sh followed by configure followed by make. [09:36] <mchehab> trying now [09:37] <mchehab> Generating locale v4l-utils-po build files for gettext version 0.21 [09:37] <mchehab> Generating locale libdvbv5-po build files for gettext version 0.21 [09:37] <mchehab> worked [09:37] <hverkuil> autoreconf --version ? [09:37] <hverkuil> (2.69 for me) [09:37] <mchehab> $ autoreconf --version [09:37] <mchehab> autoreconf (GNU Autoconf) 2.69 [09:38] <mchehab> everything built fine [09:38] <mchehab> 242ad0b774c7 (HEAD -> master, origin/master, origin/HEAD) cec-compliance: improve current latency checks [09:38] <hverkuil> ah, do a git pull [09:39] <hverkuil> that commit accidentally incremented the minimum gettext version to 0.21. [09:41] <mchehab> make [09:41] <mchehab> make --no-print-directory all-recursive [09:41] <mchehab> Making all in v4l-utils-po [09:41] <mchehab> *** error: gettext infrastructure mismatch: using a Makefile.in.in from gettext version 0.20 but the autoconf macros are from gettext version 0.19 [09:41] <mchehab> btw, our bootstrap.sh is weird [09:42] <mchehab> I'm wandering if it could be the culprit [09:45] <mchehab> here, dropping AM_GNU_GETTEXT_VERSION solved the issue [09:45] <mchehab> $ git diff -U0 [09:45] <mchehab> diff --git a/configure.ac b/configure.ac [09:45] <mchehab> index 5290fa015177..fe25b9985ce1 100644 [09:45] <mchehab> --- a/configure.ac [09:45] <mchehab> +++ b/configure.ac [09:45] <mchehab> @@ -100 +99,0 @@ ALL_LINGUAS="" [09:45] <mchehab> -AM_GNU_GETTEXT_VERSION([0.19.8]) [09:45] <mchehab> IMHO, this is the cleanest way [09:46] <mchehab> (tested on a new clone) [09:46] <mchehab> but I don't know if this would work fine with autoconf 2.70 [09:49] <mchehab> this also worked on a new clone: [09:49] <mchehab> $ git diff -U0 [09:49] <mchehab> diff --git a/configure.ac b/configure.ac [09:49] <mchehab> index 5290fa015177..bc37055e4b06 100644 [09:49] <mchehab> --- a/configure.ac [09:49] <mchehab> +++ b/configure.ac [09:49] <mchehab> @@ -100 +100 @@ ALL_LINGUAS="" [09:49] <mchehab> -AM_GNU_GETTEXT_VERSION([0.19.8]) [09:49] <mchehab> +AM_GNU_GETTEXT_REQUIRE_VERSION([0.19.8]) [09:49] <hverkuil> That won't work (tried it). [09:50] <mchehab> here, it worked, but yeah, I remember I tried this before (maybe on another repo) [09:50] <mchehab> and someone complained it broke build for him [09:50] <hverkuil> Older autopoint versions do not recognize AM_GNU_GETTEXT_REQUIRE_VERSION. [09:51] <mchehab> IMO, the best would be to test removing AM_GNU_GETTEXT_VERSION with autoconf 2.70 [09:51] <mchehab> I guess Ubuntu SID comes with 2.70 [09:51] <hverkuil> My current patch works with 2.69, and I am now testing on a system with gettext-0.19.8. [09:54] <hverkuil> debian sid is still on 2.69 [10:35] <hverkuil> Posted two patches for v4l-utils. Tested in various environments, including Cisco's internal build. [10:51] <mchehab> hmm... you could use m4_ifndef() and m4_ifdef() to check if a var exists at configure.ac [10:52] <mchehab> m4_ifdef([AM_GNU_GETTEXT_REQUIRE_VERSION],[ AM_GNU_GETTEXT_REQUIRE_VERSION([0.19.8]) ]) [10:53] <mchehab> btw, this package uses something like that: https://github.com/p11-glue/p11-kit/blob/master/configure.ac [10:54] <mchehab> https://github.com/centic9/dpkg-ppa/blob/master/configure.ac has a similar check [10:55] * pinchartl wonders if we'd have the same issue with meson :-D [10:56] <mchehab> we'll likely have another set of issues with meson, that will likely break when either meson or python version changes :-p [10:57] <mchehab> and, btw, this issue is related to gettext, so it is likely that it will also affect meson somehow [11:01] <pinchartl> what's the status of meson support in v4l-utils btw, what's blocking/stalling it at the moment ? [11:03] <mchehab> hverkuil: what about this: [11:03] <mchehab> diff --git a/configure.ac b/configure.ac [11:03] <mchehab> index 5290fa015177..a1d0acfb2af9 100644 [11:03] <mchehab> --- a/configure.ac [11:03] <mchehab> +++ b/configure.ac [11:03] <mchehab> @@ -100 +100 @@ ALL_LINGUAS="" [11:03] <mchehab> -AM_GNU_GETTEXT_VERSION([0.19.8]) [11:03] <mchehab> +m4_ifdef(AM_GNU_GETTEXT_REQUIRE_VERSION,[AM_GNU_GETTEXT_REQUIRE_VERSION(0.19.8)],[AM_GNU_GETTEXT_VERSION([0.19.8])]) [11:03] <mchehab> the m4_ifdef line is enough to avoid bootstrap.sh to rewrite the version number [11:05] <mchehab> and, as this is checking if AM_GNU_GETTEXT_REQUIRE_VERSION exists, this should also work with older versions [11:06] <mchehab> on Fedora 33, this works fine (autoconf 2.69, gettext 0.21) [11:08] <pinchartl> btw, any reason we're discussing this here and no in #v4l ? [11:16] <hverkuil> mchehab: nice one, I'll try this. [11:20] <hverkuil> mchehab: cool, works perfectly. No need to mess with bootstrap.sh that way. [11:20] <mchehab> great! [11:21] <hverkuil> Do you want to commit this? (your idea, after all!) [11:21] <mchehab> you tested and rised the question.... feel free to commit [11:21] <mchehab> btw, as a matter of coherence... [11:22] <hverkuil> OK, will do. [11:22] <mchehab> I guess the version number should either be [0.19.8] or 0.19.8 on both macros [11:22] <mchehab> (my patch mixed both syntaxes ) [11:22] <hverkuil> I'll fix that [11:24] <mchehab> thanks! [11:52] <mchehab> built fine on Ubuntu 20.04 with 0.19.8.1 [11:52] <mchehab> just one warning [11:52] <mchehab> control/libv4lcontrol.c: In function ‘v4lcontrol_create’: [11:52] <mchehab> control/libv4lcontrol.c:728:3: warning: ignoring return value of ‘ftruncate’, declared with attribute warn_unused_result [-Wunused-result] [11:52] <mchehab> 728 | ftruncate(shm_fd, V4LCONTROL_SHM_SIZE); [11:52] <mchehab> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [11:56] <hverkuil> old warning, has been there for ages, I think. [12:00] <mchehab> warnings are more visible now that the build is less verbose ;-)