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 check for index() expecting to live in vendor/ directory #50

Merged
merged 3 commits into from
Dec 5, 2023

Conversation

adamkiss
Copy link
Contributor

@adamkiss adamkiss commented Sep 26, 2023

When you call Kirby CLI via absolute path (e.g.: cron jobs, daemons, etc.), the working directory is set to whatever the running program deems a good idea (often home directory).

Since this is pure composer plugin, we can be be almost 100% sure that the bootstrap script looking for index() lives in __ROOT__/vendor/getkirby/cms/bootstrap.php, and look for the possible index with this expectation.

I haven't added any tests, because I have no idea how to test __DIR__ calls, but since it's a very simple duplication of an existing check… manually testing it, it works on my machine 👍🏻😀

@lukasbestle
Copy link
Member

I see how it can make sense. At first I was a bit worried about possible issues where a index.php is in the directory three levels up for unrelated reasons. But given the installation with Composer, that could only be the project where the CLI was required locally or ~/.composer/index.php if required globally. So both cases should be fine.

Alternatively we could consider a --root flag to manually pass the site root. Then it will also work with a globally installed CLI. But I'd also be fine with your solution.

@adamkiss
Copy link
Contributor Author

I think both are preferable? This one's quite cheap (four checks at most), so you can merge this, and I can PR --root check

@lukasbestle lukasbestle requested a review from a team October 28, 2023 13:46
@distantnative distantnative mentioned this pull request Nov 28, 2023
2 tasks
@distantnative distantnative changed the base branch from main to develop November 28, 2023 15:51
@bastianallgeier bastianallgeier merged commit c94ded7 into getkirby:develop Dec 5, 2023
5 checks passed
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