Correct cpu features detection during cross-compiation

aol-nnov commented 2 weeks ago

Closes https://github.com/pjsip/pjproject/issues/4150

CLAassistant commented 2 weeks ago

All committers have signed the CLA.

aol-nnov commented 2 weeks ago

You also need to commit the modified aconfigure

Should I include aclocal.m4 as well? AX_CHECK_COMPILE_FLAG is an external macro, provided by autoconf-archive package in Debian.

sauwming commented 2 weeks ago

You also need to commit the modified aconfigure Should I include aclocal.m4 as well? AX_CHECK_COMPILE_FLAG is an external macro, provided by autoconf-archive package in Debian.

Then this could be a problem. I check my Mac machine and it doesn't have autoconf-archive installed by default. Try to replace it with readily available macros instead, such as AC_COMPILE... or something else.

aol-nnov commented 2 weeks ago

Then this could be a problem

But why? If I run aclocal, all external macro are copied over to self-sufficient aclocal.m4 and I can include it into the commit as well.

aol-nnov commented 2 weeks ago

Also, autoconf-archive is available on Mac via HomeBrew: https://formulae.brew.sh/formula/autoconf-archive#default

aol-nnov commented 2 weeks ago

@sauwming Let's do another round. I've implemented your comments.

Also, please check aclocal.m4 if it works for you that way.

sauwming commented 2 weeks ago

The aclocal.m4 seems to work okay for me, as well as on the CI machines. Although quite frankly, it still leaves me worried.

==== aclocal.m4 itself is not inherently portable across different environments without considering some nuances.

Portability Considerations:

Generated by aclocal: aclocal.m4 is typically generated on the build machine where aclocal runs. The macros it includes are influenced by the m4 macro files available on that machine.
Environment-Specific Dependencies: If aclocal.m4 depends on custom or third-party macros (e.g., from the Autoconf Archive), it may reference macros that are not present on another system. This can lead to portability issues when transferring the aclocal.m4 file to a different environment without the necessary macro definitions.
Not a Replacement for Autoconf and Automake: While you can transfer aclocal.m4 to another system, it’s not a full replacement for running aclocal, automake, or autoconf. The environment where the configure script is regenerated must have the necessary tools and macro files.

Conclusion: aclocal.m4 is not highly portable by itself because it relies on macros defined by the environment in which it was generated. For distributing software, ensure that you distribute the configure script and any other build system files needed for an end-user to compile the code without requiring Autotools.

====

I know I can install autoconf-archive, but I don't want to do this on every machine and I don't wish to make this a requirement, especially if it's not really necessary.

So, I would prefer if we check the availability of compile flags using the standard macros instead, like everywhere else in the currently existing configure script.

aol-nnov commented 2 weeks ago

So, you want me to copy and paste aclocal.m4 contents inside the aconfigure.ac? Okay, will do that tomorrow.

I'm out for today and will be back online in next 12 hours or so.

Thank you for the cooperation!

sauwming commented 2 weeks ago

I believe you can replace it with AC_COMPILE_IFELSE.

Don't worry, no hurry.

aol-nnov commented 2 weeks ago

@sauwming I've rewrote the patch with AC_COMPILE_IFELSE as you proposed earlier. Please review.

As for factoring out aarch64*) into separate branch, I had a small chat with our HW team and they had some uncertanities about the fact, that all aarch64 CPUS have neon support. On the other hand, I have no access to arm-apple-darwin* hw to check.

So, it's the least intrusive patch for now.

pjsip / pjproject

Correct cpu features detection during cross-compiation #4151