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

feat: better default for the php.ini path #553

Merged
merged 7 commits into from
Oct 18, 2024

Conversation

dunglas
Copy link
Contributor

@dunglas dunglas commented Oct 8, 2024

What does this PR do?

By default, the php.ini path is /lib/php.ini. This value is "hardcoded" in PHP but can be changed with configure flags. Most (all?) Linux distributions as well as the official PHP Docker images change this default path to /usr/local/etc/php/php.ini or /etc/php.ini.

This default value is very confusing for end users (see for instance dunglas/frankenphp#1066, it's a recurrent issue we have with FrankenPHP).

Additionally, /lib doesn't even exist by default on Mac, and cannot be created because because the system filesystem is read-only.

This PR aligns the default path to the one used by official Docker images: /usr/local/etc/php/php.ini
In addition, it loads all files defined in /usr/local/etc/php/conf.d, as the Docker images do.

It can still be changed easily by end users if needed by overriding SPC_CMD_PREFIX_PHP_CONFIGURE.

@crazywhalecc crazywhalecc added the kind/php-and-sapi Issues related to php source and SAPI label Oct 9, 2024
@crazywhalecc
Copy link
Owner

It can still be changed easily by end users if needed by overriding SPC_CMD_PREFIX_PHP_CONFIGURE.

Do you think this happens often? I just found out there is a PR about this before switching to env.ini: #511, which adds command line arguments to specify. I'm not sure which way is more suitable for most people, but we can change the default anyway.

@dunglas
Copy link
Contributor Author

dunglas commented Oct 9, 2024

It could be convenient to provide an easier way to change the path, indeed. Changing the default and adding a new option looks like the best option to me.

@crazywhalecc
Copy link
Owner

crazywhalecc commented Oct 12, 2024

@dunglas I have merged #511 to main, now we can set a default value in this PR.

@dunglas
Copy link
Contributor Author

dunglas commented Oct 12, 2024

@crazywhalecc great! How would you like to proceed? Change the default value in the option definition directly and add a new option to set --with-config-file-scan-dir?

@crazywhalecc
Copy link
Owner

crazywhalecc commented Oct 12, 2024

@crazywhalecc great! How would you like to proceed? Change the default value in the option definition directly and add a new option to set --with-config-file-scan-dir?

TLDR: Yes (only if we use command options).

Off topic: My personal preference for various inputs and variables:

  • Command options: for parts that need to be changed frequently
  • Values ​​of env.ini: for parts that do not need to be modified under normal circumstances
  • Internal mutable environment variables: for system paths, these values ​​can be modified in edge cases, but modification is not recommended
  • Internal immutable environment variables: different values ​​obtained from the system environment, such as WORKING_DIR, SPC_LINUX_DEFAULT_CC, these values ​​are used internally by spc

If we use the same default location for macOS, Linux, and FreeBSD, then just use the command option defaults. But if there are different defaults for different systems, it may be better to use env.ini with Internal mutable environment variables directly.

For the --with-config-file-path and --with-config-scan-dir compilation options, they may be edited frequently, but it is also possible that people usually do not need to modify them. This is a fuzzy boundary, so I don't have a clear goal to decide how to deal with these two options.

@dunglas dunglas force-pushed the feat/better-php.ini-path branch from c64eaec to f94d9bb Compare October 15, 2024 09:15
@dunglas
Copy link
Contributor Author

dunglas commented Oct 15, 2024

Code and docs updated.

I used DeepL for the Chinese translation, it may be totally wrong!

Copy link
Owner

@crazywhalecc crazywhalecc left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution. I will adjust the documentation section after finish modifications.

I also found that it seems that I missed Windows support, and Windows configure does not seem to be able to modify the default config-file-path, only config-file-scan-dir. And Windows cli will look for php.ini in C:\Windows\ and $(cwd).

docs/zh/faq/index.md Show resolved Hide resolved
src/SPC/command/BuildCliCommand.php Outdated Show resolved Hide resolved
@crazywhalecc crazywhalecc merged commit 0fb7784 into crazywhalecc:main Oct 18, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/php-and-sapi Issues related to php source and SAPI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants