BrianHenryIE / strauss

Prefix PHP namespaces and classnames to allow multiple versions of libraries to exist without conflict.
https://brianhenryie.github.io/strauss/
MIT License
143 stars 23 forks source link

Handle symlinks properly #84

Closed doekenorg closed 8 months ago

doekenorg commented 9 months ago

Currently the logic to resolve symlinks is flawed making Strauss ignore symlinked packages. This PR solves that problem, and adds a test to ensure the original symlink is intact after running Strauss with "delete_vendor_packages": true.

Thanks for providing this tool. If this PR needs changing let me know.

I think this PR can finish #64.

Ref: https://github.com/BrianHenryIE/strauss/issues/64#issuecomment-1842061294

BrianHenryIE commented 9 months ago

Hey. I made a release today which was related to another issue and I haven't properly looked at this yet.

I would think that when deleteVendorPackages is true the symlink should be removed: https://github.com/BrianHenryIE/strauss/commit/503f1105b25cdfb96ddd6fb435a51e21a4bbf169

Does that contradict where you say

original symlink is intact

doekenorg commented 9 months ago

Hey sorry for the confusion. The thing is, if I have that setting on, the original folder (from which it is symlinked) gets cleared out. And that shouldn't happen. The symlink can be deleted. But the original files should stay intact. Does that make sense?

My plugin is located in ~/my-plugin. It uses ~/dependency as a aymlinked dependency. For me it clears out ~/dependency instead of just removing ~/plugin/vendor/namespace/dependency.

This fix makes sure it can find the location properly, when using relative paths. The test provides the use case that didn't work for me properly.

Ps, this worked for me from the PR version; but I couldn't figure out how to package that to a phar, and try that file as well. Could you help with that so I can make 100% sure this solves my problem?

BrianHenryIE commented 9 months ago

Create the phar (from GitHub Actions workflow):

composer install --no-dev --prefer-dist;
wget -O phar-composer.phar https://github.com/clue/phar-composer/releases/download/v1.4.0/phar-composer-1.4.0.phar;
mkdir build;
mv vendor build/vendor;
mv src build/src;
mv bin build/bin;
mv composer.json build;
php -d phar.readonly=off phar-composer.phar build ./build/;

Clean up:

mv ./build/*.* .
mv ./build/* .
rm -rf build
composer install

Run it:

chmod +x strauss.phar
php strauss.phar

TBH, I don't know what --prefer-dist is doing there, I'd expect that to be the default. But it was in the command from wherever I copied it from, so maybe it's important!

Those instructions don't include chmod +x phar-composer.phar. Let me know if that step is needed please.

I should document this somewhere better.

doekenorg commented 9 months ago

@BrianHenryIE Cool, I was finally able to test this solution and it works! It no longer clears out the folders!

When using the normal version I get a Warning: rmdir(/Users/doekenorg/code/.../vendor/...): Not a directory in... error. This is because it resolves the path incorrectly; and doesn't mark it as a symlink. So I can confirm the solution here works; as the test case also shows 👍

Edit: Funny enough I did find a small bug when a package configures an autoload path without a directory separator. Adjusted for that. So to sum up: