[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: rebootstrap and g++



Hello,

Helmut Grohne, le jeu. 01 août 2024 18:18:29 +0200, a ecrit:
> Do you mind if we make this mail discussion public e.g. at
> debian-cross@lists.debian.org?

Ah, sure! Done so.

> On Wed, Jul 31, 2024 at 07:55:47PM +0200, Samuel Thibault wrote:
> > It's looking at usr/include/c++/14 instead of /usr/include/c++/14, while
> > --with-gxx-include-dir=/usr/include/c++/14 is properly passed.
> 
> I independently arrived at the same conclusion until this point earlier.
> 
> Please allow me to give some more context here. Until gcc-13,
> rebootstrap used the patches from cross-gcc-dev at
> https://sources.debian.org/src/cross-gcc/249/patches/gcc-13/ and then
> Matthias merged my gcc-for-host patches into gcc-14 and later gcc-13.
> That's when these patches had severe hunk failures. Rather than fix the
> patches, I attempted rewriting them from scratch and using them as
> inspiration. In that process, I dropped all code that I didn't
> understand and that didn't appear to make any difference (possibly too
> much). The result of that is what you find in rebootstrap.git.
> 
> As far as I remember, this vaguely worked, but I am no longer sure
> whether my refactoring introduced the issue we are seeing here or
> whether that was a later failure as linux-libc-dev concurrently became
> Architecture: all and Multi-Arch: foreign and that significantly broke
> rebootstrap. When problems emerged, Matthias and Bastian disagreed hard
> enough that they took it to CTTE and that left rebootstrap broken for an
> extensive period of time. That dispute remains unsolved.
> 
> The gist of this is that everything you see in patch_gcc_wdotap is
> barely tested only and I expect bugs there.

Ok :)

> > Apparently this is coming from gcc/configure:
> > 
> > # If both --with-sysroot and --with-gxx-include-dir are passed, we interpolate
> > # the former in the latter and, upon success, compute gcc_gxx_include_dir as
> > # relative to the sysroot.
> > 
> > and gcc_gxx_include_dir_add_sysroot is getting set to 1 to record
> > that sysroot should be prepended, but I guess it's not properly done
> > everywhere. As a workaround, I forced passing an empty sysroot:
> > 
> > diff --git a/bootstrap.sh b/bootstrap.sh
> > index a8e6038..f8330a6 100755
> > --- a/bootstrap.sh
> > +++ b/bootstrap.sh
> > @@ -2131,14 +2213,14 @@ patch_gcc_wdotap() {
> >       DEB_CROSS = yes
> >       build_type = build-cross
> >  +    ifeq ($(with_deps_on_target_arch_pkgs),yes)
> > -+      with_sysroot = /
> > ++      with_sysroot = 
> >  +    endif
> >     else ifeq ($(FORCE_CROSS_LAYOUT),yes)
> >       # a native build with a cross layout
> >       DEB_CROSS = yes
> >       build_type = build-cross
> >  +    ifeq ($(with_deps_on_target_arch_pkgs),yes)
> > -+      with_sysroot = /
> > ++      with_sysroot = 
> >  +    endif
> >     else
> >       # native build
> > @@ -2207,6 +2289,27 @@ patch_gcc_wdotap() {
> >   debian_patches += hurd-multiarch
> >  --- a/debian/rules2
> >  +++ b/debian/rules2
> > +@@ -250,7 +250,7 @@ ifneq (,$(filter $(DEB_STAGE),stage1 stage2))
> > +       --libdir=/$(PF)/$(configured_libdir) \
> > +       $(if $(with_build_sysroot),--with-build-sysroot=$(with_build_sysroot)) \
> > +       $(if $(findstring build-cross, $(build_type)), \
> > +-              $(if $(with_sysroot),--with-sysroot=$(with_sysroot))) \
> > ++              --with-sysroot=$(with_sysroot)) \
> > +       --enable-linker-build-id
> > + 
> > +   ifeq ($(with_multiarch_lib),yes)
> > +@@ -297,9 +297,9 @@ else ifneq ($(with_bootstrap),)
> > +   CONFARGS += --enable-bootstrap
> > + endif
> > + 
> > +-ifneq ($(with_sysroot),)
> > ++#ifneq ($(with_sysroot),)
> > +   CONFARGS += --with-sysroot=$(with_sysroot)
> > +-endif
> > ++#endif
> > + ifneq ($(with_build_sysroot),)
> > +   CONFARGS += --with-build-sysroot=$(with_build_sysroot)
> > + endif
> >  @@ -854,9 +854,12 @@
> >         --target=$(TARGET_ALIAS)
> 
> While I see how this improves the symptom, I'd prefer to better
> understand what caused the regression.

Yes, sure! We'd want to spot where gcc_gxx_include_dir_add_sysroot is
not taken into account and add the missing sysroot there.

> Specifically, the older wdotap
> patches that used to work set with_sysroot = /.
> 
> https://sources.debian.org/src/cross-gcc/249/patches/gcc-13/0005-setting-all-the-various-paths-options-for-with_deps_.patch/#L86
> 
> Then I'd like to keep the wdotap patchset in a way that is upstreamable
> in principle (even though Matthias does not want it). Unconditionally
> adding --with-sysroot to all builds (including native builds) very much
> makes it not upstreamable,

Yes, that looked weird to me.

> so even if that approach ends up being "right", we will need to change
> it somehow to not affect native builds.
> 
> In refactoring the wdotap patchset, I dropped another relevant patch in
> favour of doing things differently. It affects code right below the
> comment you cite.
> 
> https://sources.debian.org/src/cross-gcc/249/patches/gcc-13/0008-g-include-directories-functional-again.patch/
> 
> cross-gcc-dev would earlier set with_multiarch_cxxheaders := yes and
> thus cause g++-multiarch-incdir.diff to be applied, which is patched via
> the 0008 one. I opted for replacing these changes (and thus my wdotap
> patchset does not enable g+++-multiarch-incdir.diff) with passing
> --with-gxx-include-dir='/$(PF)/include/c++/$(BASE_VERSION)' as this was
> mentioned in that 0008 patch as broken but apparently did actually work
> (though not the same way as you discovered).
> 
> The gcc.git commit that introduced the configure.ac comment you
> mentioned earlier was added in 2019
> https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=662b9c55cf06d3df6682ef865fb2b685820317a9
> so if this was a source of failure, we should have seen it earlier.
> 
> The commit message also says:
> 
> | At runtime, we prepend the sysroot that is actually active.
> 
> This hints that our build time and runtime sysroots might differ. I was
> earlier unaware that a runtime sysroot exists at all and I didn't find
> anything about it.
> 
> Equipped with all of this, I figured that the sysroot is simply stripped
> of the gxx-include-dir as a string, so how about passing
> 
>     --with-gxx-include-dir='//$(PF)/include/c++/$(BASE_VERSION)'
> 
> (and observing the double slash)? This surely warrants a comment, but it
> actually seems to work and get us past the gmp build.
> 
> Do you agree that this is a better workaround?

Yes, that looks better :)

At some point I tried --with-sysroot=//, which failed, and thought about
using /./, but I guess it's simply realpath-ed. Making gxx-include-dir
have an additional / to circumvent the issue looks fine enough to me.

> Do you see a better long-term solution that makes it work in a way
> intended by upstream rather than working by accident?

I didn't know about the runtime sysroot either. I guess it's still
somewhere that gcc_gxx_include_dir_add_sysroot is not being taken into
account. Possibly it's not noticing that an empty runtime sysroot should
be replaced by a / in that case otherwise gcc_gxx_include_dir gets
relative.

> Thanks for your support in all of this. Much appreciated.

You're welcome! The rebootstrap effort has been fantastically useful to
bootstrap hurd-amd64.

Samuel


Reply to: