This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [ping] Re: [patch v2] support for multiarch systems
- From: Ian Lance Taylor <iant at google dot com>
- To: Matthias Klose <doko at ubuntu dot com>
- Cc: gcc-patches at gcc dot gnu dot org, "Joseph S. Myers" <joseph at codesourcery dot com>, Paolo Bonzini <bonzini at gnu dot org>, Thomas Schwinge <thomas at codesourcery dot com>
- Date: Fri, 2 Nov 2012 10:37:45 -0700
- Subject: Re: [ping] Re: [patch v2] support for multiarch systems
- References: <4E501045.40102@ubuntu.com> <4FF9D5F7.6090505@ubuntu.com> <5020EE69.4020102@ubuntu.com> <CAKOQZ8w2P5C4-jQMcMZKFLpssUx-jsXdbdmiAY3PrfrhQnqs-w@mail.gmail.com> <5093CA0D.8010909@ubuntu.com>
On Fri, Nov 2, 2012 at 6:26 AM, Matthias Klose <doko@ubuntu.com> wrote:
>
>> +ifeq ($(enable_multiarch),yes)
>> + if_multiarch = $(1)
>> +else ifeq ($(enable_multiarch),auto-detect)
>> + if_multiarch = $(if $(wildcard $(shell echo
>> $(SYSTEM_HEADER_DIR))/../../usr/lib/*/crti.o),$(1))
>> +else
>> + if_multiarch =
>> +endif
>>
>> This is nicely tricky but I don't understand why you are doing this in
>> such a confusing way. Why not simply set the names in config.gcc?
>> What information do you have at make time that you don't have at
>> configure time?
>
>
> this is for the auto case. SYSTEM_HEADER_DIR is not accessible/available
> from
> config.gcc. This is the same approach as taken for MULTILIB_OSDDIRNAMES.
> Also setting the names for the multiarch directories in the same place as
> done for for the MULTILIB_OSDIRNAMES seems to be better choice.
SYSTEM_HEADER_DIR is available in gcc/configure.ac, and you could do
the $(if $(wildcard $(shell echo
>> $(SYSTEM_HEADER_DIR))/../../usr/lib/*/crti.o) there. I can't see any reason to do it at make time rather than configure time.
> + if (*q2 == ':')
> + end = q2;
Don't reuse end here. Use a new variable.
Please update the comment for set_multilib_dir. It's not a very good
comment, but it does describe the syntax, and you seem to be changing
the syntax.
> - if (this_path[0] == '.' && this_path[1] == ':')
> + if (this_path[0] == '.' && this_path[1] == ':' && this_path[2] != ':')
The existing comment here is reasonably clear. Please update the
comment to show why this change is correct.
> +Specify wether to enable or disable multiarch support. The default is
s/wether/whether/
> +to detect for glibc start files in a multiarch location, and enable it
s/detect/check/ or something along those lines.
Ian