On Fri, 2023-04-14 at 06:20 +0000, Shinde, Yash wrote:
> As discussed in the meeting, sharing the GCC upstream’s reply for the
> “Share work directories” patch.

Thanks for sharing this. With a few of these patches we need to
understand the history of them and why we're doing what we do.

Since we share ${S}, the source directory between multiple gcc builds
and multiple target builds, the configuration can vary but we can't
change files in ${S} as that would cause races. I believe we therefore
take a copy of defaults.h into ${B} and that is a copy we can modify in
different builds.

grepping the gcc recipes for defaults.h, we find this:

gcc-configure-common.inc-do_configure:prepend () {
gcc-configure-common.inc-       # teach gcc to find correct target includedir 
when checking libc ssp support
gcc-configure-common.inc-       mkdir -p ${B}/gcc
gcc-configure-common.inc-       echo "NATIVE_SYSTEM_HEADER_DIR = 
${SYSTEMHEADERS}" > ${B}/gcc/t-oe
gcc-configure-common.inc:       cat ${S}/gcc/defaults.h | grep -v 
"\#endif.*GCC_DEFAULTS_H" > ${B}/gcc/defaults.h.new
gcc-configure-common.inc:       cat >>${B}/gcc/defaults.h.new <<_EOF
gcc-configure-common.inc-#define NATIVE_SYSTEM_HEADER_DIR "${SYSTEMHEADERS}"
gcc-configure-common.inc-#define STANDARD_STARTFILE_PREFIX_1 "${SYSTEMLIBS}"
gcc-configure-common.inc-#define STANDARD_STARTFILE_PREFIX_2 "${SYSTEMLIBS1}"
gcc-configure-common.inc-#define SYSTEMLIBS_DIR "${SYSTEMLIBS}"
gcc-configure-common.inc-#endif /* ! GCC_DEFAULTS_H */
gcc-configure-common.inc-_EOF
gcc-configure-common.inc:       mv ${B}/gcc/defaults.h.new ${B}/gcc/defaults.h
gcc-configure-common.inc-}

which is the code taking the copy. The patch you mention then causes
gcc to use the copy rather than the one in ${S}.

The patch as it stands will break gcc since there is no file copy in
upstream so it is not surprisingly upstream wouldn't take it. Proposing
a change like this which breaks things for them will undermine our
relationship with them too, so we need to be careful with this.

The question then becomes can we override the values we want to set
here without changing defaults.h? I think we struggled to find a way to
change these defines so that gcc always used the new values which is
why we have the patch.

Our goal is to stop patching gcc with as many patches. The question is
therefore, can we find a way to keep this working without a patch.

At the very least we should update the patch with the information I've
collected above.

Cheers,

Richard

>  
> Regards,
> Yash
>  
> From: Andrew Pinski
> Sent: 22 February 2023 21:34
> To: Shinde, Yash
> Cc: [email protected]; [email protected]; MacLeod,
> Randy;Kokkonda, Sundeep
> Subject: Re: [PATCH] Share work directories
>  
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the
> sender and know the content is safe.
> 
> On Wed, Feb 22, 2023 at 4:22 AM Yash Shinde
> <[email protected]> wrote:
> > 
> > From: Khem Raj <[email protected]>
> > 
> > Fix configure and Makefile files to read the defaults.hand t-oe
> > from build directory,
> > so that the source can be shared between gcc-cross-initial, gcc-
> > cross-intermediate, gcc-cross, gcc-runtime,
> > and also the sdk build which use a separate build directory
> > compared to source directory.
> 
> > 
> > While compiling gcc-crosssdk-initial-x86_64 on some host, there is
> > occasionally failure that test the existance of default.h doesn't
> > work,
> > the reason is tm_include_list='** defaults.h' rather than
> > tm_include_list='** ./defaults.h'.
> > So we add the test condition for this situation.
> 
> This patch does not make sense because $(srcdir)/defaults.h will
> always exist.
> I build all the time with different object directories and I know of
> many people who build with a read only source directory; explicitly
> so
> they can test that way.
> You are going to have to expand on why you need defaults.h from the
> build directory and not the source directory?
> As far as I Know there is no defaults.h in the build directory even.
> Do you have another patch which changes that?
> 
> 
> Thanks,
> Andrew Pinski
> 
> > 
> > gcc/ChangeLog:
> > 
> >          * configure
> >          * configure.ac
> >          * mkconfig.sh
> > 
> > Signed-off-by: Khem Raj <[email protected]>
> > Signed-off-by: Hongxu Jia <[email protected]>
> > ---
> >   gcc/configure    | 4 ++--
> >   gcc/configure.ac | 4 ++--
> >   gcc/mkconfig.sh  | 4 ++--
> >   3 files changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/gcc/configure b/gcc/configure
> > index 254f9b6c943..ff2a3e26049 100755
> > --- a/gcc/configure
> > +++ b/gcc/configure
> > @@ -13471,8 +13471,8 @@ for f in $tm_file; do
> >          tm_include_list="${tm_include_list} $f"
> >          ;;
> >       defaults.h )
> > -       tm_file_list="${tm_file_list} \$(srcdir)/$f"
> > -       tm_include_list="${tm_include_list} $f"
> > +       tm_file_list="${tm_file_list} ./$f"
> > +       tm_include_list="${tm_include_list} ./$f"
> >          ;;
> >       * )
> >          tm_file_list="${tm_file_list} \$(srcdir)/config/$f"
> > diff --git a/gcc/configure.ac b/gcc/configure.ac
> > index 62bc908b991..d36830cf2fb 100644
> > --- a/gcc/configure.ac
> > +++ b/gcc/configure.ac
> > @@ -2336,8 +2336,8 @@ for f in $tm_file; do
> >          tm_include_list="${tm_include_list} $f"
> >          ;;
> >       defaults.h )
> > -       tm_file_list="${tm_file_list} \$(srcdir)/$f"
> > -       tm_include_list="${tm_include_list} $f"
> > +       tm_file_list="${tm_file_list} ./$f"
> > +       tm_include_list="${tm_include_list} ./$f"
> >          ;;
> >       * )
> >          tm_file_list="${tm_file_list} \$(srcdir)/config/$f"
> > diff --git a/gcc/mkconfig.sh b/gcc/mkconfig.sh
> > index 054ede89647..3b2c2b9df37 100644
> > --- a/gcc/mkconfig.sh
> > +++ b/gcc/mkconfig.sh
> > @@ -77,7 +77,7 @@ if [ -n "$HEADERS" ]; then
> >       if [ $# -ge 1 ]; then
> >          echo '#ifdef IN_GCC' >> ${output}T
> >          for file in "$@"; do
> > -           if test x"$file" = x"defaults.h"; then
> > +           if test x"$file" = x"./defaults.h" -o x"$file" =
> > x"defaults.h"; then
> >                  postpone_defaults_h="yes"
> >              else
> >                  echo "# include \"$file\"" >> ${output}T
> > @@ -106,7 +106,7 @@ esac
> > 
> >   # If we postponed including defaults.h, add the #include now.
> >   if test x"$postpone_defaults_h" = x"yes"; then
> > -    echo "# include \"defaults.h\"" >> ${output}T
> > +    echo "# include \"./defaults.h\"" >> ${output}T
> >   fi
> > 
> >   # Add multiple inclusion protection guard, part two.
> > --
> > 2.39.0
> > 
>  

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#179990): 
https://lists.openembedded.org/g/openembedded-core/message/179990
Mute This Topic: https://lists.openembedded.org/mt/98257029/21656
Group Owner: [email protected]
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to