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

Use relative directories for packages when building GeneratedConfig #23

Closed
wants to merge 1 commit into from

Conversation

cjunge-work
Copy link

Build the GeneratedConfig file using relative paths so PHPStan can be run inside/outside containers/CI, etc.

Fixes #17

@cjunge-work
Copy link
Author

@ondrejmirtes any chance this could be merged?

@arnaudgoulpeau
Copy link

Hi, we work with Docker and the composer image. We have the same problem and any though on this PR would be greatly appreciated. Even if it is a big no.
By the way, thanks for all the great stuff here !

@ondrejmirtes
Copy link
Member

Hi,
I'm not sure about the implications of this, it would need a lot of testing. We'd first need an extensive test suite that would allow us testing this on Linux/Windows using GitHub Actions making sure we didn't break anything.

@ondrejmirtes
Copy link
Member

When looking at the implementation, I already know that this is wrong:

		$vendorPos = strpos(__DIR__, 'vendor');
		if ($vendorPos === false) {
			return;
		}

Composer users can change their vendor-dir using configuration: https://getcomposer.org/doc/06-config.md#vendor-dir

@ondrejmirtes
Copy link
Member

I stand by my comment from #17:

I'm fine if this utility isn't usable by everyone. It can afford to only solve the happy path for its users. The only thing it does is to save typing a few lines in your phpstan.neon.

// walk up to vendor
$basePathTail = substr(__DIR__, $vendorPos);
// how many parts?
$this->dirCount = substr_count($basePathTail, DIRECTORY_SEPARATOR) + 1;
Copy link

Choose a reason for hiding this comment

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

Actually, we can know that the vendor folder is always dirname(__DIR__, 3) here, as this file is in <vendor-dir>/phpstan/extension-installer/src whatever the vendor-dir is named.

@@ -129,4 +150,27 @@ public function process(Event $event): void
}
}

private function getInstallPath(InstallationManager $installationManager, PackageInterface $package): string
Copy link

Choose a reason for hiding this comment

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

An alternative solution could be to rely on Composer\Util\Filesystem::findShortestPathCode instead (but it might require changing the way $data is handled for the file generation then as var_export($data, true) won't do it anymore)

@ondrejmirtes
Copy link
Member

I plan to tackle this myself, thank you.

@ondrejmirtes
Copy link
Member

This was all that was needed: 66c7adc + phpstan/phpstan-src@0e306c6

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.

Do not use absolute paths for storing found plugins
4 participants