PATCH: --sysroot-suffix

PATCH: --sysroot-suffix

[Mark Mitchell]

This patch adds a --sysroot-suffix option to ld.  

The scenario in which this is useful is as follows.  Suppose that you
have built a single cross compiler (say, for MIPS), but two versions of
GLIBC (installed, in, say, "eb" and "el" subdirectories of prefix).
It's insufficient just to provide appropriate -L options because the
linker has to be able to resolve linker scripts that contain absolute
paths correctly.  The --sysroot-suffix option to the linker
complements the SYSROOT_SUFFIX_SPEC already in GCC.

OK to commit?

--
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com


2005-01-16  Mark Mitchell  <mark@codesourcery.com>

	* ldmain.c (main): Add spport --sysroot-suffix option.
	* ld.texinfo: Document --sysroot-suffix.

*** ./ld.texinfo	Mon Nov 15 15:21:27 2004
--- /home/mitchell/scratch/wrs-linux/src/binutils-20041119/ld/ld.texinfo	Sun Jan 16 18:48:49 2005
***************
*** 793,798 ****
--- 793,807 ----
  @itemx --strip-debug
  Omit debugger symbol information (but not all symbols) from the output file.
  
+ @kindex --sysroot-suffix @var{suffix}
+ @cindex subdirectory of the sysroot, specifying
+ @item --sysroot-suffix @var{suffix}
+ This option can be used to specify that a subdirectory of the usual
+ sysroot prefix should be used as the sysroot prefix.  For example, if
+ '/opt/binutils' is the normal system root, and the suffix specified is
+ '/bigendian', then the sysroot becomes '/opt/binutils/bigendian'.  If
+ present, this option must be the first option on the command-line.
+ 
  @kindex -t
  @kindex --trace
  @cindex input files, displaying
Only in /home/mitchell/scratch/wrs-linux/src/binutils-20041119/ld: ld.texinfo~
diff -c -r ./ldmain.c /home/mitchell/scratch/wrs-linux/src/binutils-20041119/ld/ldmain.c
*** ./ldmain.c	Mon Nov 15 15:21:27 2004
--- /home/mitchell/scratch/wrs-linux/src/binutils-20041119/ld/ldmain.c	Fri Jan 14 15:38:27 2005
***************
*** 237,246 ****
  
    if (! ld_sysroot)
  #endif
!     ld_sysroot = TARGET_SYSTEM_ROOT;
  
    if (ld_sysroot && *ld_sysroot)
!     ld_canon_sysroot = lrealpath (ld_sysroot);
  
    if (ld_canon_sysroot)
      ld_canon_sysroot_len = strlen (ld_canon_sysroot);
--- 237,271 ----
  
    if (! ld_sysroot)
  #endif
!     ld_sysroot = xstrdup (TARGET_SYSTEM_ROOT);
  
    if (ld_sysroot && *ld_sysroot)
!     {
!       char *ld_sysroot_suffix = NULL;
! 
!       if (argc > 1 && strcmp (argv[1], "--sysroot-suffix") == 0)
! 	{
! 	  if (argc < 3)
! 	    {
! 	      einfo (_("--sysroot-suffix requires argument"));
! 	      xexit (1);
! 	    }
! 	  ld_sysroot_suffix = argv[2];
! 	  argc -= 2;
! 	  argv += 2;
! 	}
! 
!       if (ld_sysroot_suffix)
! 	{
! 	  size_t sysroot_len = strlen (ld_sysroot);
! 	  ld_sysroot = xrealloc (ld_sysroot,
! 				 sysroot_len
! 				 + strlen (ld_sysroot_suffix)
! 				 + 1 /* '\0' */);
! 	  strcpy (ld_sysroot + sysroot_len, ld_sysroot_suffix);
! 	}
!       ld_canon_sysroot = lrealpath (ld_sysroot);
!     }
  
    if (ld_canon_sysroot)
      ld_canon_sysroot_len = strlen (ld_canon_sysroot);

Re: PATCH: --sysroot-suffix

[Daniel Jacobowitz]

On Sun, Jan 16, 2005 at 07:22:19PM -0800, Mark Mitchell wrote:
> 
> This patch adds a --sysroot-suffix option to ld.  
> 
> The scenario in which this is useful is as follows.  Suppose that you
> have built a single cross compiler (say, for MIPS), but two versions of
> GLIBC (installed, in, say, "eb" and "el" subdirectories of prefix).
> It's insufficient just to provide appropriate -L options because the
> linker has to be able to resolve linker scripts that contain absolute
> paths correctly.  The --sysroot-suffix option to the linker
> complements the SYSROOT_SUFFIX_SPEC already in GCC.
> 
> OK to commit?

Would a --sysroot option work for you instead?  I can't think of any
realistic configurations where GCC and LD would have different
sysroots, so passing it down from gcc makes some sense.

(This would work well with the Darwin folks trying to make -isysroot
work for everything.)

-- 
Daniel Jacobowitz

Re: PATCH: --sysroot-suffix

[Mark Mitchell]

Daniel Jacobowitz wrote:

> Would a --sysroot option work for you instead?  I can't think of any
> realistic configurations where GCC and LD would have different
> sysroots, so passing it down from gcc makes some sense.

I thought about this, but it seemed more complicated.

To do what you suggest, I would have to make more invasive changes to 
the GCC driver.  Because the driver can't assume that binutils supports 
the new option, I was just modifying the linker spec for the target in 
question.  (That target also defines SYSROOT_SUFFIX_SPEC.)  I don't know 
of a way for a spec to request that the driver substitute in the current 
sysroot.

In summary, to do what you're suggesting, the way this would work would be:

1) Modify the linker to support --sysroot.  This would be along the 
lines of my current patch, except that it would smply override the 
calculated sysroot.

2) Modify the GCC driver to support a new spec substitution character, 
like "%r".

3) Modify the GCC target in question to use "%r" appropriately in the 
link spec, so as to append the right suffix.

I'm willing to do that if the binutils people will pre-approve that 
approach in concept and prefer it to my current patch.  However, I'm not 
sure I see a major advantage.  My current patch will work for the Darwin 
folks too, so far as I can tell; they would just provide a relative path 
from the configured binutils sysroot to the actual location of the 
sysroot they want the linker to use.

Thanks,

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: PATCH: --sysroot-suffix

[Richard Sandiford]

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

Mark Mitchell <mark@codesourcery.com> writes:
> Daniel Jacobowitz wrote:
>> Would a --sysroot option work for you instead?  I can't think of any
>> realistic configurations where GCC and LD would have different
>> sysroots, so passing it down from gcc makes some sense.

Sounds like this changeable sysroot thing is a hotter topic than
I'd realised.  As mentioned on gcc@:

    http://gcc.gnu.org/ml/gcc/2004-12/msg00099.html

I've got some patches to add the --sysroot option that Daniel suggests,
but I've been sitting on them waiting for gcc to enter stage 1 (or for
the 2.16 release process to kick off, whichever was sooner).

FWIW, the patches are attached.  They were developed against slightly
older versions of the tools and I haven't tested whether they still
work.  There are no docs yet either.  Still, I thought it might be
worth throw ingthem into this thread to see what you folks think.
I can update them, test them, and document them if it's thought
to be way to go.

The patches were originally tested on a mips64-linux-gnu cross.
Due to the wonders of gcc's option mangling, the gcc part will
accept both --sysroot=/path and -fsysroot=/path.

> To do what you suggest, I would have to make more invasive changes to
> the GCC driver.  Because the driver can't assume that binutils supports
> the new option, I was just modifying the linker spec for the target in
> question.

Might be missing the point here, but in the patches below, gcc will only
handle --sysroot=/-fsysroot= correctly if the linker also understands
--sysroot=.  It seemed like a reasonable restriction at the time
but I'd be interested to know what others think.

Richard


ld/
	* ldmain.h (ld_sysroot): Change type to a constant string.
	* ldmain.c (ld_sysroot): Likewise.
	(get_relative_sysroot, get_sysroot): New functions, adding command-line
	support for changing the sysroot.
	(main): Call the new functions.
	* lexsup.c (OPTION_SYSROOT): New.
	(ld_options): Add --sysroot.
	(parse_args): Add a dummy handler for it.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: ld-sysroot.diff --]
[-- Type: text/x-patch, Size: 5891 bytes --]

Index: ld/ldmain.h
===================================================================
RCS file: /cvs/src/src/ld/ldmain.h,v
retrieving revision 1.9
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.9 ldmain.h
*** ld/ldmain.h	19 Jul 2004 16:40:52 -0000	1.9
--- ld/ldmain.h	17 Jan 2005 10:27:02 -0000
***************
*** 23,29 ****
  #define LDMAIN_H
  
  extern char *program_name;
! extern char *ld_sysroot;
  extern char *ld_canon_sysroot;
  extern int ld_canon_sysroot_len;
  extern bfd *output_bfd;
--- 23,29 ----
  #define LDMAIN_H
  
  extern char *program_name;
! extern const char *ld_sysroot;
  extern char *ld_canon_sysroot;
  extern int ld_canon_sysroot_len;
  extern bfd *output_bfd;
Index: ld/ldmain.c
===================================================================
RCS file: /cvs/src/src/ld/ldmain.c,v
retrieving revision 1.90
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.90 ldmain.c
*** ld/ldmain.c	20 Dec 2004 15:16:06 -0000	1.90
--- ld/ldmain.c	17 Jan 2005 10:27:04 -0000
*************** const char *output_filename = "a.out";
*** 68,74 ****
  char *program_name;
  
  /* The prefix for system library directories.  */
! char *ld_sysroot;
  
  /* The canonical representation of ld_sysroot.  */
  char * ld_canon_sysroot;
--- 68,74 ----
  char *program_name;
  
  /* The prefix for system library directories.  */
! const char *ld_sysroot;
  
  /* The canonical representation of ld_sysroot.  */
  char * ld_canon_sysroot;
*************** ld_config_type config;
*** 110,115 ****
--- 110,117 ----
  
  sort_type sort_section;
  
+ static const char *get_sysroot
+   (int, char **);
  static char *get_emulation
    (int, char **);
  static void set_scripts_dir
*************** main (int argc, char **argv)
*** 201,247 ****
  
    xatexit (remove_output);
  
! #ifdef TARGET_SYSTEM_ROOT_RELOCATABLE
!   ld_sysroot = make_relative_prefix (program_name, BINDIR,
! 				     TARGET_SYSTEM_ROOT);
! 
!   if (ld_sysroot)
!     {
!       struct stat s;
!       int res = stat (ld_sysroot, &s) == 0 && S_ISDIR (s.st_mode);
! 
!       if (!res)
! 	{
! 	  free (ld_sysroot);
! 	  ld_sysroot = NULL;
! 	}
!     }
! 
!   if (! ld_sysroot)
      {
!       ld_sysroot = make_relative_prefix (program_name, TOOLBINDIR,
! 					 TARGET_SYSTEM_ROOT);
! 
!       if (ld_sysroot)
  	{
! 	  struct stat s;
! 	  int res = stat (ld_sysroot, &s) == 0 && S_ISDIR (s.st_mode);
! 
! 	  if (!res)
! 	    {
! 	      free (ld_sysroot);
! 	      ld_sysroot = NULL;
! 	    }
  	}
      }
- 
-   if (! ld_sysroot)
- #endif
-     ld_sysroot = TARGET_SYSTEM_ROOT;
- 
-   if (ld_sysroot && *ld_sysroot)
-     ld_canon_sysroot = lrealpath (ld_sysroot);
- 
    if (ld_canon_sysroot)
      ld_canon_sysroot_len = strlen (ld_canon_sysroot);
    else
--- 203,220 ----
  
    xatexit (remove_output);
  
!   /* Set up the sysroot directory.  */
!   ld_sysroot = get_sysroot (argc, argv);
!   if (*ld_sysroot)
      {
!       if (*TARGET_SYSTEM_ROOT == 0)
  	{
! 	  einfo ("%P%F: this linker was not configured to use sysroots");
! 	  ld_sysroot = "";
  	}
+       else
+ 	ld_canon_sysroot = lrealpath (ld_sysroot);
      }
    if (ld_canon_sysroot)
      ld_canon_sysroot_len = strlen (ld_canon_sysroot);
    else
*************** main (int argc, char **argv)
*** 587,592 ****
--- 560,610 ----
    return 0;
  }
  
+ /* If the configured sysroot is relocatable, try relocating it based on
+    default prefix FROM.  Return the relocated directory if it exists,
+    otherwise return null.  */
+ 
+ static char *
+ get_relative_sysroot (const char *from ATTRIBUTE_UNUSED)
+ {
+ #ifdef TARGET_SYSTEM_ROOT_RELOCATABLE
+   char *path;
+   struct stat s;
+ 
+   path = make_relative_prefix (program_name, from, TARGET_SYSTEM_ROOT);
+   if (path)
+     {
+       if (stat (path, &s) == 0 && S_ISDIR (s.st_mode))
+ 	return path;
+       free (path);
+     }
+ #endif
+   return 0;
+ }
+ 
+ /* Return the sysroot directory.  Return "" if no sysroot is being used.  */
+ 
+ static const char *
+ get_sysroot (int argc, char **argv)
+ {
+   int i;
+   const char *path;
+ 
+   for (i = 1; i < argc; i++)
+     if (strncmp (argv[i], "--sysroot=", strlen ("--sysroot=")) == 0)
+       return argv[i] + strlen ("--sysroot=");
+ 
+   path = get_relative_sysroot (BINDIR);
+   if (path)
+     return path;
+ 
+   path = get_relative_sysroot (TOOLBINDIR);
+   if (path)
+     return path;
+ 
+   return TARGET_SYSTEM_ROOT;
+ }
+ 
  /* We need to find any explicitly given emulation in order to initialize the
     state that's needed by the lex&yacc argument parser (parse_args).  */
  
Index: ld/lexsup.c
===================================================================
RCS file: /cvs/src/src/ld/lexsup.c,v
retrieving revision 1.81
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.81 lexsup.c
*** ld/lexsup.c	15 Nov 2004 23:21:27 -0000	1.81
--- ld/lexsup.c	17 Jan 2005 10:27:06 -0000
*************** enum option_values
*** 71,76 ****
--- 71,77 ----
    OPTION_DEFSYM,
    OPTION_DEMANGLE,
    OPTION_DYNAMIC_LINKER,
+   OPTION_SYSROOT,
    OPTION_EB,
    OPTION_EL,
    OPTION_EMBEDDED_RELOCS,
*************** static const struct ld_option ld_options
*** 233,238 ****
--- 234,241 ----
    { {"library-path", required_argument, NULL, 'L'},
      'L', N_("DIRECTORY"), N_("Add DIRECTORY to library search path"),
      TWO_DASHES },
+   { {"sysroot", required_argument, NULL, OPTION_SYSROOT},
+     '\0', N_("PATH"), N_("Change the configured sysroot"), TWO_DASHES },
    { {NULL, required_argument, NULL, '\0'},
      'm', N_("EMULATION"), N_("Set emulation"), ONE_DASH },
    { {"print-map", no_argument, NULL, 'M'},
*************** parse_args (unsigned argc, char **argv)
*** 747,752 ****
--- 750,758 ----
  	case OPTION_DYNAMIC_LINKER:
  	  command_line.interpreter = optarg;
  	  break;
+ 	case OPTION_SYSROOT:
+ 	  /* Already handled in ldmain.c.  */
+ 	  break;
  	case OPTION_EB:
  	  command_line.endian = ENDIAN_BIG;
  	  break;

[-- Attachment #3: Type: text/plain, Size: 142 bytes --]


gcc/
	* gcc.c (process_command): Handle -fsysroot=.  Bypass relocatable
	sysroot handling if the sysroot has been set on the command line.
	

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: gcc-sysroot.diff --]
[-- Type: text/x-patch, Size: 2141 bytes --]

Index: gcc.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/gcc.c,v
retrieving revision 1.444
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.444 gcc.c
*** gcc.c	7 Jan 2005 01:05:37 -0000	1.444
--- gcc.c	17 Jan 2005 10:14:59 -0000
*************** warranty; not even for MERCHANTABILITY o
*** 3449,3454 ****
--- 3449,3467 ----
  	  add_assembler_option ("--target-help", 13);
  	  add_linker_option ("--target-help", 13);
  	}
+       else if (! strncmp (argv[i], "-fsysroot=", strlen ("-fsysroot=")))
+ 	{
+ 	  if (target_system_root == 0)
+ 	    error ("warning: this compiler was not configured "
+ 		   "to use sysroots");
+ 	  else
+ 	    {
+ 	      target_system_root = argv[i] + strlen ("-fsysroot=");
+ 	      target_system_root_changed = 1;
+ 	      add_linker_option (concat ("--", argv[i] + 2, NULL),
+ 				 strlen (argv[i]));
+ 	    }
+ 	}
        else if (! strcmp (argv[i], "-pass-exit-codes"))
  	{
  	  pass_exit_codes = 1;
*************** warranty; not even for MERCHANTABILITY o
*** 3869,3875 ****
       then consider it to relocate with the rest of the GCC installation
       if GCC_EXEC_PREFIX is set.
       ``make_relative_prefix'' is not compiled for VMS, so don't call it.  */
!   if (target_system_root && gcc_exec_prefix)
      {
        char *tmp_prefix = make_relative_prefix (argv[0],
  					       standard_bindir_prefix,
--- 3882,3888 ----
       then consider it to relocate with the rest of the GCC installation
       if GCC_EXEC_PREFIX is set.
       ``make_relative_prefix'' is not compiled for VMS, so don't call it.  */
!   if (target_system_root && gcc_exec_prefix && !target_system_root_changed)
      {
        char *tmp_prefix = make_relative_prefix (argv[0],
  					       standard_bindir_prefix,
*************** warranty; not even for MERCHANTABILITY o
*** 3915,3920 ****
--- 3928,3935 ----
  	;
        else if (! strncmp (argv[i], "-Wp,", 4))
  	;
+       else if (! strncmp (argv[i], "-fsysroot=", strlen ("-fsysroot=")))
+ 	;
        else if (! strcmp (argv[i], "-pass-exit-codes"))
  	;
        else if (! strcmp (argv[i], "-print-search-dirs"))

Re: PATCH: --sysroot-suffix

[Hans-Peter Nilsson]

On Mon, 17 Jan 2005, Richard Sandiford wrote:
> I've got some patches to add the --sysroot option that Daniel suggests,
> but I've been sitting on them waiting for gcc to enter stage 1 (or for
> the 2.16 release process to kick off, whichever was sooner).

I don't understand the wait on the binutils patches, but (no
surprise) I should way I like this.

> Might be missing the point here, but in the patches below, gcc will only
> handle --sysroot=/-fsysroot= correctly if the linker also understands
> --sysroot=.  It seemed like a reasonable restriction at the time
> but I'd be interested to know what others think.

With that, I see no problem at all.

I do see a problem that this patch seems to require that the
tools were configured with --with-sysroot=.
Am I misunderstanding?  If not, can this restriction be lifted?

brgds, H-P

Re: PATCH: --sysroot-suffix

[Richard Sandiford]

Hans-Peter Nilsson <hp@bitrange.com> writes:
> On Mon, 17 Jan 2005, Richard Sandiford wrote:
>> I've got some patches to add the --sysroot option that Daniel suggests,
>> but I've been sitting on them waiting for gcc to enter stage 1 (or for
>> the 2.16 release process to kick off, whichever was sooner).
>
> I don't understand the wait on the binutils patches,

Well, I was hoping gcc would enter stage 1 before the 2.16 process
began, and that I could then submit both patches at the same time.
Seemed easier to justify that way.

> I do see a problem that this patch seems to require that the
> tools were configured with --with-sysroot=.

...or just plain --with-sysroot...

> Am I misunderstanding?

No.

> If not, can this restriction be lifted?

I'll leave that as an exercise to the reader ;)  The patch was
written with cross toolchains in mind, and in my experience,
you either use sysroots all the time for a particular cross
toolchain, or you don't use them at all.

Richard

Re: PATCH: --sysroot-suffix

[Daniel Jacobowitz]

On Mon, Jan 17, 2005 at 11:44:41AM +0000, Richard Sandiford wrote:
> Hans-Peter Nilsson <hp@bitrange.com> writes:
> > On Mon, 17 Jan 2005, Richard Sandiford wrote:
> >> I've got some patches to add the --sysroot option that Daniel suggests,
> >> but I've been sitting on them waiting for gcc to enter stage 1 (or for
> >> the 2.16 release process to kick off, whichever was sooner).
> >
> > I don't understand the wait on the binutils patches,
> 
> Well, I was hoping gcc would enter stage 1 before the 2.16 process
> began, and that I could then submit both patches at the same time.
> Seemed easier to justify that way.
> 
> > I do see a problem that this patch seems to require that the
> > tools were configured with --with-sysroot=.
> 
> ...or just plain --with-sysroot...
> 
> > Am I misunderstanding?
> 
> No.
> 
> > If not, can this restriction be lifted?
> 
> I'll leave that as an exercise to the reader ;)  The patch was
> written with cross toolchains in mind, and in my experience,
> you either use sysroots all the time for a particular cross
> toolchain, or you don't use them at all.

Apple were interested in using it for native toolchains, however.  Plus
the restriction is one of those things that is awkward to document. 
Can you explain why there was a problem?

-- 
Daniel Jacobowitz

Re: PATCH: --sysroot-suffix

[Richard Sandiford]

Daniel Jacobowitz <drow@false.org> writes:
> Apple were interested in using it for native toolchains, however.  Plus
> the restriction is one of those things that is awkward to document. 
> Can you explain why there was a problem?

I never tried it for native configurations, so I don't know for sure
that there is a problem.  But I was worried if we didn't have this
restriction, people would expect passing --sysroot=/foo to a non-
sysrooted toolchain to have exactly the same meaning as configuring
with --with-sysroot=/foo.  That isn't going to be true without further
surgery because --with-sysroot affects the default search path.  (see
ld/genscripts.sh, for example.)

But to be honest, the main blocker for me is that I don't really know
what --sysroot is supposed to do on a non-sysrooted toolchain.  If this
patch does what's required, then great.  I've no objection to removing
the error.

Richard

Re: PATCH: --sysroot-suffix

[Daniel Jacobowitz]

On Mon, Jan 17, 2005 at 02:46:28PM +0000, Richard Sandiford wrote:
> Daniel Jacobowitz <drow@false.org> writes:
> > Apple were interested in using it for native toolchains, however.  Plus
> > the restriction is one of those things that is awkward to document. 
> > Can you explain why there was a problem?
> 
> I never tried it for native configurations, so I don't know for sure
> that there is a problem.  But I was worried if we didn't have this
> restriction, people would expect passing --sysroot=/foo to a non-
> sysrooted toolchain to have exactly the same meaning as configuring
> with --with-sysroot=/foo.  That isn't going to be true without further
> surgery because --with-sysroot affects the default search path.  (see
> ld/genscripts.sh, for example.)

You're right - it won't work without further surgery.  Hmm.  It should
not be too hard to fix that.

-- 
Daniel Jacobowitz

Re: PATCH: --sysroot-suffix

[Mark Mitchell]

Richard Sandiford wrote:
> Daniel Jacobowitz <drow@false.org> writes:
> 
>>Apple were interested in using it for native toolchains, however.  Plus
>>the restriction is one of those things that is awkward to document. 
>>Can you explain why there was a problem?
> 
> 
> I never tried it for native configurations, so I don't know for sure
> that there is a problem. 

The situation I'm interested in is for cross configurations as well. 
Anyhow, I'm just as happy with your patch as with mine.  All I want is 
to have an agreed-upon mechanism available to solve the problem.

I would definitely encourage the binutils maintainers to settle on one 
of the approaches, and get that set of patches committed, independent of 
the GCC development stage.  That will actually make it easier to get the 
matching patches into GCC later, and will keep a third person from 
independently developing yet another set of patches.

Thanks,

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: PATCH: --sysroot-suffix

[Hans-Peter Nilsson]

On Mon, 17 Jan 2005, Richard Sandiford wrote:
> I never tried it for native configurations, so I don't know for sure
> that there is a problem.  But I was worried if we didn't have this
> restriction, people would expect passing --sysroot=/foo to a non-
> sysrooted toolchain to have exactly the same meaning as configuring
> with --with-sysroot=/foo.

I know *I* would. ;-)

>  That isn't going to be true without further
> surgery because --with-sysroot affects the default search path.  (see
> ld/genscripts.sh, for example.)

For that specific example, IMHO it's certainly fine to require
COMPILE_IN for targets where --sysroot should work out of the
box -- or of course that the user has moved the scripts to the
right place.  It's not a requirement of --sysroot to make sure
stuff is in the place pointed at.

> But to be honest, the main blocker for me is that I don't really know
> what --sysroot is supposed to do on a non-sysrooted toolchain.

Um, the same as one configure --with-sysroot? :-)

Having said that, I'm fine with documenting the restriction and
getting a solution that doesn't require --with-sysroot later.

I'm also ok with making this a cross-toolchain-only option
(independently).

FWIW.

brgds, H-P

Re: PATCH: --sysroot-suffix

[Richard Sandiford]

OK, the concensus seemed to be that a --sysroot flag was worth having,
and that it would be nice to support it on non-sysrooted linkers too,
but that it wasn't necessary for the first cut.

Here's a cleaned-up version of the patch, this time with docs and
NEWS item.  Tested on mips-linux-gnu.  OK to install?

Richard


	* ldmain.h (ld_sysroot): Change type to a constant string.
	* ldmain.c (ld_sysroot): Likewise.
	(get_relative_sysroot, get_sysroot): New functions, adding command-line
	support for changing the sysroot.
	(main): Call the new functions.
	* lexsup.c (OPTION_SYSROOT): New.
	(ld_options): Add --sysroot.
	(parse_args): Add a dummy handler for it.
	* ld.texinfo (--sysroot): Document.
	* NEWS: Mention the new --sysroot option.

Index: ld/ldmain.h
===================================================================
RCS file: /cvs/src/src/ld/ldmain.h,v
retrieving revision 1.9
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.9 ldmain.h
*** ld/ldmain.h	19 Jul 2004 16:40:52 -0000	1.9
--- ld/ldmain.h	18 Jan 2005 15:40:10 -0000
***************
*** 23,29 ****
  #define LDMAIN_H
  
  extern char *program_name;
! extern char *ld_sysroot;
  extern char *ld_canon_sysroot;
  extern int ld_canon_sysroot_len;
  extern bfd *output_bfd;
--- 23,29 ----
  #define LDMAIN_H
  
  extern char *program_name;
! extern const char *ld_sysroot;
  extern char *ld_canon_sysroot;
  extern int ld_canon_sysroot_len;
  extern bfd *output_bfd;
Index: ld/ldmain.c
===================================================================
RCS file: /cvs/src/src/ld/ldmain.c,v
retrieving revision 1.90
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.90 ldmain.c
*** ld/ldmain.c	20 Dec 2004 15:16:06 -0000	1.90
--- ld/ldmain.c	18 Jan 2005 15:40:12 -0000
*************** const char *output_filename = "a.out";
*** 68,74 ****
  char *program_name;
  
  /* The prefix for system library directories.  */
! char *ld_sysroot;
  
  /* The canonical representation of ld_sysroot.  */
  char * ld_canon_sysroot;
--- 68,74 ----
  char *program_name;
  
  /* The prefix for system library directories.  */
! const char *ld_sysroot;
  
  /* The canonical representation of ld_sysroot.  */
  char * ld_canon_sysroot;
*************** ld_config_type config;
*** 110,115 ****
--- 110,117 ----
  
  sort_type sort_section;
  
+ static const char *get_sysroot
+   (int, char **);
  static char *get_emulation
    (int, char **);
  static void set_scripts_dir
*************** main (int argc, char **argv)
*** 201,247 ****
  
    xatexit (remove_output);
  
! #ifdef TARGET_SYSTEM_ROOT_RELOCATABLE
!   ld_sysroot = make_relative_prefix (program_name, BINDIR,
! 				     TARGET_SYSTEM_ROOT);
! 
!   if (ld_sysroot)
!     {
!       struct stat s;
!       int res = stat (ld_sysroot, &s) == 0 && S_ISDIR (s.st_mode);
! 
!       if (!res)
! 	{
! 	  free (ld_sysroot);
! 	  ld_sysroot = NULL;
! 	}
!     }
! 
!   if (! ld_sysroot)
      {
!       ld_sysroot = make_relative_prefix (program_name, TOOLBINDIR,
! 					 TARGET_SYSTEM_ROOT);
! 
!       if (ld_sysroot)
  	{
! 	  struct stat s;
! 	  int res = stat (ld_sysroot, &s) == 0 && S_ISDIR (s.st_mode);
! 
! 	  if (!res)
! 	    {
! 	      free (ld_sysroot);
! 	      ld_sysroot = NULL;
! 	    }
  	}
      }
- 
-   if (! ld_sysroot)
- #endif
-     ld_sysroot = TARGET_SYSTEM_ROOT;
- 
-   if (ld_sysroot && *ld_sysroot)
-     ld_canon_sysroot = lrealpath (ld_sysroot);
- 
    if (ld_canon_sysroot)
      ld_canon_sysroot_len = strlen (ld_canon_sysroot);
    else
--- 203,220 ----
  
    xatexit (remove_output);
  
!   /* Set up the sysroot directory.  */
!   ld_sysroot = get_sysroot (argc, argv);
!   if (*ld_sysroot)
      {
!       if (*TARGET_SYSTEM_ROOT == 0)
  	{
! 	  einfo ("%P%F: this linker was not configured to use sysroots");
! 	  ld_sysroot = "";
  	}
+       else
+ 	ld_canon_sysroot = lrealpath (ld_sysroot);
      }
    if (ld_canon_sysroot)
      ld_canon_sysroot_len = strlen (ld_canon_sysroot);
    else
*************** main (int argc, char **argv)
*** 587,592 ****
--- 560,610 ----
    return 0;
  }
  
+ /* If the configured sysroot is relocatable, try relocating it based on
+    default prefix FROM.  Return the relocated directory if it exists,
+    otherwise return null.  */
+ 
+ static char *
+ get_relative_sysroot (const char *from ATTRIBUTE_UNUSED)
+ {
+ #ifdef TARGET_SYSTEM_ROOT_RELOCATABLE
+   char *path;
+   struct stat s;
+ 
+   path = make_relative_prefix (program_name, from, TARGET_SYSTEM_ROOT);
+   if (path)
+     {
+       if (stat (path, &s) == 0 && S_ISDIR (s.st_mode))
+ 	return path;
+       free (path);
+     }
+ #endif
+   return 0;
+ }
+ 
+ /* Return the sysroot directory.  Return "" if no sysroot is being used.  */
+ 
+ static const char *
+ get_sysroot (int argc, char **argv)
+ {
+   int i;
+   const char *path;
+ 
+   for (i = 1; i < argc; i++)
+     if (strncmp (argv[i], "--sysroot=", strlen ("--sysroot=")) == 0)
+       return argv[i] + strlen ("--sysroot=");
+ 
+   path = get_relative_sysroot (BINDIR);
+   if (path)
+     return path;
+ 
+   path = get_relative_sysroot (TOOLBINDIR);
+   if (path)
+     return path;
+ 
+   return TARGET_SYSTEM_ROOT;
+ }
+ 
  /* We need to find any explicitly given emulation in order to initialize the
     state that's needed by the lex&yacc argument parser (parse_args).  */
  
Index: ld/lexsup.c
===================================================================
RCS file: /cvs/src/src/ld/lexsup.c,v
retrieving revision 1.81
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.81 lexsup.c
*** ld/lexsup.c	15 Nov 2004 23:21:27 -0000	1.81
--- ld/lexsup.c	18 Jan 2005 15:40:13 -0000
*************** enum option_values
*** 71,76 ****
--- 71,77 ----
    OPTION_DEFSYM,
    OPTION_DEMANGLE,
    OPTION_DYNAMIC_LINKER,
+   OPTION_SYSROOT,
    OPTION_EB,
    OPTION_EL,
    OPTION_EMBEDDED_RELOCS,
*************** static const struct ld_option ld_options
*** 233,238 ****
--- 234,241 ----
    { {"library-path", required_argument, NULL, 'L'},
      'L', N_("DIRECTORY"), N_("Add DIRECTORY to library search path"),
      TWO_DASHES },
+   { {"sysroot=<DIRECTORY>", required_argument, NULL, OPTION_SYSROOT},
+     '\0', NULL, N_("Override the default sysroot location"), TWO_DASHES },
    { {NULL, required_argument, NULL, '\0'},
      'm', N_("EMULATION"), N_("Set emulation"), ONE_DASH },
    { {"print-map", no_argument, NULL, 'M'},
*************** parse_args (unsigned argc, char **argv)
*** 747,752 ****
--- 750,758 ----
  	case OPTION_DYNAMIC_LINKER:
  	  command_line.interpreter = optarg;
  	  break;
+ 	case OPTION_SYSROOT:
+ 	  /* Already handled in ldmain.c.  */
+ 	  break;
  	case OPTION_EB:
  	  command_line.endian = ENDIAN_BIG;
  	  break;
Index: ld/ld.texinfo
===================================================================
RCS file: /cvs/src/src/ld/ld.texinfo,v
retrieving revision 1.133
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.133 ld.texinfo
*** ld/ld.texinfo	26 Nov 2004 09:42:03 -0000	1.133
--- ld/ld.texinfo	18 Jan 2005 15:40:22 -0000
*************** many relocations.  @var{count} defaults 
*** 1504,1509 ****
--- 1504,1515 ----
  Compute and display statistics about the operation of the linker, such
  as execution time and memory usage.
  
+ @kindex --sysroot
+ @item --sysroot=@var{directory}
+ Use @var{directory} as the location of the sysroot, overriding the
+ configure-time default.  This option is only supported by linkers
+ that were configured using @option{--with-sysroot}.
+ 
  @kindex --traditional-format
  @cindex traditional format
  @item --traditional-format
Index: ld/NEWS
===================================================================
RCS file: /cvs/src/src/ld/NEWS,v
retrieving revision 1.55
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.55 NEWS
*** ld/NEWS	19 Nov 2004 09:31:53 -0000	1.55
--- ld/NEWS	18 Jan 2005 15:40:22 -0000
***************
*** 1,5 ****
--- 1,9 ----
  -*- text -*-
  
+ * A new command-line option, --sysroot, can be used to override the
+   default sysroot location.  It only applies to toolchains that were
+   configured using --with-sysroot.
+ 
  * New linker script functions: ORIGIN() and LENGTH() which return information
    about a specified memory region.
  

Re: PATCH: --sysroot-suffix

[Daniel Jacobowitz]

On Tue, Jan 18, 2005 at 03:53:48PM +0000, Richard Sandiford wrote:
> OK, the concensus seemed to be that a --sysroot flag was worth having,
> and that it would be nice to support it on non-sysrooted linkers too,
> but that it wasn't necessary for the first cut.
> 
> Here's a cleaned-up version of the patch, this time with docs and
> NEWS item.  Tested on mips-linux-gnu.  OK to install?

There seems to be plenty of precedent for --argument=value in GNU ld,
but the documentation says:

       Arguments  to  multiple-letter  options  must  either  be
       separated from the option name by an equals sign, or be given as
       separate arguments immediately following the option that
       requires them.  For example, --trace-symbol foo and
       --trace-symbol=foo are equivalent.  Unique abbreviations of
       the names of multiple-letter options are accepted.

I always use the space, personally, so I'd be pretty confused by the
dummy option handling; how about supporting that too?

I'm somewhat surprised the hack is necessary, but I'm sure you're right
:-)

As a first cut to fix the configury option, we could always enable
sysroots for native tools.  Right now the differences in linker script
generation are only the addition of the '=' marker, the use of
NATIVE_LIB_DIRS (already enabled for natives), and something
complicated for $(tooldir) that I can't quite remember the purpose
of...

-- 
Daniel Jacobowitz

Re: PATCH: --sysroot-suffix

[Richard Sandiford]

Daniel Jacobowitz <drow@false.org> writes:
> There seems to be plenty of precedent for --argument=value in GNU ld,
> but the documentation says:
>
>        Arguments  to  multiple-letter  options  must  either  be
>        separated from the option name by an equals sign, or be given as
>        separate arguments immediately following the option that
>        requires them.  For example, --trace-symbol foo and
>        --trace-symbol=foo are equivalent.  Unique abbreviations of
>        the names of multiple-letter options are accepted.
>
> I always use the space, personally, so I'd be pretty confused by the
> dummy option handling; how about supporting that too?

It's much harder to do that in gcc though, and it's not really
consistent with gcc option handling, which usually enforces the
"--arg=value" form.  I thought it would be confusing to support
"--arg value" in one but not the other, especially given that
the interface is identical otherwise.

Richard

Re: PATCH: --sysroot-suffix

[Daniel Jacobowitz]

On Tue, Jan 18, 2005 at 04:10:13PM +0000, Richard Sandiford wrote:
> Daniel Jacobowitz <drow@false.org> writes:
> > There seems to be plenty of precedent for --argument=value in GNU ld,
> > but the documentation says:
> >
> >        Arguments  to  multiple-letter  options  must  either  be
> >        separated from the option name by an equals sign, or be given as
> >        separate arguments immediately following the option that
> >        requires them.  For example, --trace-symbol foo and
> >        --trace-symbol=foo are equivalent.  Unique abbreviations of
> >        the names of multiple-letter options are accepted.
> >
> > I always use the space, personally, so I'd be pretty confused by the
> > dummy option handling; how about supporting that too?
> 
> It's much harder to do that in gcc though, and it's not really
> consistent with gcc option handling, which usually enforces the
> "--arg=value" form.  I thought it would be confusing to support
> "--arg value" in one but not the other, especially given that
> the interface is identical otherwise.

I think violating the ld manual's description is a bigger problem.
It says that "--sysroot=foo" and "--sysroot foo" should be
equivalent...

-- 
Daniel Jacobowitz

Re: PATCH: --sysroot-suffix

[Alexandre Oliva]

On Jan 18, 2005, Richard Sandiford <rsandifo@redhat.com> wrote:

> Daniel Jacobowitz <drow@false.org> writes:
>> There seems to be plenty of precedent for --argument=value in GNU ld,
>> but the documentation says:
>> 
>> Arguments  to  multiple-letter  options  must  either  be
>> separated from the option name by an equals sign, or be given as
>> separate arguments immediately following the option that
>> requires them.  For example, --trace-symbol foo and
>> --trace-symbol=foo are equivalent.  Unique abbreviations of
>> the names of multiple-letter options are accepted.
>> 
>> I always use the space, personally, so I'd be pretty confused by the
>> dummy option handling; how about supporting that too?

> It's much harder to do that in gcc though, and it's not really
> consistent with gcc option handling, which usually enforces the
> "--arg=value" form.

But that's a bug in GCC.  GNU tools are supposed to handle both
forms.

Also, I'd value command-line handling consistency of a tool in itself
over consistency with other tools.  I.e., if GNU ld handles both
forms in general, let's make it handle both in this particular case,
even if GCC doesn't handle them both.

A counter-argument would get me to bring up the fact that GCC handles
-fwhatever as equivalent to --whatever, so how come the
counter-argument doesn't push for -fsysroot= as well? :-)

-- 
Alexandre Oliva             http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}

Re: PATCH: --sysroot-suffix

[Mark Mitchell]

Richard Sandiford wrote:
> Daniel Jacobowitz <drow@false.org> writes:
> 
>>There seems to be plenty of precedent for --argument=value in GNU ld,
>>but the documentation says:
>>
>>       Arguments  to  multiple-letter  options  must  either  be
>>       separated from the option name by an equals sign, or be given as
>>       separate arguments immediately following the option that
>>       requires them.  For example, --trace-symbol foo and
>>       --trace-symbol=foo are equivalent.  Unique abbreviations of
>>       the names of multiple-letter options are accepted.
>>
>>I always use the space, personally, so I'd be pretty confused by the
>>dummy option handling; how about supporting that too?
> 
> 
> It's much harder to do that in gcc though, and it's not really
> consistent with gcc option handling, which usually enforces the
> "--arg=value" form.  I thought it would be confusing to support
> "--arg value" in one but not the other, especially given that
> the interface is identical otherwise.

In my patch, I rejected the scan over argv[] to look for the option 
because you will get fooled by something like "-L --sysroot".  That's 
why I required the --sysroot-suffix option to be the first option on the 
command line.  I understand the usability argument in favor of your 
patch, and the fact that using --sysroot as a name is pretty unlikely, 
but I did want to raise the issue.

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: PATCH: --sysroot-suffix

[Richard Sandiford]

Mark Mitchell <mark@codesourcery.com> writes:
> In my patch, I rejected the scan over argv[] to look for the option
> because you will get fooled by something like "-L --sysroot".  That's
> why I required the --sysroot-suffix option to be the first option on the
> command line.  I understand the usability argument in favor of your
> patch, and the fact that using --sysroot as a name is pretty unlikely,
> but I did want to raise the issue.

OK.  I'm beginning to wonder if we should just go with your patch.
It looks like mine isn't acceptable as things stand and I'm not
sure how soon I'll have time to bring it up to scratch.

Richard

Re: PATCH: --sysroot-suffix

[Mark Mitchell]

Richard Sandiford wrote:
> Mark Mitchell <mark@codesourcery.com> writes:
> 
>>In my patch, I rejected the scan over argv[] to look for the option
>>because you will get fooled by something like "-L --sysroot".  That's
>>why I required the --sysroot-suffix option to be the first option on the
>>command line.  I understand the usability argument in favor of your
>>patch, and the fact that using --sysroot as a name is pretty unlikely,
>>but I did want to raise the issue.
> 
> 
> OK.  I'm beginning to wonder if we should just go with your patch.
> It looks like mine isn't acceptable as things stand and I'm not
> sure how soon I'll have time to bring it up to scratch.

 From my point of view, I'd just like to see *something* get in. :-)  I 
certainly don't want to let perfect be the enemy of good; I'm happy with 
either patch, or reasonable variants of either.  Perhaps the binutils 
maintainers could suggest a direction; I'd be happy to massage either 
patch in a pre-approved direction.

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: PATCH: --sysroot-suffix

[Alan Modra]

On Tue, Jan 18, 2005 at 09:24:37AM -0800, Mark Mitchell wrote:
> In my patch, I rejected the scan over argv[] to look for the option 
> because you will get fooled by something like "-L --sysroot".

I'm happy enough with Richard's patch.  If something like the above
becomes an issue, we can fix it by rewriting ld's arg parser.  See
lexsup.c:parse_args for more examples of the problem Mark is raising.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: PATCH: --sysroot-suffix

[Mark Mitchell]

Alan Modra wrote:
> On Tue, Jan 18, 2005 at 09:24:37AM -0800, Mark Mitchell wrote:
> 
>>In my patch, I rejected the scan over argv[] to look for the option 
>>because you will get fooled by something like "-L --sysroot".
> 
> 
> I'm happy enough with Richard's patch.  If something like the above
> becomes an issue, we can fix it by rewriting ld's arg parser.  See
> lexsup.c:parse_args for more examples of the problem Mark is raising.

Cool.  I'll look forward to Richard checking in his patch.

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: PATCH: --sysroot-suffix

[Richard Sandiford]

Alan Modra <amodra@bigpond.net.au> writes:
> On Tue, Jan 18, 2005 at 09:24:37AM -0800, Mark Mitchell wrote:
>> In my patch, I rejected the scan over argv[] to look for the option 
>> because you will get fooled by something like "-L --sysroot".
>
> I'm happy enough with Richard's patch.  If something like the above
> becomes an issue, we can fix it by rewriting ld's arg parser.

Thanks, patch installed.

I realise the objections/potential problems that Daniel, Alex & Mark
raised are good ones, and I will try to find time to make --sysroot a
first-class citizen as far as option handling is concerned (including
handling it in the normal getopt way).  I can't promise anything soon
though.

Richard

Re: PATCH: --sysroot-suffix

[Mark Mitchell]

Richard Sandiford wrote:
> Alan Modra <amodra@bigpond.net.au> writes:
> 
>>On Tue, Jan 18, 2005 at 09:24:37AM -0800, Mark Mitchell wrote:
>>
>>>In my patch, I rejected the scan over argv[] to look for the option 
>>>because you will get fooled by something like "-L --sysroot".
>>
>>I'm happy enough with Richard's patch.  If something like the above
>>becomes an issue, we can fix it by rewriting ld's arg parser.
> 
> 
> Thanks, patch installed.

Yay!

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304