Skip to content
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

Add support for static binary in GHA #291

Merged
merged 2 commits into from
Feb 22, 2024
Merged

Add support for static binary in GHA #291

merged 2 commits into from
Feb 22, 2024

Conversation

lyrixx
Copy link
Member

@lyrixx lyrixx commented Feb 20, 2024

No description provided.

Copy link
Member

@pyrech pyrech left a comment

Choose a reason for hiding this comment

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

That's an awesome feature to increase Castor adoption. Thanks 💛

CHANGELOG.md Show resolved Hide resolved
@lyrixx lyrixx force-pushed the test branch 10 times, most recently from 8757633 to 87a33ba Compare February 20, 2024 22:14
@tigitz
Copy link
Contributor

tigitz commented Feb 21, 2024

A small note related to my early comment

The subset of extensions chosen to be compiled currently in the binary is really small, it was designed to run a simple "Hello World".

My concern would be people trying to use it in their own real world Castor projects and hitting errors straight away. This would be a subpar experience.

Could we find a right balance of php extensions to include natively without bloating the binary too much ?

Maybe we could take insipiration from the official docker image, running docker run --rm php:8.3-alpine php -m gives :

[PHP Modules]
Core
ctype
curl
date
dom
fileinfo
filter
hash
iconv
json
libxml
mbstring
mysqlnd
openssl
pcre
PDO
pdo_sqlite
Phar
posix
random
readline
Reflection
session
SimpleXML
sodium
SPL
sqlite3
standard
tokenizer
xml
xmlreader
xmlwriter
zlib

Or maybe introducing a slim and normal version like docker official image tend to do would cover most use cases ?

@lyrixx
Copy link
Member Author

lyrixx commented Feb 21, 2024

ATM, the static binary weight <5mb, this is quite light IMHO.
What extension are missing?

@tigitz
Copy link
Contributor

tigitz commented Feb 21, 2024

The comparison with the official PHP Docker image reveals that the following extensions are absent:

  • ctype
  • curl
  • dom
  • fileinfo
  • filter
  • iconv
  • mysqlnd
  • openssl
  • PDO
  • pdo_sqlite
  • readline
  • session
  • SimpleXML
  • sodium
  • sqlite3
  • xml
  • xmlreader
  • xmlwriter

Incorporating all these extensions (bin/castor compile tools/phar/build/castor.linux-amd64.phar --php-extensions=mbstring,phar,posix,tokenizer,pcntl,ctype,curl,dom,fileinfo,filter,iconv,mysqlnd,openssl,pdo,pdo_sqlite,readline,session,simplexml,sodium,sqlite3,xml,xmlreader,xmlwriter) extends the build time to 4 minutes on my machine, which includes the time taken to download the extensions, from the prior 1 minute and 20 seconds.

This also increases the size of the build from 9MB to 28M.

However, it's worth noting that the default libraries in the Docker image are selected not merely for their popularity but due to the complexity involved in installing them manually, as discussed here.

Given this, it may be beneficial for Castor to determine its own default set of PHP extensions. For my projects, I typically add:

  • ctype
  • curl
  • dom
  • PDO
  • pdo_pgsql
  • openssl
  • sodium

This selection, excluding the specific requirement for pdo_pgsql, seems to offer a balanced extension of the default library, albeit increasing the size to 18M, which is a significant jump.

@lyrixx
Copy link
Member Author

lyrixx commented Feb 21, 2024

Size is 28M vs 9MB before.

something is strange on you computer. on github, it's only 4mb. see this build.

I'll made some bench with

ctype
curl
dom
PDO
pdo_pgsql
openssl
sodium

you really do some nice things with castor :D

@tigitz
Copy link
Contributor

tigitz commented Feb 21, 2024

something is strange on you computer. on github, it's only 4mb. see this build.

Not sure what's going on but I've tried from a freshly cloned project, phar is the same size from build but binary is still around 9M 🤷‍♂️

Maybe others experience the same issue ?

you really do some nice things with castor :D

Yeah 😅 I manage backup scripts, send emails, scrape the latest update of a webpage, and have my own vault secret management based on symfony one among other things. It's all about devops automation.

@lyrixx
Copy link
Member Author

lyrixx commented Feb 22, 2024

For the record, we had some concerns about security of the tool chain.

We use https://github.com/dixyes/phpmicro and we didn't review all the code. But this packages is used a lot by Laravel (herd, native php, etc)

So I think it's fine and I'll merge this PR since it's a real game changer !

@lyrixx lyrixx merged commit 6c795f9 into main Feb 22, 2024
8 checks passed
@lyrixx lyrixx deleted the test branch February 22, 2024 06:28
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.

4 participants