Re: [PATCH] libstdc++: testsuite: test symlnks ifdef _GLIBCXX_HAVE_SYMLINK

[Jonathan Wakely <jwakely@redhat.com>]

On Thu, 23 Jun 2022 at 12:28, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> On Thu, 23 Jun 2022 at 12:02, Alexandre Oliva <oliva@adacore.com> wrote:
> >
> > On Jun 22, 2022, Jonathan Wakely <jwakely@redhat.com> wrote:
> >
> > > OK.
> >
> > > I'd like to clean this up so the tests don't rely on the "internal"
> > > HAVE_SYMLINK macro. We could add something like this to
> > > testsuite/util/testsuite_fs.h
> >
> > > #if defined(__MINGW32__) || defined(__MINGW64__) \
> > >   || !defined (_GLIBCXX_HAVE_SYMLINK)
> > > # define NO_SYMLINKS
> > > #endif
> >
> > I took your suggestion, and took it a little further.  Here's the
> > result.
>
> Nice, thanks!
>
> >
> > Regstrapped on x86_64-linux-gnu, also tested with a cross to
> > aarch64-rtems6.  Ok to install?
>
> All the changes to the actual tests are fine.
>
> > diff --git a/libstdc++-v3/testsuite/lib/libstdc++.exp b/libstdc++-v3/testsuite/lib/libstdc++.exp
> > index 93fdfee687ddb..d71d88fb99837 100644
> > --- a/libstdc++-v3/testsuite/lib/libstdc++.exp
> > +++ b/libstdc++-v3/testsuite/lib/libstdc++.exp
> > @@ -974,13 +974,16 @@ proc v3_try_preprocess { name code flags } {
> >  }
> >
> >  # Return 1 if COND evaluates to true in the preprocessor, 0 otherwise.
> > -# The <bits/c++config.h> config header is included.
> > -proc v3_check_preprocessor_condition { name cond } {
> > +# The <bits/c++config.h> config header is included, and INC, if given,
> > +# is pasted between it and the condition evaluation, so it can be used
> > +# for additional #include's.
> > +proc v3_check_preprocessor_condition { name cond inc } {
>
> Could this new arg be given a default value, so every caller doesn't
> have to pass "" to it?
>
> proc v3_check_preprocessor_condition { name cond {inc ""} } {
>
> > diff --git a/libstdc++-v3/testsuite/util/testsuite_fs.h b/libstdc++-v3/testsuite/util/testsuite_fs.h
> > index 9358a04e56c1f..048f03103e436 100644
> > --- a/libstdc++-v3/testsuite/util/testsuite_fs.h
> > +++ b/libstdc++-v3/testsuite/util/testsuite_fs.h
> > @@ -42,6 +42,11 @@ namespace test_fs = std::experimental::filesystem;
> >  #include <random>   // std::random_device
> >  #endif
> >
> > +#if defined(__MINGW32__) || defined(__MINGW64__) \
> > +  || !defined (_GLIBCXX_HAVE_SYMLINK)
> > +#define NO_SYMLINKS
> > +#endif
>
> I think this could be simplified to just
> #ifndef _GLIBCXX_HAVE_SYMLINK
> because that is false on mingw:
> $ grep HAVE_SYMLINK include/x86_64-w64-mingw32/bits/c++config.h
> /* #undef _GLIBCXX_HAVE_SYMLINK */
>
> Windows uses CreateSymbolicLinkA to create its symlinks.
>
> And if NO_SYMLINKS just becomes a "public" name for
> _GLIBCXX_HAVE_SYMLINK that we can use in tests, then the dg
> check_v3_target_fs_symlinks proc could just test that macro too,
> without needing to include <testsuite_fs.h> to test NO_SYMLINKS. I
> don't feel strongly about that though. The $inc addition to
> v3_check_preprocessor_condition seems useful (although it will slow
> down evaluation of that condition, because <testsuite_fs.h> includes a
> lot of other headers).
> And if the definition of NO_SYMLINKS gets more complicated in future
> (e.g. we add something like && !defined __foo__) then using
> NO_SYMLINKS in the dg proc will keep them in sync.

Oh, and we need to be able to include <testsuite_fs.h> for the similar
NO_SPACE macro in your next patch, so we might as well check
NO_SYMLINKS the same way.

> I'd like the default argument for v3_check_preprocessor_condition so
> we don't need to add "" everywhere in that file, but the rest of the
> patch is OK as-is or simplified as above, I don't mind. Even keeping
> the MINGW tests in the NO_SYMLINKS definition is OK (it certainly
> doesn't hurt, it's just a bit redundant).