arrow: fix turning simd off

[dvirtz]

Summary

Changes to recipe: arrow/*

Motivation

It's impossible to set an option to None as it's interpreted as a string.

fixes #27886

Details

This change adds a "none" option to simd_level and runtime_simd_level while keeping the old None value for backward compatibility.

---

• Read the contributing guidelines
• x ] Checked that this PR is not a duplicate: list of PRs by recipe
• Tested locally with at least one configuration using a recent version of Conan

[franramirez688]

Hi @dvirtz

Thanks for this PR 👏

I think that we can simplify it more:

• None never worked as an option, so let's remove it.
• Both options are never removed from the recipe, so there is no need to use get_safe and convert them into a string, right?

[franramirez688]

I think that if this is not working, we can remove it. Wdyt? cc @AbrilRBS

Suggested change

	"simd_level": [None, "default", "sse4_2", "avx2", "avx512", "neon", "none"],
	"runtime_simd_level": [None, "sse4_2", "avx2", "avx512", "max", "none"],
	"simd_level": ["default", "sse4_2", "avx2", "avx512", "neon", "none"],
	"runtime_simd_level": ["sse4_2", "avx2", "avx512", "max", "none"],

[franramirez688]

Suggested change

	if str(self.options.get_safe("simd_level")).lower() != "none" or \
	str(self.options.get_safe("runtime_simd_level")).lower() != "none":
	if self.options.simd_level != "none" or \
	self.options.runtime_simd_level != "none":

[franramirez688]

Suggested change

	if str(self.options.get_safe("simd_level")).lower() != "none" or \
	str(self.options.get_safe("runtime_simd_level")).lower() != "none":
	if self.options.simd_level != "none" or \
	self.options.runtime_simd_level != "none":

[AbrilRBS]

Hi @dvirtz thanks a lot for taking the time to report this and submitting a PR, we appreciate it.

After trying this locally, I'm getting compilation errors after disabling the simd levels in Windows/msvc/arm64,

C:\Users\abril\.conan2\p\b\arrowe065a76516bd6\b\src\cpp\src\arrow\vendored\pcg\pcg_uint128.hpp(76,1): error C1189: #error:  Unable to determine target endianness [C:\Users\rub
en\.conan2\p\b\arrowe065a76516bd6\b\build\src\parquet\parquet_static.vcxproj]
  (compilando archivo de origen "../../../src/cpp/src/parquet/metadata.cc")

  properties.cc
  size_statistics.cc
  statistics.cc
  stream_reader.cc
  stream_writer.cc
  types.cc
  encryption_internal_nossl.cc

Note that disabling parquet works, have you tried this locally? Do you see the same error?

This does bring up the point that https://arrow.apache.org/docs/developers/cpp/windows.html#building-on-windows-arm64-using-ninja-and-clang says that msvc/arm64 is not a supported configuration in upsteam, and to instead use some flavour of clang, which would be compatible for the msvc binaries in the rest of your graph, we cn help with setting it up if needed :)

[dvirtz]

Thanks for the review @franramirez688 and @AbrilRBS.
I didn't try it on Windows ARM64 actually only on Ubuntu but this change is useful anyway I guess.
Added raising ConanInvalidConfiguration for ARM64/MSVC system.
I'll try switching to clang.

[dvirtz]

With clang it hits #26390

[jcar87]

Thanks @dvirtz - this is now handled in #28675 and superseded by that PR.

Indeed, the None option was wrong. None as a valid option value means "allow this option to not have a value". But if default_options gives it a value, then it can never be set. So None as an option value should only be used when it is also the default value, to mean "the default value is unset, do something else" - for example when the default value needs to be set conditional to a setting.

To avoid the eternal confusion regarding the word "none", we've just used "disabled" in #28675, thanks!