Hi,
On Feb 3, 2013, at 7:33 PM, Alexandre Rostovtsev wrote:
Commit c14bdaf1 made winebuild use Clang to assemble if found.
However, just because a user has some version of Clang installed, it does not mean that she wants to use Clang to assemble Wine. For example, a user who has both Clang and GAS installed may want to use GAS to avoid textrels (see https://bugs.gentoo.org/show_bug.cgi?id=455308).
Long term, that should really get fixed in LLVM/Clang. I'll file a PR (if one wasn't already).
This patch allows the user to override which assembler gets used by exporting CCAS at Wine configure time; the name CCAS was chosen for compatibility with automake's standard AM_PROG_AS macro.
I wrote the patch that prompted this one. And I'll freely admit, this is why I wanted to limit that patch to Mac OS--to minimize fallout on other platforms (like, obviously, Gentoo :).
configure | 106 ++++++++++++++++++++++++++++++++++++++++++++ configure.ac | 4 ++ tools/winebuild/Makefile.in | 5 ++- tools/winebuild/utils.c | 12 +++++ 4 files changed, 126 insertions(+), 1 deletion(-)
diff --git a/configure b/configure index e3253ee..d0b7777 100755 --- a/configure +++ b/configure @@ -732,6 +732,8 @@ FLEX TOOLSDIR WOW64_DISABLE TARGETFLAGS +ac_ct_CCAS +CCAS CPPBIN ac_ct_CXX CXXFLAGS @@ -861,6 +863,7 @@ CPPFLAGS CXX CXXFLAGS CCC +CCAS CPP XMKMF'
@@ -1549,6 +1552,7 @@ Some influential environment variables: you have headers in a nonstandard directory <include dir> CXX C++ compiler command CXXFLAGS C++ compiler flags
- CCAS Assembler command CPP C preprocessor XMKMF Path to xmkmf, Makefile generator for X Window System
@@ -4075,6 +4079,108 @@ cat >>confdefs.h <<_ACEOF _ACEOF
+if test -n "$ac_tool_prefix"; then
- for ac_prog in clang gas as
- do
- # Extract the first word of "$ac_tool_prefix$ac_prog", so it can be a program name with args.
+set dummy $ac_tool_prefix$ac_prog; ac_word=$2 +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5 +$as_echo_n "checking for $ac_word... " >&6; } +if ${ac_cv_prog_CCAS+:} false; then :
- $as_echo_n "(cached) " >&6
+else
- if test -n "$CCAS"; then
- ac_cv_prog_CCAS="$CCAS" # Let the user override the test.
+else +as_save_IFS=$IFS; IFS=$PATH_SEPARATOR +for as_dir in $PATH +do
- IFS=$as_save_IFS
- test -z "$as_dir" && as_dir=.
- for ac_exec_ext in '' $ac_executable_extensions; do
- if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then
- ac_cv_prog_CCAS="$ac_tool_prefix$ac_prog"
- $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext" >&5
- break 2
- fi
+done
- done
+IFS=$as_save_IFS
+fi +fi +CCAS=$ac_cv_prog_CCAS +if test -n "$CCAS"; then
- { $as_echo "$as_me:${as_lineno-$LINENO}: result: $CCAS" >&5
+$as_echo "$CCAS" >&6; } +else
- { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; } +fi
- test -n "$CCAS" && break
- done
+fi +if test -z "$CCAS"; then
- ac_ct_CCAS=$CCAS
- for ac_prog in clang gas as
+do
- # Extract the first word of "$ac_prog", so it can be a program name with args.
+set dummy $ac_prog; ac_word=$2 +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5 +$as_echo_n "checking for $ac_word... " >&6; } +if ${ac_cv_prog_ac_ct_CCAS+:} false; then :
- $as_echo_n "(cached) " >&6
+else
- if test -n "$ac_ct_CCAS"; then
- ac_cv_prog_ac_ct_CCAS="$ac_ct_CCAS" # Let the user override the test.
+else +as_save_IFS=$IFS; IFS=$PATH_SEPARATOR +for as_dir in $PATH +do
- IFS=$as_save_IFS
- test -z "$as_dir" && as_dir=.
- for ac_exec_ext in '' $ac_executable_extensions; do
- if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then
- ac_cv_prog_ac_ct_CCAS="$ac_prog"
- $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext" >&5
- break 2
- fi
+done
- done
+IFS=$as_save_IFS
+fi +fi +ac_ct_CCAS=$ac_cv_prog_ac_ct_CCAS +if test -n "$ac_ct_CCAS"; then
- { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_ct_CCAS" >&5
+$as_echo "$ac_ct_CCAS" >&6; } +else
- { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; } +fi
- test -n "$ac_ct_CCAS" && break
+done
- if test "x$ac_ct_CCAS" = x; then
- CCAS=""$CC""
- else
- case $cross_compiling:$ac_tool_warned in
+yes:) +{ $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: using cross tools not prefixed with host triplet" >&5 +$as_echo "$as_me: WARNING: using cross tools not prefixed with host triplet" >&2;} +ac_tool_warned=yes ;; +esac
- CCAS=$ac_ct_CCAS
- fi
+fi
case $host in *-darwin*) if test "x$enable_win64" = "xyes" diff --git a/configure.ac b/configure.ac index c93cd08..760f325 100644 --- a/configure.ac +++ b/configure.ac @@ -126,6 +126,10 @@ dnl We can't use AC_PROG_CPP for winegcc, it uses by default $(CC) -E AC_CHECK_TOOL(CPPBIN,cpp,cpp) AC_DEFINE_UNQUOTED(EXEEXT,["$ac_exeext"],[Define to the file extension for executables.])
+dnl Analogous to AM_PROG_AS +AC_ARG_VAR([CCAS],[Assembler command]) +AC_CHECK_TOOLS(CCAS,[clang gas as],["$CC"])
If the user doesn't set CCAS--which she doesn't the majority of the time--configure will pick up the first of {clang, gas, as} in the PATH. Are you sure that's what you want?
One of the earlier versions of the "Use Clang to assemble" patch attempted to detect Clang in the PATH, and use it if it was there. I was told that winebuild should detect the binary to use at runtime instead. Perhaps we should just not define CCAS if the user didn't specify it. Then below, you can wrap the block you added in an #ifdef.
AJ may have more to say about this.
case $host in *-darwin*) if test "x$enable_win64" = "xyes" diff --git a/tools/winebuild/Makefile.in b/tools/winebuild/Makefile.in index 2017129..3fe47a3 100644 --- a/tools/winebuild/Makefile.in +++ b/tools/winebuild/Makefile.in @@ -1,4 +1,7 @@ -DEFS = -D__WINESRC__ $(EXTRADEFS) +DEFS = \
- -DCCAS=""@CCAS@"" \
- -D__WINESRC__ \
- $(EXTRADEFS)
PROGRAMS = winebuild$(EXEEXT) MANPAGE = winebuild.man diff --git a/tools/winebuild/utils.c b/tools/winebuild/utils.c index 262ff3a..1c5f918 100644 --- a/tools/winebuild/utils.c +++ b/tools/winebuild/utils.c @@ -352,6 +352,18 @@ struct strarray *get_as_command(void) static int as_is_clang = 0; struct strarray *args = strarray_init();
- if (!as_command && strlen( CCAS ))
- {
struct stat st;if (!stat( CCAS, &st ))as_command = CCAS;elseas_command = find_tool( CCAS, NULL );if (as_command && strstr( as_command, "clang" )) as_is_clang = 1;
You may recall that one of the earlier versions (later than the one that detected Clang in configure) used strstr(3) to detect Clang; I was then told not to use it. I suspect this is because some systems' strstr(3) exhibits quadratic-time behavior. I don't know if you can avoid that in this case, though. Maybe you can just grab the basename and strip off the CTARGET prefix; then you should just be able to do a strcmp(3). Or am I missing something?
More generally, calling the variable "CCAS" implies that any old C compiler driver that knows how to assemble can be specified. If the user specifies a compiler that isn't Clang--GCC, for example--she'll get the wrong behavior. In particular, the 'as_is_clang' variable is used below to pass the right switches and options to Clang. Presumably we'd want to do that for the GCC case, too.
- }
- if (!as_command) { as_command = find_tool( "clang", NULL );
-- 1.8.1.2
I'm sorry for any inconvenience I might have caused, and I hope we can get this fixed soon.
Chip