[bug#42339] [PATCH core-updates] gnu: glibc-intermediate: Fixup the pre-configure phase.

[bug#42339] [PATCH core-updates] gnu: glibc-intermediate: Fixup the pre-configure phase.

[Jakub Kądziołka]

* gnu/packages/commencement.scm
  (glibc-final-with-bootstrap-bash)[arguments]: Don't patch sunrpc,
  as it's no longer required. Tweak C_INCLUDE_PATH and
  CPLUS_INCLUDE_PATH instead of CPATH.
---

Hi Guix,

I have stumbled upon this phase while I was working on making substitute
not fail silently. Turns out this substitution is no longer necessary
since we aren't building with --enable-obsolete-rpc anymore. I'm not
sure about the unsetenv parts, though. Switching them to
{C,CPLUS}_INCLUDE_PATH, which is where the header paths actually are
right now, did not change how `guix graph --type=references` looks for
glibc-final (nor glibc-final-with-bootstrap-bash, for that matter).

The phase itself was introduced in...

commit 1c93be5600fb90a64cbbdf7a55061902d2ff150a
Author: Ludovic Courtès <ludo@gnu.org>
Date:   Sat Jun 7 17:23:53 2014 +0200

    gnu: glibc: Make sure the bootstrap libc is not in $CPATH.

    This fixes a bug whereby the bootstrap-glibc headers could be picked up
    when building libc.so, which could be noticed by the fact that the
    .debug files contained references to bootstrap-glibc.

    * gnu/packages/base.scm (glibc-final-with-bootstrap-bash)[arguments]:
      Add 'pre-configure' phase.
      [inputs]: Remove 'alist-delete' call.

The debug output does not refer to a bootstrap glibc anymore whether the phase
is there or not, though it does refer to gcc-cross-boot0:lib. Does this
mean that the phase is simply obsolete, and only the hurd parts should
remain, or is the reference to gcc a bug? Perhaps gcc should have a
separate output for the includes it provides (stdarg and such) so that
this reference doesn't bring in the whole mesboot tree? This would
improve the closure of gcc-toolchain:debug...

Thoughts?

 gnu/packages/commencement.scm | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/gnu/packages/commencement.scm b/gnu/packages/commencement.scm
index 1a9c0eaf29..c645ac2589 100644
--- a/gnu/packages/commencement.scm
+++ b/gnu/packages/commencement.scm
@@ -3268,9 +3268,9 @@ memoized as a function of '%current-system'."
             `(modify-phases ,phases
                (add-before 'configure 'pre-configure
                  (lambda* (#:key inputs #:allow-other-keys)
-                   ;; Don't clobber CPATH with the bootstrap libc.
-                   (setenv "NATIVE_CPATH" (getenv "CPATH"))
-                   (unsetenv "CPATH")
+                   ;; Don't clobber include paths with the bootstrap libc.
+                   (unsetenv "C_INCLUDE_PATH")
+                   (unsetenv "CPLUS_INCLUDE_PATH")
 
                    ;; Tell 'libpthread' where to find 'libihash' on Hurd systems.
                    ,@(if (hurd-system?)
@@ -3281,13 +3281,6 @@ memoized as a function of '%current-system'."
                                            (assoc-ref %build-inputs "kernel-headers")
                                            "/lib/libihash.a\n"))))
                        '())
-
-                   ;; 'rpcgen' needs native libc headers to be built.
-                   (substitute* "sunrpc/Makefile"
-                     (("sunrpc-CPPFLAGS =.*" all)
-                      (string-append "CPATH = $(NATIVE_CPATH)\n"
-                                     "export CPATH\n"
-                                     all "\n")))
                    #t)))))))
     (propagated-inputs `(("kernel-headers" ,(kernel-headers-boot0))))
     (native-inputs
-- 
2.27.0

[bug#42339] [PATCH core-updates] gnu: glibc-intermediate: Fixup the pre-configure phase.

[Ludovic Courtès]

Hello,

Jakub Kądziołka <kuba@kadziolka.net> skribis:

> * gnu/packages/commencement.scm
>   (glibc-final-with-bootstrap-bash)[arguments]: Don't patch sunrpc,
>   as it's no longer required. Tweak C_INCLUDE_PATH and
>   CPLUS_INCLUDE_PATH instead of CPATH.
> ---
>
> Hi Guix,
>
> I have stumbled upon this phase while I was working on making substitute
> not fail silently. Turns out this substitution is no longer necessary
> since we aren't building with --enable-obsolete-rpc anymore. I'm not
> sure about the unsetenv parts, though. Switching them to
> {C,CPLUS}_INCLUDE_PATH, which is where the header paths actually are
> right now, did not change how `guix graph --type=references` looks for
> glibc-final (nor glibc-final-with-bootstrap-bash, for that matter).
>
> The phase itself was introduced in...
>
> commit 1c93be5600fb90a64cbbdf7a55061902d2ff150a
> Author: Ludovic Courtès <ludo@gnu.org>
> Date:   Sat Jun 7 17:23:53 2014 +0200
>
>     gnu: glibc: Make sure the bootstrap libc is not in $CPATH.
>
>     This fixes a bug whereby the bootstrap-glibc headers could be picked up
>     when building libc.so, which could be noticed by the fact that the
>     .debug files contained references to bootstrap-glibc.
>
>     * gnu/packages/base.scm (glibc-final-with-bootstrap-bash)[arguments]:
>       Add 'pre-configure' phase.
>       [inputs]: Remove 'alist-delete' call.
>
> The debug output does not refer to a bootstrap glibc anymore whether the phase
> is there or not, though it does refer to gcc-cross-boot0:lib. Does this
> mean that the phase is simply obsolete, and only the hurd parts should
> remain, or is the reference to gcc a bug? Perhaps gcc should have a
> separate output for the includes it provides (stdarg and such) so that
> this reference doesn't bring in the whole mesboot tree? This would
> improve the closure of gcc-toolchain:debug...

Oooh, nice.  LGTM!

Please confirm that nothing breaks (everything builds at least up to
‘gcc-final’) and you can push to ‘core-updates’.

Thank you!

Ludo’.

[bug#42339] [PATCH core-updates] gnu: glibc-intermediate: Fixup the pre-configure phase.

[Jakub Kądziołka]

[-- Attachment #1: Type: text/plain, Size: 2596 bytes --]

On Thu, Jul 23, 2020 at 12:00:22AM +0200, Ludovic Courtès wrote:
> Hello,
> 
> Jakub Kądziołka <kuba@kadziolka.net> skribis:
> 
> > * gnu/packages/commencement.scm
> >   (glibc-final-with-bootstrap-bash)[arguments]: Don't patch sunrpc,
> >   as it's no longer required. Tweak C_INCLUDE_PATH and
> >   CPLUS_INCLUDE_PATH instead of CPATH.
> > ---
> >
> > Hi Guix,
> >
> > I have stumbled upon this phase while I was working on making substitute
> > not fail silently. Turns out this substitution is no longer necessary
> > since we aren't building with --enable-obsolete-rpc anymore. I'm not
> > sure about the unsetenv parts, though. Switching them to
> > {C,CPLUS}_INCLUDE_PATH, which is where the header paths actually are
> > right now, did not change how `guix graph --type=references` looks for
> > glibc-final (nor glibc-final-with-bootstrap-bash, for that matter).
> >
> > The phase itself was introduced in...
> >
> > commit 1c93be5600fb90a64cbbdf7a55061902d2ff150a
> > Author: Ludovic Courtès <ludo@gnu.org>
> > Date:   Sat Jun 7 17:23:53 2014 +0200
> >
> >     gnu: glibc: Make sure the bootstrap libc is not in $CPATH.
> >
> >     This fixes a bug whereby the bootstrap-glibc headers could be picked up
> >     when building libc.so, which could be noticed by the fact that the
> >     .debug files contained references to bootstrap-glibc.
> >
> >     * gnu/packages/base.scm (glibc-final-with-bootstrap-bash)[arguments]:
> >       Add 'pre-configure' phase.
> >       [inputs]: Remove 'alist-delete' call.
> >
> > The debug output does not refer to a bootstrap glibc anymore whether the phase
> > is there or not, though it does refer to gcc-cross-boot0:lib. Does this
> > mean that the phase is simply obsolete, and only the hurd parts should
> > remain, or is the reference to gcc a bug? Perhaps gcc should have a
> > separate output for the includes it provides (stdarg and such) so that
> > this reference doesn't bring in the whole mesboot tree? This would
> > improve the closure of gcc-toolchain:debug...
> 
> Oooh, nice.  LGTM!

Thanks for your review!

> Please confirm that nothing breaks (everything builds at least up to
> ‘gcc-final’) and you can push to ‘core-updates’.

I have confirmed that hello builds both natively and cross-built. On a
similar topic, the same test succeeded for removing the native-gcc input.
As the comment suggests, it was only being used for --enable-obsolete-rpc.
I am thus considering a v2 that also removes said input. Does that make
sense?

Regards,
Jakub Kądziołka

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#42339] [PATCH core-updates] gnu: glibc-intermediate: Fixup the pre-configure phase.

[Ludovic Courtès]

Hi,

Jakub Kądziołka <kuba@kadziolka.net> skribis:

> On Thu, Jul 23, 2020 at 12:00:22AM +0200, Ludovic Courtès wrote:
>> Hello,
>> 
>> Jakub Kądziołka <kuba@kadziolka.net> skribis:
>> 
>> > * gnu/packages/commencement.scm
>> >   (glibc-final-with-bootstrap-bash)[arguments]: Don't patch sunrpc,
>> >   as it's no longer required. Tweak C_INCLUDE_PATH and
>> >   CPLUS_INCLUDE_PATH instead of CPATH.

[...]

>> > The debug output does not refer to a bootstrap glibc anymore whether the phase
>> > is there or not, though it does refer to gcc-cross-boot0:lib. Does this
>> > mean that the phase is simply obsolete, and only the hurd parts should
>> > remain, or is the reference to gcc a bug? Perhaps gcc should have a
>> > separate output for the includes it provides (stdarg and such) so that
>> > this reference doesn't bring in the whole mesboot tree? This would
>> > improve the closure of gcc-toolchain:debug...
>> 
>> Oooh, nice.  LGTM!
>
> Thanks for your review!
>
>> Please confirm that nothing breaks (everything builds at least up to
>> ‘gcc-final’) and you can push to ‘core-updates’.
>
> I have confirmed that hello builds both natively and cross-built. On a
> similar topic, the same test succeeded for removing the native-gcc input.
> As the comment suggests, it was only being used for --enable-obsolete-rpc.
> I am thus considering a v2 that also removes said input. Does that make
> sense?

Yes it does (again provided nothing breaks).

Thank you!

Ludo’.

bug#42339: [PATCH core-updates] gnu: glibc-intermediate: Fixup the pre-configure phase.

[Jakub Kądziołka]

[-- Attachment #1: Type: text/plain, Size: 1693 bytes --]

On Thu, Jul 23, 2020 at 09:49:17AM +0200, Ludovic Courtès wrote:
> Hi,
> 
> Jakub Kądziołka <kuba@kadziolka.net> skribis:
> 
> > On Thu, Jul 23, 2020 at 12:00:22AM +0200, Ludovic Courtès wrote:
> >> Hello,
> >> 
> >> Jakub Kądziołka <kuba@kadziolka.net> skribis:
> >> 
> >> > * gnu/packages/commencement.scm
> >> >   (glibc-final-with-bootstrap-bash)[arguments]: Don't patch sunrpc,
> >> >   as it's no longer required. Tweak C_INCLUDE_PATH and
> >> >   CPLUS_INCLUDE_PATH instead of CPATH.
> 
> [...]
> 
> >> > The debug output does not refer to a bootstrap glibc anymore whether the phase
> >> > is there or not, though it does refer to gcc-cross-boot0:lib. Does this
> >> > mean that the phase is simply obsolete, and only the hurd parts should
> >> > remain, or is the reference to gcc a bug? Perhaps gcc should have a
> >> > separate output for the includes it provides (stdarg and such) so that
> >> > this reference doesn't bring in the whole mesboot tree? This would
> >> > improve the closure of gcc-toolchain:debug...
> >> 
> >> Oooh, nice.  LGTM!
> >
> > Thanks for your review!
> >
> >> Please confirm that nothing breaks (everything builds at least up to
> >> ‘gcc-final’) and you can push to ‘core-updates’.
> >
> > I have confirmed that hello builds both natively and cross-built. On a
> > similar topic, the same test succeeded for removing the native-gcc input.
> > As the comment suggests, it was only being used for --enable-obsolete-rpc.
> > I am thus considering a v2 that also removes said input. Does that make
> > sense?
> 
> Yes it does (again provided nothing breaks).
> 
> Thank you!

Thanks, patches pushed!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]