driver: Extend 'getenv' function to allow default value

Message ID 20220827051422.1023580-1-keithp@keithp.com
State New, archived
Headers
Series driver: Extend 'getenv' function to allow default value |

Commit Message

Keith Packard Aug. 27, 2022, 5:14 a.m. UTC
  Right now, a missing environment variable provided to the 'getenv'
function in a .specs file causes a fatal error. That makes writing a
spec file that uses the GCC_EXEC_PREFIX value difficult as that
variable is only set when the driver has been relocated, but not when
run from the defined location. This makes building a relocatable
toolchain difficult to extend to other ancilary pieces which use specs
files to locate header and library files adjacent to the toolchain.

This patch adds an optional third argument to the getenv function that
can be used to fall back to the standard installation path when the
driver hasn't set GCC_EXEC_PREFIX in the environment.

For example, if an alternate C library is installed in
${prefix}/extra, then this change allows the specs file to locate that
relative to the gcc directory, if gcc is located in the original
installation directory (which would leave GCC_EXEC_PREFIX unset), or
if the gcc tree has been moved to a different location (where gcc
would set GCC_EXEC_PREFIX itself):

*cpp:
-isystem %:getenv(GCC_EXEC_PREFIX ../../extra/include ${prefix}/extra/include)

I considered changing the behavior of either the %R sequence so that
it had a defined behavior when there was no sysroot defined, or making
the driver always set the GCC_EXEC_PREFIX environment variable and
decided that the approach of adding functionality to getenv where it
was previously invalid would cause the least potential for affecting
existing usage.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 gcc/doc/invoke.texi | 18 +++++++++++-------
 gcc/gcc.cc          | 10 +++++++++-
 2 files changed, 20 insertions(+), 8 deletions(-)
  

Comments

Keith Packard Sept. 23, 2022, 6:40 p.m. UTC | #1
I submitted the referenced patch to extend the 'getenv' .specs function
back in August and didn't see any response, so I wanted to provide a bit
more context to see if that would help people understand why I wrote
this.

Here's a link to that message:

        https://gcc.gnu.org/pipermail/gcc-patches/2022-August/600452.html

I'm working with embedded toolchains where I want to distribute binary
versions of binutils, gcc and a suite of libraries in a tar file which
the user can unpack anywhere on their system. To make this work, I need
to create .spec file fragments that can locate the correct libraries
relative to the location where the toolchain was unpacked.

An easy way to do this, which doesn't depend on a default sysroot value,
is to use the GCC_EXEC_PREFIX environment variable in the .specs
file. Gcc sets that whenever it discovers that it hasn't been run from
the defined installation path. However, if the user does end up
installing gcc in the defined installation path, then that variable
isn't set at all. If a .specs file attempts to reference the variable,
gcc will emit a fatal error and exit.

This patch makes it possible for the .specs file fragment to provide the
default path as a fallback for a missing environment variable so that,
instead of exiting, the correct value will be substituted instead.

By doing this, I can create portable .specs file fragments which work
wherever the toolchain is installed.

This patch seemed like the least invasive approach to solving this
problem, but there are two other approaches that could work, and which
would make the .specs files simpler:

 1. Always set the GCC_EXEC_PREFIX environment variable, even if GCC
    is executed from the expected location.

 2. Make %R in .specs files expand to the appropriate value even if
    there is no sysroot defined.

I'd be happy to provide an implementation of either of those if that
would be more acceptable.
  
Jeff Law Nov. 20, 2022, 6:34 p.m. UTC | #2
On 9/23/22 12:40, Keith Packard via Gcc-patches wrote:
> I submitted the referenced patch to extend the 'getenv' .specs function
> back in August and didn't see any response, so I wanted to provide a bit
> more context to see if that would help people understand why I wrote
> this.

I think most folks generally loathe getting into specs and such. So as 
one of the reviewers of last resort, I'll see what I can do here.


So we already have a goodly amount of infrastructure for relocateable 
toolchains and relocateable sysroots.  So the natural question that 
arises is what is it about your environment that is different and 
prevents those existing mechanisms from working.



>
> Here's a link to that message:
>
>          https://gcc.gnu.org/pipermail/gcc-patches/2022-August/600452.html
>
> I'm working with embedded toolchains where I want to distribute binary
> versions of binutils, gcc and a suite of libraries in a tar file which
> the user can unpack anywhere on their system. To make this work, I need
> to create .spec file fragments that can locate the correct libraries
> relative to the location where the toolchain was unpacked.

So the first half of that paragraph describes what I do all the time.  
I've got cross toolchain (gcc, binutils, various libraries & headers).  
Those all go into a sysroot and I can relocate the toolchain to anywhere 
in the filesystem and it "just works". I do this for both bare metal 
tooclhains using newlib as well as linux-gnu toolchains using glibc.


Now you mention you need to create .spec file fragments to locate the 
correct libraries.  But if the libraries are in the sysroot, then you 
shouldn't really need to do anything special.  Drop them into the right 
place and they should relocate just like glibc, newlib, etc.


So maybe the problem is you're not using sysroots?




>
> An easy way to do this, which doesn't depend on a default sysroot value,
> is to use the GCC_EXEC_PREFIX environment variable in the .specs

Are you not using sysroots at all?  If so, why not?



> file. Gcc sets that whenever it discovers that it hasn't been run from
> the defined installation path. However, if the user does end up
> installing gcc in the defined installation path, then that variable
> isn't set at all. If a .specs file attempts to reference the variable,
> gcc will emit a fatal error and exit.

This is a good hint.  Have you considered building with a sysroot like 
/wontexist, installing everything into there, then moving the whole 
thing to a more sensible directory before tarring it up?


What's useful about that is you always have a sysroot defined, but it 
generally won't exist at runtime.  so GCC_EXEC_PREFIX should always be 
set and everything should "just work" from that point onward.


Jeff
  

Patch

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 6131bfa7acf..669c28a609a 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -34223,17 +34223,21 @@  The following built-in spec functions are provided:
 
 @table @code
 @item @code{getenv}
-The @code{getenv} spec function takes two arguments: an environment
-variable name and a string.  If the environment variable is not
-defined, a fatal error is issued.  Otherwise, the return value is the
-value of the environment variable concatenated with the string.  For
-example, if @env{TOPDIR} is defined as @file{/path/to/top}, then:
+
+The @code{getenv} spec function takes two or three arguments: an
+environment variable name, a string and an optional default value.  If
+the environment variable is not defined and a default value is
+provided, that is used as the return value; otherwise a fatal error is
+issued.  Otherwise, the return value is the value of the environment
+variable concatenated with the string.  For example, if @env{TOPDIR}
+is defined as @file{/path/to/top}, then:
 
 @smallexample
-%:getenv(TOPDIR /include)
+%:getenv(TOPDIR /include /path/to/default/include)
 @end smallexample
 
-expands to @file{/path/to/top/include}.
+expands to @file{/path/to/top/include}. If @env{TOPDIR} is not
+defined, then this expands to @file{/path/to/default/include}.
 
 @item @code{if-exists}
 The @code{if-exists} spec function takes one argument, an absolute
diff --git a/gcc/gcc.cc b/gcc/gcc.cc
index b6d562a92f0..16295702db7 100644
--- a/gcc/gcc.cc
+++ b/gcc/gcc.cc
@@ -10169,12 +10169,20 @@  getenv_spec_function (int argc, const char **argv)
   char *ptr;
   size_t len;
 
-  if (argc != 2)
+  if (argc != 2 && argc != 3)
     return NULL;
 
   varname = argv[0];
   value = env.get (varname);
 
+  if (!value && argc == 3)
+    {
+      value = argv[2];
+      result = XNEWVAR(char, strlen(value) + 1);
+      strcpy(result, value);
+      return result;
+    }
+
   /* If the variable isn't defined and this is allowed, craft our expected
      return value.  Assume variable names used in specs strings don't contain
      any active spec character so don't need escaping.  */