[v2,for-4.14] docs/support-matrix: unbreak docs rendering

Commit Message

Andrew Cooper June 8, 2020, 1:34 p.m. UTC

The cronjob which renders https://xenbits.xen.org/docs/ has been broken for a
while.  commitish_version() pulls an old version of xen/Makefile out of
history, and uses the xenversion rule.

Currently, this fails with:

  tmp.support-matrix.xen.make:130: scripts/Kbuild.include: No such file or directory

which is because the Makefile legitimately references Kbuild.include with a
relative rather than absolute path.

Rework support-matrix-generate to use sed to extract the major/minor, rather
than expecting xen/Makefile to be usable in a different tree.

Signed-off-by: Andrew Cooper <[e-mail]>
---
CC: George Dunlap <[e-mail]>
CC: Ian Jackson <[e-mail]>
CC: Jan Beulich <[e-mail]>
CC: Konrad Rzeszutek Wilk <[e-mail]>
CC: Stefano Stabellini <[e-mail]>
CC: Wei Liu <[e-mail]>
CC: Julien Grall <[e-mail]>
CC: Anthony PERARD <[e-mail]>
CC: Paul Durrant <[e-mail]>

v2:
 * Use sed rather than fixing up the makefile environment

This needs backporting to all trees with the support matrix logic, to unbreak
docs rendering
---
 docs/support-matrix-generate | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

Comments

Paul Durrant June 8, 2020, 1:38 p.m. UTC | #1

> -----Original Message-----
> From: Andrew Cooper <[e-mail]>
> Sent: 08 June 2020 14:35
> To: Xen-devel <[e-mail]>
> Cc: Andrew Cooper <[e-mail]>; George Dunlap <[e-mail]>; Ian
> Jackson <[e-mail]>; Jan Beulich <[e-mail]>; Konrad Rzeszutek Wilk
> <[e-mail]>; Stefano Stabellini <[e-mail]>; Wei Liu <[e-mail]>; Julien
> Grall <[e-mail]>; Anthony PERARD <[e-mail]>; Paul Durrant <[e-mail]>
> Subject: [PATCH v2 for-4.14] docs/support-matrix: unbreak docs rendering
> 
> The cronjob which renders https://xenbits.xen.org/docs/ has been broken for a
> while.  commitish_version() pulls an old version of xen/Makefile out of
> history, and uses the xenversion rule.
> 
> Currently, this fails with:
> 
>   tmp.support-matrix.xen.make:130: scripts/Kbuild.include: No such file or directory
> 
> which is because the Makefile legitimately references Kbuild.include with a
> relative rather than absolute path.
> 
> Rework support-matrix-generate to use sed to extract the major/minor, rather
> than expecting xen/Makefile to be usable in a different tree.
> 
> Signed-off-by: Andrew Cooper <[e-mail]>

Release-acked-by: Paul Durrant <[e-mail]>

> ---
> CC: George Dunlap <[e-mail]>
> CC: Ian Jackson <[e-mail]>
> CC: Jan Beulich <[e-mail]>
> CC: Konrad Rzeszutek Wilk <[e-mail]>
> CC: Stefano Stabellini <[e-mail]>
> CC: Wei Liu <[e-mail]>
> CC: Julien Grall <[e-mail]>
> CC: Anthony PERARD <[e-mail]>
> CC: Paul Durrant <[e-mail]>
> 
> v2:
>  * Use sed rather than fixing up the makefile environment
> 
> This needs backporting to all trees with the support matrix logic, to unbreak
> docs rendering
> ---
>  docs/support-matrix-generate | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/docs/support-matrix-generate b/docs/support-matrix-generate
> index a3d93321f1..b759d0440c 100755
> --- a/docs/support-matrix-generate
> +++ b/docs/support-matrix-generate
> @@ -26,12 +26,9 @@
>  # SUPPORT.md into json.
>  #
>  # Then we try to find the next previous revision.  This is done by
> -# extracting the current version number from xen/Makefile.  (We make
> -# some slight assumption about how xen/Makefile's xenversion target
> -# works, because we want to be able to do this without checking out
> -# the whole tree for the version in question.)  Then we use git log on
> -# xen/Makefile to try to find a commit where the version changed.
> -# This gives us the previous version number, NN.
> +# extracting the current version number from xen/Makefile.  Then we
> +# use git log on xen/Makefile to try to find a commit where the
> +# version changed.  This gives us the previous version number, NN.
>  #
>  # That is substituted into the `refs/remotes/origin/stable-NN'
>  # argument to get the tip of the relevant branch.  That in turns
> @@ -102,12 +99,16 @@ commitish_version () {
>      esac
> 
>      git cat-file blob "$commitish:$versionfile" >"$tmp_versionfile"
> -    version=$(make --no-print-directory -C docs \
> -                   -f "${tmp_versionfile#docs/}" xenversion)
> -    case "$version" in
> -        *.*.*) version="${version%.*}" ;;
> -    esac
> -    printf "%s\n" "${version%%-*}"
> +
> +    local maj=$(sed -n 's/.*XEN_VERSION.*= \([0-9]\+\)/\1/p' < "$tmp_versionfile")
> +    local min=$(sed -n 's/.*XEN_SUBVERSION.*= \([0-9]\+\)/\1/p' < "$tmp_versionfile")
> +
> +    if [[ -z $maj || -z $min ]];
> +    then
> +        fail "Unable to identify Xen version for ${commitish}";
> +    fi
> +
> +    printf "%d.%d\n" "${maj}" "${min}"
>  }
> 
>  exec 4>"$tmp_revisions"
> --
> 2.11.0

Jan Beulich June 8, 2020, 1:41 p.m. UTC | #2

On 08.06.2020 15:34, Andrew Cooper wrote:
> The cronjob which renders https://xenbits.xen.org/docs/ has been broken for a
> while.  commitish_version() pulls an old version of xen/Makefile out of
> history, and uses the xenversion rule.
> 
> Currently, this fails with:
> 
>   tmp.support-matrix.xen.make:130: scripts/Kbuild.include: No such file or directory
> 
> which is because the Makefile legitimately references Kbuild.include with a
> relative rather than absolute path.
> 
> Rework support-matrix-generate to use sed to extract the major/minor, rather
> than expecting xen/Makefile to be usable in a different tree.
> 
> Signed-off-by: Andrew Cooper <[e-mail]>
> ---
> CC: George Dunlap <[e-mail]>
> CC: Ian Jackson <[e-mail]>
> CC: Jan Beulich <[e-mail]>
> CC: Konrad Rzeszutek Wilk <[e-mail]>
> CC: Stefano Stabellini <[e-mail]>
> CC: Wei Liu <[e-mail]>
> CC: Julien Grall <[e-mail]>
> CC: Anthony PERARD <[e-mail]>
> CC: Paul Durrant <[e-mail]>
> 
> v2:
>  * Use sed rather than fixing up the makefile environment
> 
> This needs backporting to all trees with the support matrix logic, to unbreak
> docs rendering

Did I understand the earlier discussion wrong then that the breakage
was connected to the introduction of xen/scripts/Kbuild.include?

Jan

Ian Jackson June 8, 2020, 1:48 p.m. UTC | #3

Andrew Cooper writes ("[PATCH v2 for-4.14] docs/support-matrix: unbreak docs rendering"):
> The cronjob which renders https://xenbits.xen.org/docs/ has been broken for a
> while.  commitish_version() pulls an old version of xen/Makefile out of
> history, and uses the xenversion rule.
> 
> Currently, this fails with:

Thanks for fixing this!

> +    local maj=$(sed -n 's/.*XEN_VERSION.*= \([0-9]\+\)/\1/p' < "$tmp_versionfile")
> +    local min=$(sed -n 's/.*XEN_SUBVERSION.*= \([0-9]\+\)/\1/p' < "$tmp_versionfile")

> +    if [[ -z $maj || -z $min ]];

I would prefer to avoid use of the unusual bash-specific [[ ]] syntax,
not because of concerns about portability to other shells (since this
is a #!/bin/bash script) but because many programmers won't be
familiar with it.

Would you mind writing this instead

  +    if test -z "$maj" || test -z "$min"; then

?

> +    printf "%d.%d\n" "${maj}" "${min}"

The { } here are not necessary but I don't kind if you feel they add
clarity.

You might also want to retain some text in the comment about what
assumptions we are making about xen/Makefile.  Different assumptions
to before, but assumptions nevertheless.

In any case, with or without the changes I suggest above:

Reviewed-by: Ian Jackson <[e-mail]>

On IRC:

14:35 <andyhhp__> Diziet: jbeulich: For the docs support-matrix
fix, any concerns with me backporting it immediately?

Having thought about this properly, I don't think we need to urgently
backport this.  In our own infrastructure, the one from staging (or
maybe it is master) is used.  The same script is used to parse all
older versions.

But I think it is a backport candidate.  Maybe add a Backport:
tag and my Backport-Requested-By ?

Thanks,
Ian.

Andrew Cooper June 8, 2020, 1:49 p.m. UTC | #4

On 08/06/2020 14:41, Jan Beulich wrote:
> On 08.06.2020 15:34, Andrew Cooper wrote:
>> The cronjob which renders https://xenbits.xen.org/docs/ has been broken for a
>> while.  commitish_version() pulls an old version of xen/Makefile out of
>> history, and uses the xenversion rule.
>>
>> Currently, this fails with:
>>
>>   tmp.support-matrix.xen.make:130: scripts/Kbuild.include: No such file or directory
>>
>> which is because the Makefile legitimately references Kbuild.include with a
>> relative rather than absolute path.
>>
>> Rework support-matrix-generate to use sed to extract the major/minor, rather
>> than expecting xen/Makefile to be usable in a different tree.
>>
>> Signed-off-by: Andrew Cooper <[e-mail]>
>> ---
>> CC: George Dunlap <[e-mail]>
>> CC: Ian Jackson <[e-mail]>
>> CC: Jan Beulich <[e-mail]>
>> CC: Konrad Rzeszutek Wilk <[e-mail]>
>> CC: Stefano Stabellini <[e-mail]>
>> CC: Wei Liu <[e-mail]>
>> CC: Julien Grall <[e-mail]>
>> CC: Anthony PERARD <[e-mail]>
>> CC: Paul Durrant <[e-mail]>
>>
>> v2:
>>  * Use sed rather than fixing up the makefile environment
>>
>> This needs backporting to all trees with the support matrix logic, to unbreak
>> docs rendering
> Did I understand the earlier discussion wrong then that the breakage
> was connected to the introduction of xen/scripts/Kbuild.include?

The xendocs cron email has been broken for an unknown period of time,
hence the issue going unnoticed.

The first of at least 3 breakages in the current docs rendering
manifests like this.

It may be that xen/scripts/Kbuild.include was the first thing to
actually break this script, but I haven't checked, and its not relevant
to the fragility of the old logic.

~Andrew

Andrew Cooper June 8, 2020, 2:11 p.m. UTC | #5

On 08/06/2020 14:48, Ian Jackson wrote:
> Andrew Cooper writes ("[PATCH v2 for-4.14] docs/support-matrix: unbreak docs rendering"):
>> The cronjob which renders https://xenbits.xen.org/docs/ has been broken for a
>> while.  commitish_version() pulls an old version of xen/Makefile out of
>> history, and uses the xenversion rule.
>>
>> Currently, this fails with:
> Thanks for fixing this!
>
>> +    local maj=$(sed -n 's/.*XEN_VERSION.*= \([0-9]\+\)/\1/p' < "$tmp_versionfile")
>> +    local min=$(sed -n 's/.*XEN_SUBVERSION.*= \([0-9]\+\)/\1/p' < "$tmp_versionfile")
>> +    if [[ -z $maj || -z $min ]];
> I would prefer to avoid use of the unusual bash-specific [[ ]] syntax,
> not because of concerns about portability to other shells (since this
> is a #!/bin/bash script) but because many programmers won't be
> familiar with it.
>
> Would you mind writing this instead
>
>   +    if test -z "$maj" || test -z "$min"; then
>
> ?

Single square brackets would be consistent with the rest of the script,
if you're happy with that?

>> +    printf "%d.%d\n" "${maj}" "${min}"
> The { } here are not necessary but I don't kind if you feel they add
> clarity.
>
> You might also want to retain some text in the comment about what
> assumptions we are making about xen/Makefile.  Different assumptions
> to before, but assumptions nevertheless.
>
> In any case, with or without the changes I suggest above:
>
> Reviewed-by: Ian Jackson <[e-mail]>
>
> On IRC:
>
> 14:35 <andyhhp__> Diziet: jbeulich: For the docs support-matrix
> fix, any concerns with me backporting it immediately?
>
> Having thought about this properly, I don't think we need to urgently
> backport this.  In our own infrastructure, the one from staging (or
> maybe it is master) is used.  The same script is used to parse all
> older versions.

Ah - I'd not spotted the logic to limit it to staging/master only.

In which case it won't block docs generation on older branches.

~Andrew

Patch

diff --git a/docs/support-matrix-generate b/docs/support-matrix-generate
index a3d93321f1..b759d0440c 100755
--- a/docs/support-matrix-generate
+++ b/docs/support-matrix-generate
@@ -26,12 +26,9 @@ 
 # SUPPORT.md into json.
 #
 # Then we try to find the next previous revision.  This is done by
-# extracting the current version number from xen/Makefile.  (We make
-# some slight assumption about how xen/Makefile's xenversion target
-# works, because we want to be able to do this without checking out
-# the whole tree for the version in question.)  Then we use git log on
-# xen/Makefile to try to find a commit where the version changed.
-# This gives us the previous version number, NN.
+# extracting the current version number from xen/Makefile.  Then we
+# use git log on xen/Makefile to try to find a commit where the
+# version changed.  This gives us the previous version number, NN.
 #
 # That is substituted into the `refs/remotes/origin/stable-NN'
 # argument to get the tip of the relevant branch.  That in turns
@@ -102,12 +99,16 @@  commitish_version () {
     esac
 
     git cat-file blob "$commitish:$versionfile" >"$tmp_versionfile"
-    version=$(make --no-print-directory -C docs \
-                   -f "${tmp_versionfile#docs/}" xenversion)
-    case "$version" in
-        *.*.*) version="${version%.*}" ;;
-    esac
-    printf "%s\n" "${version%%-*}"
+
+    local maj=$(sed -n 's/.*XEN_VERSION.*= \([0-9]\+\)/\1/p' < "$tmp_versionfile")
+    local min=$(sed -n 's/.*XEN_SUBVERSION.*= \([0-9]\+\)/\1/p' < "$tmp_versionfile")
+
+    if [[ -z $maj || -z $min ]];
+    then
+        fail "Unable to identify Xen version for ${commitish}";
+    fi
+
+    printf "%d.%d\n" "${maj}" "${min}"
 }
 
 exec 4>"$tmp_revisions"