mpg123: dont require msys2 when using msvc, fix macos crossbuild #26381

codereba · 2025-01-14T11:09:43Z

fixes #26380 I found if building process is in msvc environment, mpg123 never use msys2, but these lines aways add the msys2 requirement if os is windows, could we only add the requirement if that's not msvc environment?

please refer to:
https://github.com/conan-io/conan-center-index/blob/master/recipes/mpg123/all/conanfile.py#L103-L105

Summary

Changes to recipe: mpg123/1.31.2

Motivation

if building process is in msvc environment, mpg123 never use msys2, but these lines aways add the msys2 requirement if os is windows.

Details

I found if building process is in msvc environment, mpg123 never use msys2, but these lines aways add the msys2 requirement if os is windows, could we only add the requirement if that's not msvc environment?
please refer to:
https://github.com/conan-io/conan-center-index/blob/master/recipes/mpg123/all/conanfile.py#L103-L105

Maintainer changes

(by @jcar87)

Fix crossbuilding issue on macOS - ensure that when testing for assembler support, CFLAGS are propagated
Remove conan 1.x only logic that is a no-op in Conan 2
Use self.settings_build

[*] Read the contributing guidelines
[*] 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

I found if building process is in msvc environment, mpg123 never use msys2, but these lines aways add the msys2 requirement if os is windows, could we only add the requirement if that's not msvc environment? please refer to: https://github.com/conan-io/conan-center-index/blob/master/recipes/mpg123/all/conanfile.py#L103-L105

codereba · 2025-01-14T15:58:19Z

Hi @AbrilRBS , thanks!
Hope to do more contributions.

codereba · 2025-01-15T01:02:33Z

There is a build error for the profile:

======== Input profiles ========
Profile host:
[settings]
arch=x86_64
build_type=Release
compiler=apple-clang
compiler.cppstd=17
compiler.libcxx=libc++
compiler.version=13
os=Macos
[options]
*:shared=True
[conf]
tools.apple:sdk_path=/Applications/conan/xcode/13.0/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk
[buildenv]
DEVELOPER_DIR=/Applications/conan/xcode/13.0/Xcode.app/
SDKROOT=/Applications/conan/xcode/13.0/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk

Profile build:
[settings]
arch=armv8
build_type=Release
compiler=apple-clang
compiler.cppstd=17
compiler.libcxx=libc++
compiler.version=13
os=Macos
[conf]
tools.apple:sdk_path=/Applications/conan/xcode/13.0/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk
[buildenv]
DEVELOPER_DIR=/Applications/conan/xcode/13.0/Xcode.app/
SDKROOT=/Applications/conan/xcode/13.0/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk

The error message are:

gcc -E -I. -I/Users/jenkins/workspace/cci_prod_PR-26381/conan-home/p/b/mpg1248a9c0de2a75d/b/src -I./src  -DASMALIGN_BALIGN /Users/jenkins/workspace/cci_prod_PR-26381/conan-home/p/b/mpg1248a9c0de2a75d/b/src/src/libmpg123/dct36_avx.S | yasm - -pgas -rgas -mamd64 -f macho -o src/libmpg123/dct36_avx.o
gcc -E -I. -I/Users/jenkins/workspace/cci_prod_PR-26381/conan-home/p/b/mpg1248a9c0de2a75d/b/src -I./src  -DASMALIGN_BALIGN /Users/jenkins/workspace/cci_prod_PR-26381/conan-home/p/b/mpg1248a9c0de2a75d/b/src/src/libmpg123/dct64_avx_float.S | yasm - -pgas -rgas -mamd64 -f macho -o src/libmpg123/dct64_avx_float.o
In file included from /Users/jenkins/workspace/cci_prod_PR-26381/conan-home/p/b/mpg1248a9c0de2a75d/b/src/src/libmpg123/dct36_avx.S:9:
/Users/jenkins/workspace/cci_prod_PR-26381/conan-home/p/b/mpg1248a9c0de2a75d/b/src/src/libmpg123/mangle.h:14:10: fatal error: 'intsym.h' file not found
#include "intsym.h"
         ^~~~~~~~~~

The header file paths are not configured properly as other c++ file, and all the avx assembly files are failed to build, I think the reason may be related to the following messages in the github build:

checking __attribute__((aligned(16)))... yes
checking if assembler supports AVX instructions... no
checking for yasm... yasm
checking if yasm supports GAS syntax and AVX instructions... yes

Local host build is correctly with the settings:

======== Input profiles ========
Profile host:
[settings]
arch=x86_64
build_type=Release
compiler=apple-clang
compiler.cppstd=gnu17
compiler.libcxx=libc++
compiler.version=15
os=Macos

Profile build:
[settings]
arch=x86_64
build_type=Release
compiler=apple-clang
compiler.cppstd=gnu17
compiler.libcxx=libc++
compiler.version=15
os=Macos

If the compiler.version is 15, it can support to compile the avx assembly, this build error isn't produced.

I think the root cause is in the mpg123 building process, I will post a issue to mpg123 next step.

Hi @AbrilRBS, could change the compiler.version to 15 for the github runner?

AbrilRBS · 2025-01-15T10:39:22Z

Hi @codereba
Right now the whole CI system runs on apple-clang 13, the switch to a newer compiler version is planned but not yet implemented.

The best approach is to add an entry into the validate method to disable building for older apple-clang versions if those are not supported for the current published library versions.

Something like:

def validate(self):
    if self.settings.compiler == "apple-clang" and Version(self.compiler.version) < 15:
        raise ConanInvalidConfiguration("apple-clang < 15 can't compile due to unsupported avx")

or similar. Please let me know if this helps :)

jcar87 · 2025-01-15T10:46:49Z

The issue is in the ./configure script - dont think it has anything to do witht he appleclang version - when in x86_64, the "universal" apple-clang can very much target x86_64 including avx.

avx_support="unknown"
if test x"$avx_support" = xunknown; then
	{ $as_echo "$as_me:${as_lineno-$LINENO}: checking if assembler supports AVX instructions" >&5
$as_echo_n "checking if assembler supports AVX instructions... " >&6; }
	avx_support="no"
	echo '.text' > conftest.s
	echo 'vaddps %ymm0,%ymm0,%ymm0' >> conftest.s
	if $CCAS -c -o conftest.o conftest.s 1>/dev/null 2>&1; then
		avx_support="yes"
		{ $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
$as_echo "yes" >&6; }
	else
		{ $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
$as_echo "no" >&6; }
	fi
	rm -f conftest.o conftest.s
fi

$CCAS is the compiler, but when crossbuilding to target x86_64 - it is not passing -arch x86_64 when performing that test. In theory it should either pass CFLAGS or CCASMFLAGS (which is what it does when actually invoking the assembler)

I've pushed a fix.

IMO these are potentially both upstream issues:

the test should propagate CCASMFLAGS (which are initialized to CFLAGS) - no wonder it fails
the fallback on yasm does not seem to be working - either the flags are wrong, or the dependency order of the makefile is wrong

codereba · 2025-01-16T00:58:10Z

Thanks for your work, I discussed with mpg123 team, they know these issues.

…an-io#26381) * Remove the msys2 tool requirement if envorinment is msvc. * mpg123: fix macos crossbuild issue --------- Co-authored-by: Abril Rincón Blanco <git@rinconblanco.es> Co-authored-by: Luis Caro Campos <3535649+jcar87@users.noreply.github.com>

codereba mentioned this pull request Jan 14, 2025

[package] mpg123/1.31.2: Aways add the msys2 requirement if os is windows, but if building process is in msvc environment, mpg123 never use msys2. #26380

Closed

codereba changed the title ~~#26380 Remove the msys2 tool requirement if environment is msvc.~~ fixes #26380 Remove the msys2 tool requirement if environment is msvc. Jan 14, 2025

AbrilRBS self-assigned this Jan 14, 2025

Cleanup

4830d38

AbrilRBS changed the title ~~fixes #26380 Remove the msys2 tool requirement if environment is msvc.~~ mpg123: Remove the msys2 tool requirement if environment is msvc. Jan 14, 2025

mpg123: fix macos crossbuild issue

b93829c

jcar87 changed the title ~~mpg123: Remove the msys2 tool requirement if environment is msvc.~~ mpg123: dont require msys2 when using msvc, fix macos crossbuild Jan 15, 2025

jcar87 approved these changes Jan 15, 2025

View reviewed changes

AbrilRBS approved these changes Jan 15, 2025

View reviewed changes

jcar87 merged commit 628fdb7 into conan-io:master Jan 15, 2025
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mpg123: dont require msys2 when using msvc, fix macos crossbuild #26381

mpg123: dont require msys2 when using msvc, fix macos crossbuild #26381

codereba commented Jan 14, 2025 •

edited by jcar87

Loading

codereba commented Jan 14, 2025

codereba commented Jan 15, 2025 •

edited

Loading

AbrilRBS commented Jan 15, 2025

jcar87 commented Jan 15, 2025 •

edited

Loading

codereba commented Jan 16, 2025

mpg123: dont require msys2 when using msvc, fix macos crossbuild #26381

mpg123: dont require msys2 when using msvc, fix macos crossbuild #26381

Conversation

codereba commented Jan 14, 2025 • edited by jcar87 Loading

Summary

Motivation

Details

Maintainer changes

codereba commented Jan 14, 2025

codereba commented Jan 15, 2025 • edited Loading

AbrilRBS commented Jan 15, 2025

jcar87 commented Jan 15, 2025 • edited Loading

codereba commented Jan 16, 2025

codereba commented Jan 14, 2025 •

edited by jcar87

Loading

codereba commented Jan 15, 2025 •

edited

Loading

jcar87 commented Jan 15, 2025 •

edited

Loading