Re: [PATCH] scripts: kernel-doc: allow passing desired Sphinx C domain dialect

From: Mauro Carvalho Chehab
Date: Mon Oct 12 2020 - 08:27:59 EST


Em Tue, 6 Oct 2020 08:01:34 -0600
Jonathan Corbet <corbet@xxxxxxx> escreveu:

> On Tue, 6 Oct 2020 08:42:07 +0200
> Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx> wrote:
>
> > As right now we don't support Sphinx version 3.0[1], we're actually using just
> > $sphinx_major. So, I'm wonder if it would make sense to also make <minor>
> > optional.
>
> Maybe...someday we may need it, knowing how the Sphinx folks approach
> compatibility, but I guess we can always add it then if so.
>
> > The change would be trivial, although the regex will become even more
> > harder to read ;-)
>
> ^(\d+)(\.(\d+)){,2}
>
> ? (untested, of course)

Didn't work (perl complains about its syntax), and neither:

^(\d+)(\.(\d+)){0,2}

I also tried this:

^(\d+)(?:\.(\d+)){0,2}

But both ends misplacing the second group information when
x.y.z is used.

On the tests I did, this is the one that worked fine:

if ($ver_string =~ m/^(\d+)(?:\.(\d+))?(?:\.(\d+))?/) {
$sphinx_major = $1;
if (defined($2)) {
$sphinx_minor = $2;
} else {
$sphinx_minor = 0;
}
if (defined($3)) {
$sphinx_patch = $3
} else {
$sphinx_patch = 0;
}

Or the alternative one, using substrings:

if ($ver_string =~ m/^(\d+)(\.\d+)?(\.\d+)?/) {
$sphinx_major = $1;
if (defined($2)) {
$sphinx_minor = substr($2,1);
} else {
$sphinx_minor = 0;
}
if (defined($3)) {
$sphinx_patch = substr($3,1)
} else {
$sphinx_patch = 0;
}
}

I'll keep the last one, as it is likely simpler to understand.

>
> > [1] not sure how valuable would be adding support for Sphinx 3.0. While
> > I didn't make any tests, I'm strongly suspecting that, with the approach
> > we took for backward/forward compatibility, adding support for it
> > would mean to just do a trivial change at cdomain.py by applying a
> > patch that Markus did replacing a regex function that doesn't exist
> > anymore at Sphinx API and emulating C namespace with the logic I
> > already implemented.
>
> 3.0 might just be skippable at this point, methinks.

Yeah, agreed.

I'll fold the latest version of this patch with the following diff.

Thanks,
Mauro

index 297312824d26..c8f6b11d5da1 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -466,11 +466,15 @@ while ($ARGV[0] =~ m/^--?(.*)/) {
$show_not_found = 1; # A no-op but don't fail
} elsif ($cmd eq "sphinx-version") {
my $ver_string = shift @ARGV;
- if ($ver_string =~ m/^(\d+)\.(\d+)(\.\d+)?/) {
+ if ($ver_string =~ m/^(\d+)(\.\d+)?(\.\d+)?/) {
$sphinx_major = $1;
- $sphinx_minor = $2;
+ if (defined($2)) {
+ $sphinx_minor = substr($2,1);
+ } else {
+ $sphinx_minor = 0;
+ }
if (defined($3)) {
- $sphinx_patch = substr($3,1);
+ $sphinx_patch = substr($3,1)
} else {
$sphinx_patch = 0;
}
@@ -2368,7 +2372,10 @@ sub process_file($) {
}


-get_sphinx_version() if (!$sphinx_major);
+if ($output_mode eq "rst") {
+ get_sphinx_version() if (!$sphinx_major);
+}
+
$kernelversion = get_kernel_version();

# generate a sequence of code that will splice in highlighting information