-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Inconsistency in Filesystem::ensureDirectoryExists
#11864
Comments
Requested information for composer issues: My root `composer.json`{
"name": "project/demo",
"config": {
"vendor-dir": "Packages/Libraries",
"bin-dir": "bin",
"allow-plugins": {
"ocramius/package-versions": true,
"neos/composer-plugin": true,
"composer/package-versions-deprecated": true,
"pixelfear/composer-dist-plugin": true
},
"platform": {
"php": "8.2.4"
},
"preferred-install": {
"*": "dist"
}
},
"require": {
"demo/config": "@dev"
},
"require-dev": {
"editorconfig-checker/editorconfig-checker": "^10.0",
"mikey179/vfsstream": "^1.6",
"neos/buildessentials": "@dev",
"neos/behat": "@dev",
"packagefactory/atomicfusion-proptypes": "^2.0",
"phpmd/phpmd": "^2.11",
"phpunit/phpunit": "^9.0",
"roave/security-advisories": "dev-latest",
"shel/crawler": "^2.2",
"shel/nodetypes-analyzer": "^1.0",
"sitegeist/chantalle": "^1.0",
"squizlabs/php_codesniffer": "^3.5",
"symfony/css-selector": "~2.0",
"t3n/neos-debug": "^2.0"
},
"repositories": [
{
"type": "path",
"url": "./DistributionPackages/*"
}
],
"scripts": {
"post-update-cmd": "Neos\\Flow\\Composer\\InstallerScripts::postUpdateAndInstall",
"post-install-cmd": "Neos\\Flow\\Composer\\InstallerScripts::postUpdateAndInstall",
"post-package-update": "Neos\\Flow\\Composer\\InstallerScripts::postPackageUpdateAndInstall",
"post-package-install": "Neos\\Flow\\Composer\\InstallerScripts::postPackageUpdateAndInstall"
},
"prefer-stable": true,
"minimum-stability": "dev"
} My `DistributionPackages/Demo.Config/composer.json`{
"name": "demo/config",
"type": "neos-package",
"description": "Neos package to keep neos packages loaded before other distribution packages",
"minimum-stability": "dev",
"prefer-stable": true,
"repositories": [],
"require": {
"neos/neos": "~8.3",
"neos/neos-ui": "~8.3",
"pixelfear/composer-dist-plugin": "*"
},
"require-dev": {},
"autoload": {
"psr-4": {
"Demo\\Config\\": "Classes/"
}
},
"extra": {
"download-dist": [
{
"url": "https://github.com/tameemsafi/typewriterjs/archive/refs/tags/v2.12.1.zip",
"path": "../../Libraries/tameemsafi/typewriterjs",
"name": "tameemsafi/typewriterjs:2.12.1"
}
]
}
} Output of `composer diagnose`
When I run this command:
I get the following output: Output
And I expected this to happen: |
The description of
I think the root cause of the issue here is your usage of For your use case, I would rather solve it with a repositories of type |
Note that building path with |
What do you mean with that? Which assumptions? Do you mean, that The problem is not directly
Yeah, I'm using this already. That is the main reason why I tried to use |
Ok I tried to fix |
@Seldaek Many thanks for your support and time on this.
Changing the current line from
to
seems to work and there are no further errors and the composer install is working fine. Not sure if this would be a plausible solution. |
@gjwnc great thanks for confirming, can you try again with latest snapshot? I did the fix on $target so it only gets normalized once and we reduce overhead. |
It is working now with latest snapshot. Many thanks! |
I'll provide the requested issue description in a separate comment, because github has a comment limit of 65 K characters.
Below I've filled the issue description as requested, but I'll give more context and provide a short test case for the method
Filesystem::ensureDirectoryExists
found here. I think this is related to #11111I'm using Ubuntu 22.04.3 LTS, php 8.3.3, and the composer plugin
pixelfear/composer-dist-plugin
to download a zip to a given directory. Using this method for non local path packages works fine. For symlinked local path packages this fails because ofensureDirectoryExists
- see the exception at the bottom.If you look at the source code of the composer plugin, you notice, that the plugin does download and then install the zip in case of composer 2. This causes
ensureDirectoryExists
to be called multiple times - which should be fine, but this is the reason why the below Test case calls the method twice and fails the second time.The project structure:
We are using a neos project which creates a bunch of directories and no
vendor
directory. Depending on the package type, the packages are installed in various locations likePackages/Application
orPackages/Framework
. This is how neos does it and valid so far. Non-neos packages are installed inPackages/Libraries
- this is basically thevendor
directory for other projects.For neos projects it is also common, to develop the project in local packages located in
DistributionPackages
directory, see the root composer.json below for the repository setup.This means, packages in DistributionPackages are commited to git and just symlinked in the Packages directory during composer install on the server. This is what
composer
does by default - nothing special yet.The problem:
In this specific case, I need a local package in
DistributionPackages/Demo.Config/composer.json
to install a zip inPackages/Libraries
directory. This causes multiple calls toensureDirectoryExists($directory)
with$directory = 'Packages/Application/Demo.Config/../../Libraries/tameemsafi/typewriterjs'
.The
Packages/Application/Demo.Config
is the symlink to../../DistributionPackages/Demo.Config
.For this specific path, the call to
is_directory
,is_link
orfile_exists
all return false - see the test case below. Causingmkdir
called twice. The first mkdir call is valid, because duringcomposer update
first time the directoryPackages/Libraries/tameemsafi/typewriterjs
does not exist. But for the second call,mkdir
fails withFile exists
🤯 - see the exception message at the bottom.After the
composer update
run, the directory actually exists. But the tests used inensureDirectoryExists
don't detect this case.I'm not sure what exactly is going on, but this is related to the symlink
Demo.Config
within the$directory
path. I thought about addingbut this does not work, because
realpath
tries to validate if$directory
exists and fails too - both times.I think a solution might be to use something like this.
Questions:
Do you think this is a valid composer issue? I think there are underlying issues with php, but I also noticed that
ls -lah Packages/Application/Demo.Config/../../Libraries/tameemsafi/typewriterjs
doesn't work either. Although the directory exists. Butls -lah Packages/Application/Demo.Config/../../Packages/Libraries/tameemsafi/typewriterjs
is working fine. Keep in mind, thatmkdir('Packages/Application/Demo.Config/../../Libraries/tameemsafi/typewriterjs', 0777, true)
does actually create the directoryPackages/Libraries/tameemsafi/typewriterjs
!Or will this not be solved. I think this is related to #11111.
I think the underlying issue is that
is_dir
or alike resolve the symlink and then the relative path, but mkdir does resolve the relative path first and thus drops the symlink. This could also be not an issue for php but mkdir itself?Anyway - can someone try to reproduce this issue and maybe have an idea for a fix? Maybe I could try to create a PR, using a custom method to resolve the relative path first using the linked method from the php docs?
Test case:
Copy the following test case to an empty tmp directory and run it. It creates a few relevant directories and uses a copy of
ensureDirectoryExists
to demonstrate the issue.Test case
Output of the test case:
The text was updated successfully, but these errors were encountered: