Skip to content

Update musl detection logic in TailwindBinary #109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 28, 2025

Conversation

Spomky
Copy link
Contributor

@Spomky Spomky commented Jul 28, 2025

Attempt to fix #107.
Uses ldd (if available) for better accuracy.

kbond
kbond previously approved these changes Jul 28, 2025
@bocharsky-bw
Copy link
Member

@Spomky Could you fix the PHP CS here, please?

@Spomky
Copy link
Contributor Author

Spomky commented Jul 28, 2025

@Spomky Could you fix the PHP CS here, please?

Hi @bocharsky-bw,
I updated the PR.
Regards.

Copy link
Contributor

@kbond kbond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Spomky!

Copy link
Member

@bocharsky-bw bocharsky-bw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thank you!

@kbond kbond merged commit f270e7c into SymfonyCasts:main Jul 28, 2025
12 checks passed
@Spomky Spomky deleted the bug/musl-binary branch July 28, 2025 17:16
@Kocal
Copy link
Contributor

Kocal commented Jul 29, 2025

If it can help you, we used a different logic to detect if PHP is using musl or not https://github.com/Kocal/BiomeJsBundle/blob/main/src/BiomeJsBinary.php#L41-L57

    private static function isMusl(): bool
    {
        static $isMusl = null;

        if (null !== $isMusl) {
            return $isMusl;
        }

        if (!\function_exists('phpinfo')) {
            return $isMusl = false;
        }

        ob_start();
        phpinfo(\INFO_GENERAL);

        return $isMusl = 1 === preg_match('/--build=.*?-linux-musl/', ob_get_clean() ?: '');
    }

we didn't have any issues so far since

@Spomky
Copy link
Contributor Author

Spomky commented Jul 29, 2025

Interesting!
And it looks better tbh. No need for ldd and maybe more reliable when used within a container.
I will take it and propose another PR if you don't mind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bad binary download ?
4 participants