windows: fail early with a missing windres in autotools #9781
Conversation
vszakats
changed the title
vszakats
force-pushed
the
ac-windres-2
branch
3 times, most recently
from
c1e722d
to
cb33284
Compare
Previous solution assumed that when building a Windows target, `RC` tool is available and auto-detected by autotools. Sometime this is not true. Rename and extend the `OS_WINDOWS` conditional to also check if `RC` is set. Effectively this patch replaces a hard error with just not building the resources. The correct fix is (possibly) to set the TRIPLET to a valid one (so that auto-detection works correctly), and a workaround is to manually point the `RC` envvar to a working copy of `windres`. [^1]: https://tg.st/u/ad68a31462958a3ef0c801258bf5e8dde310d9fe062416d37d42f1e996af6bdb.txt Follow-up to 6de7322 Ref: https://curl.se/mail/lib-2022-10/0049.html Closes curl#9781
vszakats
force-pushed
the
ac-windres-2
branch
from
cb33284
to
fcfc1b9
Compare
vszakats
changed the title
|
After iterating through a few options, I'm siding that this is a local configuration issue. Unless there is a valid environment that misses a |
|
@sithglan's error from the mailing list is below for reference. I think that if I saw this error it would not be immediately apparent to me what is happening. Is it worthwhile to test if rc tool exists in the configuration and fail with "rc required" or something, or do you think this is just specific to him? I haven't heard anything else about this. |
|
The way it fails is definitely confusing. I'm not entirely sure what's happening in that env, but the cross-compiler prefix seems wrong ( From the log it seems a similar issue happened in the past with OpenSSL, hence the explicit curl How could we improve that, to be actually noticable? |
|
If it's required then AC_MSG_ERROR but now I'm not so sure because what you describe sounds more like user error. @sithglan |
vszakats
changed the title
vszakats
force-pushed
the
ac-windres-2
branch
from
cf9218a
to
c156c04
Compare
|
Hello Viktor,
The way it fails is definitely confusing. I'm not entirely sure what's
happening in that env, but the cross-compiler prefix seems wrong
(`mingw32-`). This may be a reason why several autotools
auto-detection fail (e.g. `checking for mingw32-ar... no`).
`i686-w64-mingw32-` is the correct value there, which is also the
expected one for x86.
thank you, that indeed fixed my issue:
(infra) [/srv/kvm/scratch/vlconnect] git diff compile_dependencies.sh
diff --git a/compile_dependencies.sh b/compile_dependencies.sh
index cdd0d27..cd8da42 100644
--- a/compile_dependencies.sh
+++ b/compile_dependencies.sh
@@ -85,7 +85,7 @@ build_curl_openssl()
autoreconf -fi
CC=i686-w64-mingw32-gcc ./configure CFLAGS="${CFLAGS}" \
--with-ssl=`pwd`/../local/win32/openssl \
- --host=mingw32 \
+ --host=i686-w64-mingw32 \
--prefix=`pwd`/../local/win32/openssl \
$LIBCURL_COMMON_PARAMS
make -j ${HECS}
From the log it seems a similar issue happened in the past with
OpenSSL, hence the explicit `RC=` line in the make invocation for it:
`make -j ${HECS} RC=/usr/bin/i686-w64-mingw32-windres`. This is
missing from the curl `make` command, and adding it would fix it.
Specifying the RC= line does not hurt but also was not necessary after
specifying the right 'host' value.
Cheers,
Thomas
|
|
Thank you Thomas for your logs and report, I'm happy it worked out fine. I think this patch would improve error handling, so it'd be nice to merge. We're close to the release, and this isn't a showstopper, so maybe after the release would be the best. |
vszakats
added
feature-window
vszakats
changed the title
`windres` is not always auto-detected by autotools when building for Windows. When this happened, the build failed with a confusing error due to the empty `RC` command: ``` /bin/bash ../libtool --tag=RC --mode=compile -I../include -DCURL_EMBED_MANIFEST -i curl.rc -o curl.o [...] Usage: /sandbox/curl/libtool [OPTION]... [MODE-ARG]... Try 'libtool --help' for more information. libtool: error: unrecognised option: '-I../include' ``` Improve this by verifying if `RC` is set, and fail with a clear error otherwise. Follow-up to 6de7322 Ref: https://curl.se/mail/lib-2022-10/0049.html Reported-by: Thomas Glanzmann Closes curl#9781
vszakats
force-pushed
the
ac-windres-2
branch
from
119572c
to
a22ce1a
Compare
vszakats
removed
the
feature-window
windresis not always auto-detected by autotools when building for Windows. When this happened, the build failed with a confusing error due to the emptyRCcommand:Improve this by verifying if
RCis set, and fail with a clear error otherwise.Follow-up to 6de7322
Ref: https://curl.se/mail/lib-2022-10/0049.html
Reported-by: Thomas Glanzmann
Closes #9781