Re: [PATCH 1/2] v4l2-compliance: Add version command

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Hans,

On Fri, Jul 10, 2020 at 03:33:25PM +0200, Hans Verkuil wrote:
> On 10/07/2020 15:25, Laurent Pinchart wrote:
> > On Fri, Jul 10, 2020 at 10:18:12PM +0900, Paul Elder wrote:
> >> Add a --version option to v4l2-compliance to retrieve the version of
> >> v4l2-compliance.
> >>
> >> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> >> ---
> >>  utils/v4l2-compliance/v4l2-compliance.cpp | 11 +++++++++++
> >>  1 file changed, 11 insertions(+)
> >>
> >> diff --git a/utils/v4l2-compliance/v4l2-compliance.cpp b/utils/v4l2-compliance/v4l2-compliance.cpp
> >> index 4b45f110..72b9768f 100644
> >> --- a/utils/v4l2-compliance/v4l2-compliance.cpp
> >> +++ b/utils/v4l2-compliance/v4l2-compliance.cpp
> >> @@ -79,6 +79,7 @@ enum Option {
> >>  	OptMediaBusInfo = 'z',
> >>  	OptStreamFrom = 128,
> >>  	OptStreamFromHdr,
> >> +	OptVersion,
> >>  	OptLast = 256
> >>  };
> >>  
> >> @@ -153,9 +154,15 @@ static struct option long_options[] = {
> >>  	{"stream-all-formats", optional_argument, 0, OptStreamAllFormats},
> >>  	{"stream-all-io", no_argument, 0, OptStreamAllIO},
> >>  	{"stream-all-color", required_argument, 0, OptStreamAllColorTest},
> >> +	{"version", no_argument, 0, OptVersion},
> >>  	{0, 0, 0, 0}
> >>  };
> >>  
> >> +static void version()
> >> +{
> >> +	printf("v4l2-compliance " PACKAGE_VERSION "\n");
> > 
> > Is it enough to rely on the v4l-utils package version, or should we add
> > a git commit count as well ? The traditional version number will make it
> > difficult to test for features added between two released versions.
> 
> If you add a version option, then v4l2-compliance should also show the SHA.
> It's already available (grep for SHA), so easy enough to add here.

The issue with the SHA is that, while it identifies the exact commit, it
is useless to compare versions. We are using v4l2-compliance to test the
libcamera V4L2 compatibility layer, and this depends on recent features
merged in the master branch but not available in a release yet. We would
like the test to be skipped if the v4l2-compliance is too old. Printing
the package version is a good step forward, but would require waiting
for the next release before the test can be enabled. That's probably OK
overall, but it's a bit annoying during development. That's why I was
wondering if a commit count (as output by git rev-list --count HEAD)
would be useful too. In our case, the fact that v4l2-compliance supports
the --version option will be enough to know it's recent enough, but I'm
thinking about the future (for libcamera and other users).

> Also, if you add --version here, then it really should be added to most
> other utils as well (certainly media-ctl and cec-follower/ctl/compliance).
> 
> >> +}
> >> +
> >>  static void usage()
> >>  {
> >>  	printf("Usage:\n");
> >> @@ -244,6 +251,7 @@ static void usage()
> >>  	printf("  -P, --no-progress  Turn off progress messages.\n");
> >>  	printf("  -T, --trace        Trace all called ioctls.\n");
> >>  	printf("  -v, --verbose      Turn on verbose reporting.\n");
> >> +	printf("  --version          Show version information.\n");
> >>  #ifndef NO_LIBV4L2
> >>  	printf("  -w, --wrapper      Use the libv4l2 wrapper library.\n");
> >>  #endif
> >> @@ -1664,6 +1672,9 @@ int main(int argc, char **argv)
> >>  		case OptNoProgress:
> >>  			no_progress = true;
> >>  			break;
> >> +		case OptVersion:
> >> +			version();
> >> +			std::exit(EXIT_SUCCESS);
> >>  		case ':':
> >>  			fprintf(stderr, "Option `%s' requires a value\n",
> >>  				argv[optind]);

-- 
Regards,

Laurent Pinchart