On 26-Sep-22 11:14, Daniel Stenberg via curl-library wrote:
Hello!

In my new PR[*], I am ripping out the old checks from configure and cmake that try to figure out what argument and return types recv and send want. The new system instead a) defaults to POSIX and b) offers functypes.h that can provide
overrides for system that don't use the POSIX types.

The reason for this is of course that the checks are very brute-force and can end up taking a long time and for all non-cmake/configure builds we already
have the types provided anyway.

This change removes crufty logic and simplifies the approach. I think it feels
cleaner.

 BUT

There is an obvious risk that this change will break the build on systems
outside of our autobuilds and CI setups. I don't think we can avoid it, but I still think this is a worthwhile change. Fixing the build will be easy for whoever runs into it and will in a typical case consist of adding lines to
functypes.h in the same style as other platforms already have them.

(The old check also checks select() types, but since no current code actually
use those types the new system does not care about those.)

Comment here or in the PR!

PR: https://github.com/curl/curl/pull/9592

This seems to be an unnecessary change; nothing is currently broken.

I'm not a cmake expert, but configure is almost by definition a library of "brute force" and ugly code.  It's function is to avoid having builders edit .h and make files...so this PR is essentially a step backwards for whatever non-POSIX platforms the tests support.

If curl goes down this path, where does it stop?  I think a fair argument could be made that all of configure/cmake could be replaced by "a few simple manual edits to a default config.h and Makefiles".  (Though they'd evolve to be more than a few and messy...)  While I don't think you want to go all the way there, what test do you have in mind for similar proposals in the future?  Is there a standard for how much "brute force" or "ugliness" justifies creating work for some users?

With respect to performance: with configure  I use a cache file (and recommend it - see --cache-file); this means that the test compiles aren't run except when the cache is rebuilt, which is rare.  I assume/hope that cmake has a similar facility.

Breaking something for the sake of a little less ugly code doesn't seem like the right thing to do.

Rather, it seems to be a solution in search of - or creating - a problem.

If this code was consuming a lot of maintenance effort or an uncacheable significant performance issue, I'd be more sympathetic.  However, I'd look for solutions that don't create a burden for users.

Removing the checks for select() types seems reasonable since no-one uses the result.  There probably are more tests with no consumers that could be removed; most project accumulate them...

Timothe Litt
ACM Distinguished Engineer
--------------------------
This communication may not represent the ACM or my employer's views,
if any, on the matters discussed.

Attachment: OpenPGP_signature
Description: OpenPGP digital signature

-- 
Unsubscribe: https://lists.haxx.se/listinfo/curl-library
Etiquette:   https://curl.se/mail/etiquette.html

Reply via email to