Re: [PATCH] v2: DOCUMENTATION_ROOT_URL vs. release branches [PR114738]

[David Malcolm <dmalcolm@redhat.com>]

On Wed, 2024-04-24 at 11:07 +0200, Jakub Jelinek wrote:
> On Tue, Apr 23, 2024 at 07:07:17PM -0400, David Malcolm wrote:
> > That sounds like a better approach; thanks.
> 
> Attached below.  Tested by checking
> ../configure --disable-bootstrap --enable-languages=c --disable-
> multilib
> built trunk on
> void
> foo (int x)
> {
>   __builtin_printf ("%ld\n", x);
> }
> testcase and looking for the URL in there, then repeating that after
> changing gcc/BASE-VER to 14.1.0 and again after changing it to 14.1.1
> 
> > > Still, what do you think we should do on the release branches
> > > (recommend to
> > > developers and check with the post-commit CI)?
> > 
> > My hope is that the URL suffixes don't change: we shouldn't be
> > adding
> > new command-line options on the release branches, and I'd hope that
> > texinfo doesn't change the generated anchors from run to run.
> 
> Unfortunately that is not the case.  *.opt files change all the time
> on release branches.  Sure, not all the changes in there would cause
> *.urls changes, but many of them would.
> E.g. looking at 13 branch,
> r13-7022 r13-7130 r13-7169 r13-7276 r13-7336 r13-7415 r13-7518 r13-
> 7528
> r13-7650 r13-7651 r13-7728 r13-7794 r13-8039 r13-8040 r13-8223 r13-
> 8350
> r13-8351 r13-8357 r13-8419 r13-8545
> commits changed the *.opt files.

Thanks.

I looked through the commits you mentioned above.  For reference, the
following subset are the ones that added new options:

r13-7130 added -mamx-complex to i386
r13-7336 added -minline-atomics to riscv
r13-7415(*) added -Wextra and -Wmismatched-special-enum to D
r13-7518(*) added -fexceptions to D
r13-7528 added -fconstant-cfstrings to Darwin
r13-7650 added -Wuninit-variable-checking to Modula 2
r13-7651 added -Wuninit-variable-checking=all,known,cond to Modula 2
r13-7728 added -mgather and -mscatter to i386
r13-7794 added -Wcase-enum to Modula 2
r13-8039 added --param=uninit-max-chain-len= and --param=uninit-max-
num-chains=
r13-8350(*) added -mrelax to loongarch
r13-8351 added -mpass-mrelax-to-as to loongarch

Of the above, I've marked with a (*) those new options that share the
name with an existing option: -Wextra, -fexceptions in D (shared with C
family and Fortan), and -mrelax in loongarch (shared with various other
targets).  These are the ones that are likely to change URL suffixes,
due to texinfo's html generator using a counter internally to make
these be unique.

So yes, these changes are likely to invalidate a few existing URLs. 
Bother.

> 
> > > No regeneration of *.urls except before doing a new release
> > > candidate,
> > > or a different make goal that would grab html files from the web
> > > and
> > > regenerate against that?
> > 
> > That sounds overcomplicated. 
> > 
> > If the anchors do change, it's fairly trivial to run "make
> > regenerate-
> > opt-urls" locally, isn't it?
> 
> I think the primary question is, do we want the *.urls checking CI
> on the release branches as well or only on the trunk?
> Given the xz backdoor, I think checking release branches for the
> regeneration hiccups (primarily for configure, Makefile etc.) is
> desirable.

That's a fair point.

I suppose there's also the case where a downstream adds their own
command-line options (consider e.g. an out-of-tree frontend or target),
but they can just regenerate things themselves (or set up their own
CI).

> And with the patch posted here (or with what I've committed a week
> ago)
> on the release branches the default root will be initially identical
> but
> after some commits starts diverging.  If we can live with some URLs
> being
> stale or misplaced until next release in the default case (if people
> provide
> their own root it will be always accurate), we don't need to do
> anything
> further (except perhaps enable the autoregen CI on 14 branch).

The failure due to not regenerating the .opt.urls is that the final
"fragment" part of the URL (the anchor) will be slighly wrong, but the
page will be correct.  I think we can live with that.

The other possible failure is if the page is wrong: that the 
https://gcc.gnu.org/onlinedocs/gcc-MAJOR.MINOR.0/ doesn't exist. 
Presumably that is created on our server when the branch happens, so
there's only going to be a very short window between updating git and
the website being live.  If that's the case, then I think we can live
with that as well.

So I think this patch is good, and that we want the CI on the release
branches as well as on trunk.

Thanks
Dave



> 
> 2024-04-24  Jakub Jelinek  <jakub@redhat.com>
> 
>         PR other/114738
>         * opts.cc (get_option_url): Revert 2024-04-17 changes.
>         * gcc-urlifier.cc: Don't include diagnostic-core.h.
>         (gcc_urlifier::make_doc_url): Revert 2024-04-17 changes.
>         * configure.ac (documentation-root-url): On release branches
>         append gcc-MAJOR.MINOR.0/ to the default
> DOCUMENTATION_ROOT_URL.
>         * doc/install.texi (--with-documentation-root-url=): Document
>         the change of the default.
>         * configure: Regenerate.
> 
> --- gcc/opts.cc.jj      2024-04-17 16:17:19.537749951 +0200
> +++ gcc/opts.cc 2024-04-24 09:53:01.300399491 +0200
> @@ -3761,19 +3761,7 @@ get_option_url (const diagnostic_context
>      {
>        label_text url_suffix = get_option_url_suffix (option_index,
> lang_mask);
>        if (url_suffix.get ())
> -       {
> -         char infix[32];
> -         /* On release branches, append to DOCUMENTATION_ROOT_URL
> the
> -            subdirectory with documentation of the latest release
> made
> -            from the branch.  */
> -         if (BUILDING_GCC_MINOR != 0 && BUILDING_GCC_PATCHLEVEL <=
> 1U)
> -           sprintf (infix, "gcc-%u.%u.0/",
> -                    BUILDING_GCC_MAJOR, BUILDING_GCC_MINOR);
> -         else
> -           infix[0] = '\0';
> -         return concat (DOCUMENTATION_ROOT_URL, infix,
> url_suffix.get (),
> -                        nullptr);
> -       }
> +       return concat (DOCUMENTATION_ROOT_URL, url_suffix.get (),
> nullptr);
>      }
>  
>    return nullptr;
> --- gcc/gcc-urlifier.cc.jj      2024-04-17 16:17:19.538749937 +0200
> +++ gcc/gcc-urlifier.cc 2024-04-24 09:53:01.299399505 +0200
> @@ -26,7 +26,6 @@ along with GCC; see the file COPYING3.
>  #include "gcc-urlifier.h"
>  #include "opts.h"
>  #include "options.h"
> -#include "diagnostic-core.h"
>  #include "selftest.h"
>  
>  namespace {
> @@ -209,16 +208,7 @@ gcc_urlifier::make_doc_url (const char *
>    if (!doc_url_suffix)
>      return nullptr;
>  
> -  char infix[32];
> -  /* On release branches, append to DOCUMENTATION_ROOT_URL the
> -     subdirectory with documentation of the latest release made
> -     from the branch.  */
> -  if (BUILDING_GCC_MINOR != 0 && BUILDING_GCC_PATCHLEVEL <= 1U)
> -    sprintf (infix, "gcc-%u.%u.0/",
> -            BUILDING_GCC_MAJOR, BUILDING_GCC_MINOR);
> -  else
> -    infix[0] = '\0';
> -  return concat (DOCUMENTATION_ROOT_URL, infix, doc_url_suffix,
> nullptr);
> +  return concat (DOCUMENTATION_ROOT_URL, doc_url_suffix, nullptr);
>  }
>  
>  } // anonymous namespace
> --- gcc/configure.ac.jj 2024-04-17 16:09:49.697031449 +0200
> +++ gcc/configure.ac    2024-04-24 10:41:01.189687856 +0200
> @@ -1088,9 +1088,16 @@ AC_ARG_WITH(documentation-root-url,
>        no)  AC_MSG_ERROR([documentation root URL not specified]) ;;
>        */)  DOCUMENTATION_ROOT_URL="$withval" ;;
>        *)   AC_MSG_ERROR([documentation root URL does not end with
> /]) ;;
> -     esac],
> -     DOCUMENTATION_ROOT_URL="https://gcc.gnu.org/onlinedocs/"
> -)
> +     esac],[
> +     docroot_url_suffix=
> +changequote(,)dnl
> +     case "$gcc_version" in
> +       *.[123456].0) docroot_url_suffix="gcc-$gcc_version/";;
> +       *.[123456].1) docroot_url_suffix="gcc-`echo $gcc_version |
> sed 's/1$/0/'`/";;
> +     esac
> +changequote([,])dnl
> +    
> DOCUMENTATION_ROOT_URL="https://gcc.gnu.org/onlinedocs/$docroot_url_s
> uffix"
> +])
>  AC_DEFINE_UNQUOTED(DOCUMENTATION_ROOT_URL,"$DOCUMENTATION_ROOT_URL",
>         [Define to the root for documentation URLs.])
>  
> --- gcc/doc/install.texi.jj     2024-04-09 08:12:29.384449669 +0200
> +++ gcc/doc/install.texi        2024-04-24 10:03:54.856143097 +0200
> @@ -764,7 +764,9 @@ The default value refers to the FSF's GC
>  Specify the URL root that contains GCC option documentation.  The
> @var{url}
>  should end with a @code{/} character.
>  
> -The default value is
> @uref{https://gcc.gnu.org/onlinedocs/,,https://gcc.gnu.org/onlinedocs
> /}.
> +The default value is
> @uref{https://gcc.gnu.org/onlinedocs/,,https://gcc.gnu.org/onlinedocs
> /}
> +on the GCC main development trunk.  On release branches, the default
> +is
> @code{https://gcc.gnu.org/onlinedocs/gcc-@var{major}.@var{minor}.0/}.
>  
>  @item --with-changes-root-url=@var{url}
>  Specify the URL root that contains information about changes in GCC
> --- gcc/configure.jj    2024-02-23 18:54:37.914974922 +0100
> +++ gcc/configure       2024-04-24 10:41:07.667596458 +0200
> @@ -8232,7 +8232,13 @@ if test "${with_documentation_root_url+s
>        *)   as_fn_error $? "documentation root URL does not end with
> /" "$LINENO" 5 ;;
>       esac
>  else
> -  DOCUMENTATION_ROOT_URL="https://gcc.gnu.org/onlinedocs/"
> +
> +     docroot_url_suffix=
> +     case "$gcc_version" in
> +       *.[123456].0) docroot_url_suffix="gcc-$gcc_version/";;
> +       *.[123456].1) docroot_url_suffix="gcc-`echo $gcc_version |
> sed 's/1$/0/'`/";;
> +     esac
> +    
> DOCUMENTATION_ROOT_URL="https://gcc.gnu.org/onlinedocs/$docroot_url_s
> uffix"
>  
>  fi
>  
> @@ -21569,7 +21575,7 @@ else
>    lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
>    lt_status=$lt_dlunknown
>    cat > conftest.$ac_ext <<_LT_EOF
> -#line 21572 "configure"
> +#line 21578 "configure"
>  #include "confdefs.h"
>  
>  #if HAVE_DLFCN_H
> @@ -21675,7 +21681,7 @@ else
>    lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
>    lt_status=$lt_dlunknown
>    cat > conftest.$ac_ext <<_LT_EOF
> -#line 21678 "configure"
> +#line 21684 "configure"
>  #include "confdefs.h"
>  
>  #if HAVE_DLFCN_H
> 
> 
>         Jakub
>