| Message ID | 20240512125407.946412-1-mark@klomp.org |
|---|---|
| State | New |
| Headers | show |
| Series | [COMMITTED] Regenerate cygming.opt.urls and mingw.opt.urls | expand |
Thank you Mark and sorry for missing this during the reviews. Christophe Le dim. 12 mai 2024, 14:54, Mark Wielaard <mark@klomp.org> a écrit : > The new cygming.opt.urls and mingw.opt.urls in the > gcc/config/mingw/cygming.opt.urls directory need to generated by make > regenerate-opt-urls in the gcc subdirectory. They still contained > references to the gcc/config/i386 directory from which they were > copied. > > Fixes: 1f05dfc131c7 ("Reuse MinGW from i386 for AArch64") > Fixes: e8d003736e6c ("Rename "x86 Windows Options" to "Cygwin and MinGW > Options"") > > gcc/ChangeLog: > > * config/mingw/cygming.opt.urls: Regenerate. > * config/mingw/mingw.opt.urls: Likewise. > --- > gcc/config/mingw/cygming.opt.urls | 7 +++---- > gcc/config/mingw/mingw.opt.urls | 2 +- > 2 files changed, 4 insertions(+), 5 deletions(-) > > diff --git a/gcc/config/mingw/cygming.opt.urls > b/gcc/config/mingw/cygming.opt.urls > index c624e22e4427..af11c4997609 100644 > --- a/gcc/config/mingw/cygming.opt.urls > +++ b/gcc/config/mingw/cygming.opt.urls > @@ -1,4 +1,4 @@ > -; Autogenerated by regenerate-opt-urls.py from > gcc/config/i386/cygming.opt and generated HTML > +; Autogenerated by regenerate-opt-urls.py from > gcc/config/mingw/cygming.opt and generated HTML > > mconsole > UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mconsole) > @@ -9,9 +9,8 @@ UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mdll) > mnop-fun-dllimport > UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mnop-fun-dllimport) > > -; skipping UrlSuffix for 'mthreads' due to multiple URLs: > -; duplicate: 'gcc/Cygwin-and-MinGW-Options.html#index-mthreads-1' > -; duplicate: 'gcc/x86-Options.html#index-mthreads' > +mthreads > +UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mthreads-1) > > mwin32 > UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mwin32) > diff --git a/gcc/config/mingw/mingw.opt.urls > b/gcc/config/mingw/mingw.opt.urls > index f8ee5be6a535..40fb086606b2 100644 > --- a/gcc/config/mingw/mingw.opt.urls > +++ b/gcc/config/mingw/mingw.opt.urls > @@ -1,4 +1,4 @@ > -; Autogenerated by regenerate-opt-urls.py from gcc/config/i386/mingw.opt > and generated HTML > +; Autogenerated by regenerate-opt-urls.py from gcc/config/mingw/mingw.opt > and generated HTML > > mcrtdll= > UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mcrtdll) > -- > 2.39.3 > >
Sunday, May 12, 2024 Mark Wielaard <mark@klomp.org> wrote: > The new cygming.opt.urls and mingw.opt.urls in the > gcc/config/mingw/cygming.opt.urls directory need to generated by make > regenerate-opt-urls in the gcc subdirectory. They still contained references to > the gcc/config/i386 directory from which they were copied. > > Fixes: 1f05dfc131c7 ("Reuse MinGW from i386 for AArch64") > Fixes: e8d003736e6c ("Rename "x86 Windows Options" to "Cygwin and > MinGW Options"") > > gcc/ChangeLog: > > * config/mingw/cygming.opt.urls: Regenerate. > * config/mingw/mingw.opt.urls: Likewise. > --- Hello Mark, Thank you for reviewing our changes related to the refactoring of extracting the MinGW implementation from ix64. It was expected to move the MinGW-related files without changes in this commit ("Reuse MinGW from i386 for AArch64") and apply the renaming in a follow-up commit, which has been done in 'Rename "x86 Windows Options" to "Cygwin and MinGW Options"'. The script to update opt.urls files has been used. > gcc/config/mingw/cygming.opt.urls | 7 +++---- > gcc/config/mingw/mingw.opt.urls | 2 +- > 2 files changed, 4 insertions(+), 5 deletions(-) > > diff --git a/gcc/config/mingw/cygming.opt.urls > b/gcc/config/mingw/cygming.opt.urls > index c624e22e4427..af11c4997609 100644 > --- a/gcc/config/mingw/cygming.opt.urls > +++ b/gcc/config/mingw/cygming.opt.urls > @@ -1,4 +1,4 @@ > -; Autogenerated by regenerate-opt-urls.py from gcc/config/i386/cygming.opt > and generated HTML > +; Autogenerated by regenerate-opt-urls.py from > +gcc/config/mingw/cygming.opt and generated HTML I am not sure why this comment has not been updated. Is it critical or it could be updated next time when it is needed? > > mconsole > UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mconsole) > @@ -9,9 +9,8 @@ UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index- > mdll) > mnop-fun-dllimport > UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mnop-fun-dllimport) > > -; skipping UrlSuffix for 'mthreads' due to multiple URLs: > -; duplicate: 'gcc/Cygwin-and-MinGW-Options.html#index-mthreads-1' > -; duplicate: 'gcc/x86-Options.html#index-mthreads' > +mthreads > +UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mthreads-1) mthreads has the same issue before applying changes. Has something been changed recently? This is the change in patch series in 'Rename "x86 Windows Options" to "Cygwin and MinGW Options"' commit. ; skipping UrlSuffix for 'mthreads' due to multiple URLs: +; duplicate: 'gcc/Cygwin-and-MinGW-Options.html#index-mthreads-1' ; duplicate: 'gcc/x86-Options.html#index-mthreads' -; duplicate: 'gcc/x86-Windows-Options.html#index-mthreads-1' Regards, Evgeny > mwin32 > UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mwin32) > diff --git a/gcc/config/mingw/mingw.opt.urls > b/gcc/config/mingw/mingw.opt.urls index f8ee5be6a535..40fb086606b2 > 100644 > --- a/gcc/config/mingw/mingw.opt.urls > +++ b/gcc/config/mingw/mingw.opt.urls > @@ -1,4 +1,4 @@ > -; Autogenerated by regenerate-opt-urls.py from gcc/config/i386/mingw.opt > and generated HTML > +; Autogenerated by regenerate-opt-urls.py from > +gcc/config/mingw/mingw.opt and generated HTML > > mcrtdll= > UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mcrtdll) > -- > 2.39.3
Hi Evgeny, Adding David to the CC, who might know the details. On Mon, May 13, 2024 at 08:44:12AM +0000, Evgeny Karpov wrote: > Sunday, May 12, 2024 > > Thank you for reviewing our changes related to the refactoring of > extracting the MinGW implementation from ix64. > > It was expected to move the MinGW-related files without changes in > this commit ("Reuse MinGW from i386 for AArch64") and apply the > renaming in a follow-up commit, which has been done in 'Rename "x86 > Windows Options" to "Cygwin and MinGW Options"'. > > The script to update opt.urls files has been used. > > > diff --git a/gcc/config/mingw/cygming.opt.urls > > b/gcc/config/mingw/cygming.opt.urls > > index c624e22e4427..af11c4997609 100644 > > --- a/gcc/config/mingw/cygming.opt.urls > > +++ b/gcc/config/mingw/cygming.opt.urls > > @@ -1,4 +1,4 @@ > > > -; Autogenerated by regenerate-opt-urls.py from gcc/config/i386/cygming.opt > > and generated HTML > > +; Autogenerated by regenerate-opt-urls.py from > > +gcc/config/mingw/cygming.opt and generated HTML > > I am not sure why this comment has not been updated. Is it critical > or it could be updated next time when it is needed? Odd that the script didn't update this comment, it really should have. It might be that running the script through make regenerate-opt-urls inside the gcc build subdir invokes regenerate-opt-urls.py slightly differently so that this line is updated. > > mconsole > > UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mconsole) > > @@ -9,9 +9,8 @@ UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index- > > mdll) > > mnop-fun-dllimport > > UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mnop-fun-dllimport) > > > > -; skipping UrlSuffix for 'mthreads' due to multiple URLs: > > -; duplicate: 'gcc/Cygwin-and-MinGW-Options.html#index-mthreads-1' > > -; duplicate: 'gcc/x86-Options.html#index-mthreads' > > +mthreads > > +UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mthreads-1) > > mthreads has the same issue before applying changes. Has something been changed recently? > This is the change in patch series in 'Rename "x86 Windows Options" to "Cygwin and MinGW Options"' commit. > > ; skipping UrlSuffix for 'mthreads' due to multiple URLs: > +; duplicate: 'gcc/Cygwin-and-MinGW-Options.html#index-mthreads-1' > ; duplicate: 'gcc/x86-Options.html#index-mthreads' > -; duplicate: 'gcc/x86-Windows-Options.html#index-mthreads-1' Again, it might be caused by invoking the script by hand vs with make regenerate-opt-urls.py. I believe with the make option it will renumber the suffixes making sure the urls are unique. BTW. There is a CI buildbot that tries to regenerate all generated files, which is how I spotted this: https://builder.sourceware.org/buildbot/#/builders/gcc-autoregen (It should also sent email to the author of the patch on failure.) Cheers, Mark
On Mon, 2024-05-13 at 11:14 +0200, Mark Wielaard wrote: > Hi Evgeny, > > Adding David to the CC, who might know the details. > > On Mon, May 13, 2024 at 08:44:12AM +0000, Evgeny Karpov wrote: > > Sunday, May 12, 2024 > > > > Thank you for reviewing our changes related to the refactoring of > > extracting the MinGW implementation from ix64. > > > > It was expected to move the MinGW-related files without changes in > > this commit ("Reuse MinGW from i386 for AArch64") and apply the > > renaming in a follow-up commit, which has been done in 'Rename "x86 > > Windows Options" to "Cygwin and MinGW Options"'. > > > > The script to update opt.urls files has been used. > > > > > diff --git a/gcc/config/mingw/cygming.opt.urls > > > b/gcc/config/mingw/cygming.opt.urls > > > index c624e22e4427..af11c4997609 100644 > > > --- a/gcc/config/mingw/cygming.opt.urls > > > +++ b/gcc/config/mingw/cygming.opt.urls > > > @@ -1,4 +1,4 @@ > > > > > -; Autogenerated by regenerate-opt-urls.py from > > > gcc/config/i386/cygming.opt > > > and generated HTML > > > +; Autogenerated by regenerate-opt-urls.py from > > > +gcc/config/mingw/cygming.opt and generated HTML > > > > I am not sure why this comment has not been updated. Is it critical > > or it could be updated next time when it is needed? > > Odd that the script didn't update this comment, it really should > have. > It might be that running the script through make regenerate-opt-urls > inside the gcc build subdir invokes regenerate-opt-urls.py slightly > differently so that this line is updated. It might be a "make" dependencies issue: "make regenerate-opt-urls" has dependencies on OPT_URLS_HTML_DEPS which is currently defined as: OPT_URLS_HTML_DEPS = $(build_htmldir)/gcc/Option-Index.html \ $(build_htmldir)/gdc/Option-Index.html \ $(build_htmldir)/gfortran/Option-Index.html which might not be enough for the doc changes when moving things around that affect other generated html files. So when the CI runs "make regenerate-opt-urls" in a pristine build it will forcibly rerun texinfo to regenerate the docs first, whereas if you manually run the script in a build directory, you might not be seeing the latest version of the HTML (especially in thre presence of file moves). So I think the Makefile as currently written handles most cases, but can get it slightly wrong for the case you ran into here (sorry); fully refreshing the built docs ought to fix such cases. That's my theory of what happened here, anyway. Dave > > > > mconsole > > > UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mconsole) > > > @@ -9,9 +9,8 @@ UrlSuffix(gcc/Cygwin-and-MinGW- > > > Options.html#index- > > > mdll) > > > mnop-fun-dllimport > > > UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mnop-fun- > > > dllimport) > > > > > > -; skipping UrlSuffix for 'mthreads' due to multiple URLs: > > > -; duplicate: 'gcc/Cygwin-and-MinGW-Options.html#index- > > > mthreads-1' > > > -; duplicate: 'gcc/x86-Options.html#index-mthreads' > > > +mthreads > > > +UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mthreads-1) > > > > mthreads has the same issue before applying changes. Has something > > been changed recently? > > This is the change in patch series in 'Rename "x86 Windows Options" > > to "Cygwin and MinGW Options"' commit. > > > > ; skipping UrlSuffix for 'mthreads' due to multiple URLs: > > +; duplicate: 'gcc/Cygwin-and-MinGW-Options.html#index-mthreads- > > 1' > > ; duplicate: 'gcc/x86-Options.html#index-mthreads' > > -; duplicate: 'gcc/x86-Windows-Options.html#index-mthreads-1' > > Again, it might be caused by invoking the script by hand vs with make > regenerate-opt-urls.py. I believe with the make option it will > renumber the suffixes making sure the urls are unique. > > BTW. There is a CI buildbot that tries to regenerate all generated > files, which is how I spotted this: > https://builder.sourceware.org/buildbot/#/builders/gcc-autoregen > (It should also sent email to the author of the patch on failure.) > > Cheers, > > Mark >
On Mon, 2024-05-13 at 09:42 -0400, David Malcolm wrote: > On Mon, 2024-05-13 at 11:14 +0200, Mark Wielaard wrote: > > Hi Evgeny, > > > > Adding David to the CC, who might know the details. > > > > On Mon, May 13, 2024 at 08:44:12AM +0000, Evgeny Karpov wrote: > > > Sunday, May 12, 2024 > > > > > > Thank you for reviewing our changes related to the refactoring of > > > extracting the MinGW implementation from ix64. > > > > > > It was expected to move the MinGW-related files without changes > > > in > > > this commit ("Reuse MinGW from i386 for AArch64") and apply the > > > renaming in a follow-up commit, which has been done in 'Rename > > > "x86 > > > Windows Options" to "Cygwin and MinGW Options"'. > > > > > > The script to update opt.urls files has been used. > > > > > > > diff --git a/gcc/config/mingw/cygming.opt.urls > > > > b/gcc/config/mingw/cygming.opt.urls > > > > index c624e22e4427..af11c4997609 100644 > > > > --- a/gcc/config/mingw/cygming.opt.urls > > > > +++ b/gcc/config/mingw/cygming.opt.urls > > > > @@ -1,4 +1,4 @@ > > > > > > > -; Autogenerated by regenerate-opt-urls.py from > > > > gcc/config/i386/cygming.opt > > > > and generated HTML > > > > +; Autogenerated by regenerate-opt-urls.py from > > > > +gcc/config/mingw/cygming.opt and generated HTML > > > > > > I am not sure why this comment has not been updated. Is it > > > critical > > > or it could be updated next time when it is needed? > > > > Odd that the script didn't update this comment, it really should > > have. > > It might be that running the script through make regenerate-opt- > > urls > > inside the gcc build subdir invokes regenerate-opt-urls.py slightly > > differently so that this line is updated. > > It might be a "make" dependencies issue: > "make regenerate-opt-urls" has dependencies on OPT_URLS_HTML_DEPS > which > is currently defined as: > OPT_URLS_HTML_DEPS = $(build_htmldir)/gcc/Option-Index.html \ > $(build_htmldir)/gdc/Option-Index.html \ > $(build_htmldir)/gfortran/Option-Index.html > which might not be enough for the doc changes when moving things > around > that affect other generated html files. > > So when the CI runs "make regenerate-opt-urls" in a pristine build it > will forcibly rerun texinfo to regenerate the docs first, whereas if > you manually run the script in a build directory, you might not be > seeing the latest version of the HTML (especially in thre presence of > file moves). > > So I think the Makefile as currently written handles most cases, but > can get it slightly wrong for the case you ran into here (sorry); > fully > refreshing the built docs ought to fix such cases. Specifically, if you have some generated .html files in the $(build_htmldir) from a file that has gone away (due to a move), then I suspect these .html files stick around until you fully delete the $(build_htmldir), and in the meantime they get found by regenerate-opt- urls.py and lead to duplicate enries, leading to differences against a pristine build dir. Dave > > That's my theory of what happened here, anyway. > > Dave > > > > > > > mconsole > > > > UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mconsole) > > > > @@ -9,9 +9,8 @@ UrlSuffix(gcc/Cygwin-and-MinGW- > > > > Options.html#index- > > > > mdll) > > > > mnop-fun-dllimport > > > > UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mnop-fun- > > > > dllimport) > > > > > > > > -; skipping UrlSuffix for 'mthreads' due to multiple URLs: > > > > -; duplicate: 'gcc/Cygwin-and-MinGW-Options.html#index- > > > > mthreads-1' > > > > -; duplicate: 'gcc/x86-Options.html#index-mthreads' > > > > +mthreads > > > > +UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mthreads-1) > > > > > > mthreads has the same issue before applying changes. Has > > > something > > > been changed recently? > > > This is the change in patch series in 'Rename "x86 Windows > > > Options" > > > to "Cygwin and MinGW Options"' commit. > > > > > > ; skipping UrlSuffix for 'mthreads' due to multiple URLs: > > > +; duplicate: 'gcc/Cygwin-and-MinGW-Options.html#index- > > > mthreads- > > > 1' > > > ; duplicate: 'gcc/x86-Options.html#index-mthreads' > > > -; duplicate: 'gcc/x86-Windows-Options.html#index-mthreads-1' > > > > Again, it might be caused by invoking the script by hand vs with > > make > > regenerate-opt-urls.py. I believe with the make option it will > > renumber the suffixes making sure the urls are unique. > > > > BTW. There is a CI buildbot that tries to regenerate all generated > > files, which is how I spotted this: > > https://builder.sourceware.org/buildbot/#/builders/gcc-autoregen > > (It should also sent email to the author of the patch on failure.) > > > > Cheers, > > > > Mark > > >
Monday, May 13, 2024 3:49 PM wrote: David Malcolm <dmalcolm@redhat.com> wrote: > > > > It might be a "make" dependencies issue: > > "make regenerate-opt-urls" has dependencies on OPT_URLS_HTML_DEPS > > which is currently defined as: > > OPT_URLS_HTML_DEPS = $(build_htmldir)/gcc/Option-Index.html \ > > $(build_htmldir)/gdc/Option-Index.html \ > > $(build_htmldir)/gfortran/Option-Index.html > > which might not be enough for the doc changes when moving things > > around that affect other generated html files. > > > > So when the CI runs "make regenerate-opt-urls" in a pristine build it > > will forcibly rerun texinfo to regenerate the docs first, whereas if > > you manually run the script in a build directory, you might not be > > seeing the latest version of the HTML (especially in thre presence of > > file moves). > > > > So I think the Makefile as currently written handles most cases, but > > can get it slightly wrong for the case you ran into here (sorry); > > fully refreshing the built docs ought to fix such cases. > > Specifically, if you have some generated .html files in the > $(build_htmldir) from a file that has gone away (due to a move), then I suspect > these .html files stick around until you fully delete the $(build_htmldir), and in > the meantime they get found by regenerate-opt- urls.py and lead to duplicate > enries, leading to differences against a pristine build dir. > > Dave Thank you, Mark and Dave, for clarifying why the patch series encountered this issue! The relocation of opt.urls files and the use of "make regenerate-opt-urls" instead of running the script might explain why resolving duplicates for mthreads has not been triggered earlier. Regards, Evgeny
diff --git a/gcc/config/mingw/cygming.opt.urls b/gcc/config/mingw/cygming.opt.urls index c624e22e4427..af11c4997609 100644 --- a/gcc/config/mingw/cygming.opt.urls +++ b/gcc/config/mingw/cygming.opt.urls @@ -1,4 +1,4 @@ -; Autogenerated by regenerate-opt-urls.py from gcc/config/i386/cygming.opt and generated HTML +; Autogenerated by regenerate-opt-urls.py from gcc/config/mingw/cygming.opt and generated HTML mconsole UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mconsole) @@ -9,9 +9,8 @@ UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mdll) mnop-fun-dllimport UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mnop-fun-dllimport) -; skipping UrlSuffix for 'mthreads' due to multiple URLs: -; duplicate: 'gcc/Cygwin-and-MinGW-Options.html#index-mthreads-1' -; duplicate: 'gcc/x86-Options.html#index-mthreads' +mthreads +UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mthreads-1) mwin32 UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mwin32) diff --git a/gcc/config/mingw/mingw.opt.urls b/gcc/config/mingw/mingw.opt.urls index f8ee5be6a535..40fb086606b2 100644 --- a/gcc/config/mingw/mingw.opt.urls +++ b/gcc/config/mingw/mingw.opt.urls @@ -1,4 +1,4 @@ -; Autogenerated by regenerate-opt-urls.py from gcc/config/i386/mingw.opt and generated HTML +; Autogenerated by regenerate-opt-urls.py from gcc/config/mingw/mingw.opt and generated HTML mcrtdll= UrlSuffix(gcc/Cygwin-and-MinGW-Options.html#index-mcrtdll)
The new cygming.opt.urls and mingw.opt.urls in the gcc/config/mingw/cygming.opt.urls directory need to generated by make regenerate-opt-urls in the gcc subdirectory. They still contained references to the gcc/config/i386 directory from which they were copied. Fixes: 1f05dfc131c7 ("Reuse MinGW from i386 for AArch64") Fixes: e8d003736e6c ("Rename "x86 Windows Options" to "Cygwin and MinGW Options"") gcc/ChangeLog: * config/mingw/cygming.opt.urls: Regenerate. * config/mingw/mingw.opt.urls: Likewise. --- gcc/config/mingw/cygming.opt.urls | 7 +++---- gcc/config/mingw/mingw.opt.urls | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-)