You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@stdcxx.apache.org by Martin Sebor <ms...@gmail.com> on 2011/03/05 22:59:25 UTC
patch for STDCXX-1051
Wojciech,
I've reviewed your patch for STDCXX-1051. It looks reasonable
to me. I've made a couple of minor changes (see the attached
diff):
1) Renamed EXEC_RUNNER to CFG_EXEC to match the .cfg suffix
we use for some of the configuration files (such as
GNUMakefile.cfg), and left it unset by default to avoid
unnecessarily invoking the shell by default. The extra
shell invocation can be quite expensive on systems line
CygWin and make the configuration process very slow.
2) Removed the changes to the link and compile_then_link
functions. Because of the comments within them these
were effectively commented out and had no effect.
I also don't understand why the chmod +x command is
necessary. The linker should set the executable bit after
a successful link. There is one linker (HP) that fails (or
used to) to delete the output file on failure (maybe when
when it crashes). With that linker the only way to tell
that the file is bad (other than the exit status of the
linker) is by examining the executable bit.
I've successfully tested the attached patch both ways, with
and without a CFG_EXEC script. Let me know if this works for
you.
Martin
RE: patch for STDCXX-1051
Posted by Wojciech Meyer <Wo...@arm.com>.
Hi Martin,
> I've reviewed your patch for STDCXX-1051. It looks reasonable
> to me. I've made a couple of minor changes (see the attached
> diff):
Thanks for this! :)
> 1) Renamed EXEC_RUNNER to CFG_EXEC to match the .cfg suffix
> we use for some of the configuration files (such as
> GNUMakefile.cfg), and left it unset by default to avoid
> unnecessarily invoking the shell by default. The extra
> shell invocation can be quite expensive on systems line
> CygWin and make the configuration process very slow.
Yes, I didn't know the convention, and indeed Cygwin especially on the
network is very expensive, configuration of stdcxx takes while.
> 2) Removed the changes to the link and compile_then_link
> functions. Because of the comments within them these
> were effectively commented out and had no effect.
> I also don't understand why the chmod +x command is
> necessary. The linker should set the executable bit after
> a successful link. There is one linker (HP) that fails (or
> used to) to delete the output file on failure (maybe when
> when it crashes). With that linker the only way to tell
> that the file is bad (other than the exit status of the
> linker) is by examining the executable bit.
I don't remember why I did this, but I remember it was something
that I needed to do, probably it was related somewhat to
cross-compilation.
> It is necessary in case of cross-compilation
> I've successfully tested the attached patch both ways, with
> and without a CFG_EXEC script. Let me know if this works for
> you.
Thanks! I will get back to you someday tomorrow or in the middle of this
week and let you know if that all works for me.
There will be few patches more.. I'll do my best to reduce from you
burden of veryfing and applying them.
> Martin
Wojciech
RE: patch for STDCXX-1051
Posted by Wojciech Meyer <Wo...@arm.com>.
> > Now, it would be all fine besides that stdcxx build system checks for
> > this flag here
> >
> > (..)
> > etc/config/GNUmakefile.cfg:247:
> > elif [ ! -x $$file ] ; then \
> > nm -gp $$file.o 2>&1 | grep "T *main *$$">/dev/null ; \
> > test -f $$file.o -a ! $$? -eq 0 ; \
> > else \
> > echo "./$$file">>$(LOGFILE) ; \
> > LD_LIBRARY_PATH=$$LD_LIBRARY_PATH:. ; \
> > LIBPATH=$$LIBPATH:. ; \
> > export LIBPATH LD_LIBRARY_PATH ; \
> > text=`./$$file` ; \
> > fi; \
> > (..)
> >
> > So, I needed to do it manually to ensure it does search for
> > that. Using this shell trickery everything seems to work fine.
> >
> > Let me know your thoughts.
> The change would cause problems for the HP linker. We don't
> want to break one build to make another work.
Ouch, no good.
> One way you could work around it is by writing a wrapper
> script for your linker and setting the executable bit in
> it. From a design POV, since your linker behaves in
> an "unusual" way (i.e., different than all the linkers
> stdcxx usually targets),
Yes indeed, I thought the same, maybe we should set up this flag.
I raised that to the linker person, and he said that it would be an
enhancement to actually provide a command-line switch to support it.
For time being please commit your version of the patch, and I will stick
with the wrapper script.
I will send another (bigger) patch soon. Thanks.
> I would prefer to avoid changing
> the configuration machinery this way. But if you can come
> up with a "clean" solution that improves the machinery for
> all linkers I would certainly support it.
I will try to figure out this as well.
> Martin
Thanks,
Wojciech
-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Re: patch for STDCXX-1051
Posted by Martin Sebor <ms...@gmail.com>.
On 03/08/2011 08:10 AM, Wojciech Meyer wrote:
>
>>> 2) Removed the changes to the link and compile_then_link
>>> functions. Because of the comments within them these
>>> were effectively commented out and had no effect.
>>> I also don't understand why the chmod +x command is
>>> necessary. The linker should set the executable bit after
>>> a successful link. There is one linker (HP) that fails (or
>>> used to) to delete the output file on failure (maybe when
>>> when it crashes). With that linker the only way to tell
>>> that the file is bad (other than the exit status of the
>>> linker) is by examining the executable bit.
>
>> I don't remember why I did this, but I remember it was something
>> that I needed to do, probably it was related somewhat to
>> cross-compilation.
>
> Now I know!
> The reason behind it, is that our linker which performs linking *does
> not* setup executable flag on the destination file. It's an image which
> should not be executed directly anyway.
>
> Now, it would be all fine besides that stdcxx build system checks for
> this flag here
>
> (..)
> etc/config/GNUmakefile.cfg:247:
> elif [ ! -x $$file ] ; then \
> nm -gp $$file.o 2>&1 | grep "T *main *$$">/dev/null ; \
> test -f $$file.o -a ! $$? -eq 0 ; \
> else \
> echo "./$$file">>$(LOGFILE) ; \
> LD_LIBRARY_PATH=$$LD_LIBRARY_PATH:. ; \
> LIBPATH=$$LIBPATH:. ; \
> export LIBPATH LD_LIBRARY_PATH ; \
> text=`./$$file` ; \
> fi; \
> (..)
>
> So, I needed to do it manually to ensure it does search for
> that. Using this shell trickery everything seems to work fine.
>
> Let me know your thoughts.
The change would cause problems for the HP linker. We don't
want to break one build to make another work.
One way you could work around it is by writing a wrapper
script for your linker and setting the executable bit in
it. From a design POV, since your linker behaves in
an "unusual" way (i.e., different than all the linkers
stdcxx usually targets), I would prefer to avoid changing
the configuration machinery this way. But if you can come
up with a "clean" solution that improves the machinery for
all linkers I would certainly support it.
Martin
RE: patch for STDCXX-1051
Posted by Wojciech Meyer <Wo...@arm.com>.
> > 2) Removed the changes to the link and compile_then_link
> > functions. Because of the comments within them these
> > were effectively commented out and had no effect.
> > I also don't understand why the chmod +x command is
> > necessary. The linker should set the executable bit after
> > a successful link. There is one linker (HP) that fails (or
> > used to) to delete the output file on failure (maybe when
> > when it crashes). With that linker the only way to tell
> > that the file is bad (other than the exit status of the
> > linker) is by examining the executable bit.
> I don't remember why I did this, but I remember it was something
> that I needed to do, probably it was related somewhat to
> cross-compilation.
Now I know!
The reason behind it, is that our linker which performs linking *does
not* setup executable flag on the destination file. It's an image which
should not be executed directly anyway.
Now, it would be all fine besides that stdcxx build system checks for
this flag here
(..)
etc/config/GNUmakefile.cfg:247:
elif [ ! -x $$file ] ; then \
nm -gp $$file.o 2>&1 | grep "T *main *$$" >/dev/null ; \
test -f $$file.o -a ! $$? -eq 0 ; \
else \
echo "./$$file" >>$(LOGFILE) ; \
LD_LIBRARY_PATH=$$LD_LIBRARY_PATH:. ; \
LIBPATH=$$LIBPATH:. ; \
export LIBPATH LD_LIBRARY_PATH ; \
text=`./$$file` ; \
fi; \
(..)
So, I needed to do it manually to ensure it does search for
that. Using this shell trickery everything seems to work fine.
Let me know your thoughts.
Thanks!
Wojciech