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

Update to use new Drupal Finder (Composer Runtime) API #650

Draft
wants to merge 2 commits into
base: 10.x
Choose a base branch
from

Conversation

jnoordsij
Copy link
Contributor

@jnoordsij jnoordsij commented May 13, 2024

Drupal Finder has been revamped to use the Composer Runtime API, which makes it much simpler in how it operates. This PR updates the composer script handler to use this new Drupal Finder API. It also adds an additional panic exit in case the root cannot be found.

See also https://github.com/webflo/drupal-finder/releases/tag/1.3.0.

Edit: this requires some kind of additional work for new installations, given that the Path::makeRelative call seems to be failing on new installations, supposedly due to getComposerRoot returning null on fresh installations. I'm not sure yet on the fix that is required here (or maybe upstream?). A repeated composer install run seems to fix the issues.

@AlexSkrypnyk
Copy link
Collaborator

@jnoordsij
Thank you for contributing!

Confirming that this is a real issue and that we do need to provide support for the latest version of the DrupalFider.

Could you please update your PR to make it pass CI.

Thank you

@AlexSkrypnyk AlexSkrypnyk added D10 PR: Requires more work Pull request was reviewed and reviver(s) asked to work further on the pull request labels May 13, 2024
Copy link
Collaborator

@leymannx leymannx left a comment

Choose a reason for hiding this comment

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

Yeah if we can get that in with the CI succeed that would be really helpful. 👌🏻

@jnoordsij
Copy link
Contributor Author

I've quickly looked into this a bit further, but I'm very much unsure what's the possible cause of the CI failure. When trying to run this locally, I'm actually presented with an error at an earlier step:

[Symfony\Component\Filesystem\Exception\InvalidArgumentException]
  The path "/config/sync" cannot be made relative to ".../web", because they
  have different roots ("/" and "...").

However, after that, repeated composer runs and/or other commands run fine.

This makes it hard for me to debug what's exactly going on; so I'll just leave this open here for now for anyone interested in navigating this furter.

@jnoordsij jnoordsij marked this pull request as draft May 15, 2024 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
D10 PR: Requires more work Pull request was reviewed and reviver(s) asked to work further on the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants