public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] DOCUMENTATION_ROOT_URL vs. release branches [PR114738]
@ 2024-04-17 11:16 Jakub Jelinek
  2024-04-17 12:44 ` Richard Biener
  2024-04-23 15:40 ` David Malcolm
  0 siblings, 2 replies; 7+ messages in thread
From: Jakub Jelinek @ 2024-04-17 11:16 UTC (permalink / raw)
  To: David Malcolm, Mark Wielaard; +Cc: gcc-patches

Hi!

Starting with GCC 14 we have the nice URLification of the options printed
in diagnostics, say for in
test.c:4:23: warning: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘long int’ [-Wformat=]
the -Wformat= is underlined in some terminals and hovering on it shows
https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wformat
link.

This works nicely on the GCC trunk, where the online documentation is
regenerated every day from a cron job and more importantly, people rarely
use the trunk snapshots for too long, so it is unlikely that further changes
in the documentation will make too many links stale, because users will
simply regularly update to newer snapshots.

I think it doesn't work properly on release branches though.
Some users only use the relased versions (i.e. MAJOR.MINOR.0) from tarballs
but can use them for a couple of years, others use snapshots from the
release branches, but again they could be in use for months or years and
the above mentioned online docs which represent just the GCC trunk might
diverge significantly.

Now, for the relases we always publish also online docs for the release,
which unlike the trunk online docs will not change further, under
e.g.
https://gcc.gnu.org/onlinedocs/gcc-14.1.0/gcc/Warning-Options.html#index-Wformat
or
https://gcc.gnu.org/onlinedocs/gcc-14.2.0/gcc/Warning-Options.html#index-Wformat
etc.

So, I think at least for the MAJOR.MINOR.0 releases we want to use
URLs like above rather than the trunk ones and we can use the same process
of updating *.opt.urls as well for that.

For the snapshots from release branches, we don't have such docs.
One option (implemented in the patch below for the URL printing side) is
point to the MAJOR.MINOR.0 docs even for MAJOR.MINOR.1 snapshots.
Most of the links will work fine, for options newly added on the release
branches (rare thing but still happens) can have until the next release
no URLs for them and get them with the next point release.
The question is what to do about make regenerate-opt-urls for the release
branch snapshots.  Either just document that users shouldn't
make regenerate-opt-urls on release branches (and filter out *.opt.urls
changes from their commits), add make regenerate-opt-urls task be RM
responsibility before making first release candidate from a branch and
adjust the autoregen CI to know about that.  Or add a separate goal
which instead of relying on make html created files would download
copy of the html files from the last release from web (kind of web
mirroring the https://gcc.gnu.org/onlinedocs/gcc-14.1.0/ subtree locally)
and doing regenerate-opt-urls on top of that?  But how to catch the
point when first release candidate is made and we want to update to
what will be the URLs once the release is made (but will be stale URLs
for a week or so)?

Another option would be to add to cron daily regeneration of the online
docs for the release branches.  I don't think that is a good idea though,
because as I wrote earlier, not all users update to the latest snapshot
frequently, so there can be users that use gcc 13.1.1 20230525 for months
or years, and other users which use gcc 13.1.1 20230615 for years etc.

Another question is what is most sensible for users who want to override
the default root and use the --with-documentation-root-url= configure
option.  Do we expect them to grab the whole onlinedocs tree or for release
branches at least include gcc-14.1.0/ subdirectory under the root?
If so, the patch below deals with that.  Or should we just change the
default documentation root url, so if user doesn't specify
--with-documentation-root-url= and we are on a release branch, default that
to https://gcc.gnu.org/onlinedocs/gcc-14.1.0/ or
https://gcc.gnu.org/onlinedocs/gcc-14.2.0/ etc. and don't add any infix in
get_option_url/make_doc_url, but when people supply their own, let them
point to the root of the tree which contains the right docs?
Then such changes would go into gcc/configure.ac, some case based on
"$gcc_version", from that decide if it is a release branch or trunk.

2024-04-17  Jakub Jelinek  <jakub@redhat.com>

	PR other/114738
	* opts.cc (get_option_url): On release branches append
	gcc-MAJOR.MINOR.0/ after DOCUMENTATION_ROOT_URL.
	* gcc-urlifier.cc (gcc_urlifier::make_doc_url): Likewise.

--- gcc/opts.cc.jj	2024-01-05 08:35:13.600828652 +0100
+++ gcc/opts.cc	2024-04-17 12:03:10.961525141 +0200
@@ -3761,7 +3761,19 @@ get_option_url (const diagnostic_context
     {
       label_text url_suffix = get_option_url_suffix (option_index, lang_mask);
       if (url_suffix.get ())
-	return concat (DOCUMENTATION_ROOT_URL, url_suffix.get (), 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, url_suffix.get (),
+			 nullptr);
+	}
     }
 
   return nullptr;
--- gcc/gcc-urlifier.cc.jj	2024-01-10 17:15:31.851855587 +0100
+++ gcc/gcc-urlifier.cc	2024-04-17 12:14:47.902706021 +0200
@@ -26,6 +26,7 @@ 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 {
@@ -208,7 +209,16 @@ gcc_urlifier::make_doc_url (const char *
   if (!doc_url_suffix)
     return nullptr;
 
-  return concat (DOCUMENTATION_ROOT_URL, doc_url_suffix, 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);
 }
 
 } // anonymous namespace

	Jakub


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] DOCUMENTATION_ROOT_URL vs. release branches [PR114738]
  2024-04-17 11:16 [PATCH] DOCUMENTATION_ROOT_URL vs. release branches [PR114738] Jakub Jelinek
@ 2024-04-17 12:44 ` Richard Biener
  2024-04-23 15:40 ` David Malcolm
  1 sibling, 0 replies; 7+ messages in thread
From: Richard Biener @ 2024-04-17 12:44 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: David Malcolm, Mark Wielaard, gcc-patches

On Wed, Apr 17, 2024 at 1:17 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> Hi!
>
> Starting with GCC 14 we have the nice URLification of the options printed
> in diagnostics, say for in
> test.c:4:23: warning: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘long int’ [-Wformat=]
> the -Wformat= is underlined in some terminals and hovering on it shows
> https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wformat
> link.
>
> This works nicely on the GCC trunk, where the online documentation is
> regenerated every day from a cron job and more importantly, people rarely
> use the trunk snapshots for too long, so it is unlikely that further changes
> in the documentation will make too many links stale, because users will
> simply regularly update to newer snapshots.
>
> I think it doesn't work properly on release branches though.
> Some users only use the relased versions (i.e. MAJOR.MINOR.0) from tarballs
> but can use them for a couple of years, others use snapshots from the
> release branches, but again they could be in use for months or years and
> the above mentioned online docs which represent just the GCC trunk might
> diverge significantly.
>
> Now, for the relases we always publish also online docs for the release,
> which unlike the trunk online docs will not change further, under
> e.g.
> https://gcc.gnu.org/onlinedocs/gcc-14.1.0/gcc/Warning-Options.html#index-Wformat
> or
> https://gcc.gnu.org/onlinedocs/gcc-14.2.0/gcc/Warning-Options.html#index-Wformat
> etc.
>
> So, I think at least for the MAJOR.MINOR.0 releases we want to use
> URLs like above rather than the trunk ones and we can use the same process
> of updating *.opt.urls as well for that.
>
> For the snapshots from release branches, we don't have such docs.
> One option (implemented in the patch below for the URL printing side) is
> point to the MAJOR.MINOR.0 docs even for MAJOR.MINOR.1 snapshots.
> Most of the links will work fine, for options newly added on the release
> branches (rare thing but still happens) can have until the next release
> no URLs for them and get them with the next point release.
> The question is what to do about make regenerate-opt-urls for the release
> branch snapshots.  Either just document that users shouldn't
> make regenerate-opt-urls on release branches (and filter out *.opt.urls
> changes from their commits), add make regenerate-opt-urls task be RM
> responsibility before making first release candidate from a branch and
> adjust the autoregen CI to know about that.  Or add a separate goal
> which instead of relying on make html created files would download
> copy of the html files from the last release from web (kind of web
> mirroring the https://gcc.gnu.org/onlinedocs/gcc-14.1.0/ subtree locally)
> and doing regenerate-opt-urls on top of that?  But how to catch the
> point when first release candidate is made and we want to update to
> what will be the URLs once the release is made (but will be stale URLs
> for a week or so)?
>
> Another option would be to add to cron daily regeneration of the online
> docs for the release branches.  I don't think that is a good idea though,
> because as I wrote earlier, not all users update to the latest snapshot
> frequently, so there can be users that use gcc 13.1.1 20230525 for months
> or years, and other users which use gcc 13.1.1 20230615 for years etc.
>
> Another question is what is most sensible for users who want to override
> the default root and use the --with-documentation-root-url= configure
> option.  Do we expect them to grab the whole onlinedocs tree or for release
> branches at least include gcc-14.1.0/ subdirectory under the root?
> If so, the patch below deals with that.  Or should we just change the
> default documentation root url, so if user doesn't specify
> --with-documentation-root-url= and we are on a release branch, default that
> to https://gcc.gnu.org/onlinedocs/gcc-14.1.0/ or
> https://gcc.gnu.org/onlinedocs/gcc-14.2.0/ etc. and don't add any infix in
> get_option_url/make_doc_url, but when people supply their own, let them
> point to the root of the tree which contains the right docs?
> Then such changes would go into gcc/configure.ac, some case based on
> "$gcc_version", from that decide if it is a release branch or trunk.

I think this patch is sensible and an improvement over the current situation.
I guess we'll have to see how things evolve on the branch and adapt.

So, OK.

Thanks,
Richard.

> 2024-04-17  Jakub Jelinek  <jakub@redhat.com>
>
>         PR other/114738
>         * opts.cc (get_option_url): On release branches append
>         gcc-MAJOR.MINOR.0/ after DOCUMENTATION_ROOT_URL.
>         * gcc-urlifier.cc (gcc_urlifier::make_doc_url): Likewise.
>
> --- gcc/opts.cc.jj      2024-01-05 08:35:13.600828652 +0100
> +++ gcc/opts.cc 2024-04-17 12:03:10.961525141 +0200
> @@ -3761,7 +3761,19 @@ get_option_url (const diagnostic_context
>      {
>        label_text url_suffix = get_option_url_suffix (option_index, lang_mask);
>        if (url_suffix.get ())
> -       return concat (DOCUMENTATION_ROOT_URL, url_suffix.get (), 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, url_suffix.get (),
> +                        nullptr);
> +       }
>      }
>
>    return nullptr;
> --- gcc/gcc-urlifier.cc.jj      2024-01-10 17:15:31.851855587 +0100
> +++ gcc/gcc-urlifier.cc 2024-04-17 12:14:47.902706021 +0200
> @@ -26,6 +26,7 @@ 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 {
> @@ -208,7 +209,16 @@ gcc_urlifier::make_doc_url (const char *
>    if (!doc_url_suffix)
>      return nullptr;
>
> -  return concat (DOCUMENTATION_ROOT_URL, doc_url_suffix, 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);
>  }
>
>  } // anonymous namespace
>
>         Jakub
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] DOCUMENTATION_ROOT_URL vs. release branches [PR114738]
  2024-04-17 11:16 [PATCH] DOCUMENTATION_ROOT_URL vs. release branches [PR114738] Jakub Jelinek
  2024-04-17 12:44 ` Richard Biener
@ 2024-04-23 15:40 ` David Malcolm
  2024-04-23 15:45   ` Jakub Jelinek
  1 sibling, 1 reply; 7+ messages in thread
From: David Malcolm @ 2024-04-23 15:40 UTC (permalink / raw)
  To: Jakub Jelinek, Mark Wielaard; +Cc: gcc-patches

On Wed, 2024-04-17 at 13:16 +0200, Jakub Jelinek wrote:
> Hi!
> 
> Starting with GCC 14 we have the nice URLification of the options
> printed
> in diagnostics, say for in
> test.c:4:23: warning: format ‘%d’ expects argument of type ‘int’, but
> argument 2 has type ‘long int’ [-Wformat=]
> the -Wformat= is underlined in some terminals and hovering on it
> shows
> https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wformat
> link.

That isn't new in GCC 14; we've provided the URLs for the option
guarding a warning since GCC 10, I think.  What's new is that we now
"urlify" any quoted text that mentions an option, and that the option
URLs are now based on the anchors in the generated HTML (and thus more
likely to be correct).

> 
> This works nicely on the GCC trunk, where the online documentation is
> regenerated every day from a cron job and more importantly, people
> rarely
> use the trunk snapshots for too long, so it is unlikely that further
> changes
> in the documentation will make too many links stale, because users
> will
> simply regularly update to newer snapshots.
> 
> I think it doesn't work properly on release branches though.
> Some users only use the relased versions (i.e. MAJOR.MINOR.0) from
> tarballs
> but can use them for a couple of years, others use snapshots from the
> release branches, but again they could be in use for months or years
> and
> the above mentioned online docs which represent just the GCC trunk
> might
> diverge significantly.
> 
> Now, for the relases we always publish also online docs for the
> release,
> which unlike the trunk online docs will not change further, under
> e.g.
> https://gcc.gnu.org/onlinedocs/gcc-14.1.0/gcc/Warning-Options.html#index-Wformat
> or
> https://gcc.gnu.org/onlinedocs/gcc-14.2.0/gcc/Warning-Options.html#index-Wformat
> etc.


> 
> So, I think at least for the MAJOR.MINOR.0 releases we want to use
> URLs like above rather than the trunk ones and we can use the same
> process
> of updating *.opt.urls as well for that.

Would it make sense to instead update the default value in
gcc/configure.ac for DOCUMENTATION_ROOT_URL when branching or
releasing, from https://gcc.gnu.org/onlinedocs/ to
https://gcc.gnu.org/onlinedocs/gcc-MAJOR-MINOR.0/

?

Before this patch the DOCUMENTATION_ROOT_URL expresses the location of
a built texinfo html tree of docs, and the url suffixes express the
path within that tree.

As the patch is written, if a distributor overrides --with-
documentation-root-url= at configure time, then they need to mirror the
structure of our website on their website, which seems like a burden.



> 
> For the snapshots from release branches, we don't have such docs.
> One option (implemented in the patch below for the URL printing side)
> is
> point to the MAJOR.MINOR.0 docs even for MAJOR.MINOR.1 snapshots.
> Most of the links will work fine, for options newly added on the
> release
> branches (rare thing but still happens) can have until the next
> release
> no URLs for them and get them with the next point release.
> The question is what to do about make regenerate-opt-urls for the
> release
> branch snapshots.  Either just document that users shouldn't
> make regenerate-opt-urls on release branches (and filter out
> *.opt.urls
> changes from their commits), add make regenerate-opt-urls task be RM
> responsibility before making first release candidate from a branch
> and
> adjust the autoregen CI to know about that.  Or add a separate goal
> which instead of relying on make html created files would download
> copy of the html files from the last release from web (kind of web
> mirroring the https://gcc.gnu.org/onlinedocs/gcc-14.1.0/ subtree
> locally)
> and doing regenerate-opt-urls on top of that?  But how to catch the
> point when first release candidate is made and we want to update to
> what will be the URLs once the release is made (but will be stale
> URLs
> for a week or so)?
> 
> Another option would be to add to cron daily regeneration of the
> online
> docs for the release branches.  I don't think that is a good idea
> though,
> because as I wrote earlier, not all users update to the latest
> snapshot
> frequently, so there can be users that use gcc 13.1.1 20230525 for
> months
> or years, and other users which use gcc 13.1.1 20230615 for years
> etc.
> 
> Another question is what is most sensible for users who want to
> override
> the default root and use the --with-documentation-root-url= configure
> option.  Do we expect them to grab the whole onlinedocs tree or for
> release
> branches at least include gcc-14.1.0/ subdirectory under the root?
> If so, the patch below deals with that.  Or should we just change the
> default documentation root url, so if user doesn't specify
> --with-documentation-root-url= and we are on a release branch,
> default that
> to https://gcc.gnu.org/onlinedocs/gcc-14.1.0/ or
> https://gcc.gnu.org/onlinedocs/gcc-14.2.0/ etc. and don't add any
> infix in
> get_option_url/make_doc_url, but when people supply their own, let
> them
> point to the root of the tree which contains the right docs?
> Then such changes would go into gcc/configure.ac, some case based on
> "$gcc_version", from that decide if it is a release branch or trunk.

I think changing the default for DOCUMENTATION_ROOT_URL makes much more
sense.

Dave

> 
> 2024-04-17  Jakub Jelinek  <jakub@redhat.com>
> 
>         PR other/114738
>         * opts.cc (get_option_url): On release branches append
>         gcc-MAJOR.MINOR.0/ after DOCUMENTATION_ROOT_URL.
>         * gcc-urlifier.cc (gcc_urlifier::make_doc_url): Likewise.
> 
> --- gcc/opts.cc.jj      2024-01-05 08:35:13.600828652 +0100
> +++ gcc/opts.cc 2024-04-17 12:03:10.961525141 +0200
> @@ -3761,7 +3761,19 @@ get_option_url (const diagnostic_context
>      {
>        label_text url_suffix = get_option_url_suffix (option_index,
> lang_mask);
>        if (url_suffix.get ())
> -       return concat (DOCUMENTATION_ROOT_URL, url_suffix.get (),
> 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,
> url_suffix.get (),
> +                        nullptr);
> +       }
>      }
>  
>    return nullptr;
> --- gcc/gcc-urlifier.cc.jj      2024-01-10 17:15:31.851855587 +0100
> +++ gcc/gcc-urlifier.cc 2024-04-17 12:14:47.902706021 +0200
> @@ -26,6 +26,7 @@ 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 {
> @@ -208,7 +209,16 @@ gcc_urlifier::make_doc_url (const char *
>    if (!doc_url_suffix)
>      return nullptr;
>  
> -  return concat (DOCUMENTATION_ROOT_URL, doc_url_suffix, 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);
>  }
>  
>  } // anonymous namespace
> 
>         Jakub
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] DOCUMENTATION_ROOT_URL vs. release branches [PR114738]
  2024-04-23 15:40 ` David Malcolm
@ 2024-04-23 15:45   ` Jakub Jelinek
  2024-04-23 23:07     ` David Malcolm
  0 siblings, 1 reply; 7+ messages in thread
From: Jakub Jelinek @ 2024-04-23 15:45 UTC (permalink / raw)
  To: David Malcolm; +Cc: Mark Wielaard, gcc-patches

On Tue, Apr 23, 2024 at 11:40:55AM -0400, David Malcolm wrote:
> > So, I think at least for the MAJOR.MINOR.0 releases we want to use
> > URLs like above rather than the trunk ones and we can use the same
> > process
> > of updating *.opt.urls as well for that.
> 
> Would it make sense to instead update the default value in
> gcc/configure.ac for DOCUMENTATION_ROOT_URL when branching or
> releasing, from https://gcc.gnu.org/onlinedocs/ to
> https://gcc.gnu.org/onlinedocs/gcc-MAJOR-MINOR.0/
> 
> ?
> 
> Before this patch the DOCUMENTATION_ROOT_URL expresses the location of
> a built texinfo html tree of docs, and the url suffixes express the
> path within that tree.
> 
> As the patch is written, if a distributor overrides --with-
> documentation-root-url= at configure time, then they need to mirror the
> structure of our website on their website, which seems like a burden.

Sure, that is doable (of course, it shouldn't be done by updating
gcc/configure.ac but by adjusting the default in there based on gcc_version,
I'll post a patch tomorrow).

Still, what do you think we should do on the release branches (recommend to
developers and check with the post-commit CI)?
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?

	Jakub


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] DOCUMENTATION_ROOT_URL vs. release branches [PR114738]
  2024-04-23 15:45   ` Jakub Jelinek
@ 2024-04-23 23:07     ` David Malcolm
  2024-04-24  9:07       ` [PATCH] v2: " Jakub Jelinek
  0 siblings, 1 reply; 7+ messages in thread
From: David Malcolm @ 2024-04-23 23:07 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Mark Wielaard, gcc-patches

On Tue, 2024-04-23 at 17:45 +0200, Jakub Jelinek wrote:
> On Tue, Apr 23, 2024 at 11:40:55AM -0400, David Malcolm wrote:
> > > So, I think at least for the MAJOR.MINOR.0 releases we want to
> > > use
> > > URLs like above rather than the trunk ones and we can use the
> > > same
> > > process
> > > of updating *.opt.urls as well for that.
> > 
> > Would it make sense to instead update the default value in
> > gcc/configure.ac for DOCUMENTATION_ROOT_URL when branching or
> > releasing, from https://gcc.gnu.org/onlinedocs/ to
> > https://gcc.gnu.org/onlinedocs/gcc-MAJOR-MINOR.0/
> > 
> > ?
> > 
> > Before this patch the DOCUMENTATION_ROOT_URL expresses the location
> > of
> > a built texinfo html tree of docs, and the url suffixes express the
> > path within that tree.
> > 
> > As the patch is written, if a distributor overrides --with-
> > documentation-root-url= at configure time, then they need to mirror
> > the
> > structure of our website on their website, which seems like a
> > burden.
> 
> Sure, that is doable (of course, it shouldn't be done by updating
> gcc/configure.ac but by adjusting the default in there based on
> gcc_version,
> I'll post a patch tomorrow).

That sounds like a better approach; thanks.

> 
> 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.

> 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?

As mentioned above, I like the idea of having the
DOCUMENTATION_ROOT_URL express the location of a tree of docs built
with texinfo, and for the url suffixes to be relative to that.  We can
update the default in gcc/configure.ac for released branches, and drop
the logic from your previous patch.  So if a distributor wants to
upload their docs for a particular version to their own location,
they're responsible for providing a suitable value for  --with-
documentation-root-url= at configure time.

Or am I missing something here?

Dave


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] v2: DOCUMENTATION_ROOT_URL vs. release branches [PR114738]
  2024-04-23 23:07     ` David Malcolm
@ 2024-04-24  9:07       ` Jakub Jelinek
  2024-04-24 13:39         ` David Malcolm
  0 siblings, 1 reply; 7+ messages in thread
From: Jakub Jelinek @ 2024-04-24  9:07 UTC (permalink / raw)
  To: David Malcolm; +Cc: Mark Wielaard, gcc-patches

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.

> > 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.
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).

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_suffix"
+])
 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_suffix"
 
 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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] v2: DOCUMENTATION_ROOT_URL vs. release branches [PR114738]
  2024-04-24  9:07       ` [PATCH] v2: " Jakub Jelinek
@ 2024-04-24 13:39         ` David Malcolm
  0 siblings, 0 replies; 7+ messages in thread
From: David Malcolm @ 2024-04-24 13:39 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Mark Wielaard, gcc-patches

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
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-04-24 13:39 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-17 11:16 [PATCH] DOCUMENTATION_ROOT_URL vs. release branches [PR114738] Jakub Jelinek
2024-04-17 12:44 ` Richard Biener
2024-04-23 15:40 ` David Malcolm
2024-04-23 15:45   ` Jakub Jelinek
2024-04-23 23:07     ` David Malcolm
2024-04-24  9:07       ` [PATCH] v2: " Jakub Jelinek
2024-04-24 13:39         ` David Malcolm

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).