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

Fix broken junctions leads to installation failure #11550

Merged
merged 4 commits into from
Jul 21, 2023

Conversation

Attia-Ahmed
Copy link
Contributor

@Attia-Ahmed Attia-Ahmed commented Jul 11, 2023

Fixes #11549
Root Cause: When changing the project folder, existing junctions in Windows (created using absolute paths) become invalid. The "mklink /J" command used to create junctions results in an error ("Cannot create a file when that file already exists.") if a previous junction with a different target exists, as the junction is stored as an absolute path. This error causes package folders to have junctions with the wrong target directory, leading to mkdir(): No such file or directory

Windows Behavior: Windows prohibits creating a new junction if a file or junction with the same name already exists.

Solution: The bug fix ensures a clean execution by removing any previous junctions before creating new ones, regardless of whether they are broken or non-broken junctions. This resolves the installation failure caused by junctioning the package folder with the correct target directory.

Importance: This bug fix is crucial for Windows development environments. Developers may unknowingly encounter this issue when changing folder names or moving projects.

@Attia-Ahmed Attia-Ahmed requested a review from stof July 11, 2023 09:52
@Seldaek Seldaek added this to the 2.2 milestone Jul 21, 2023
@Seldaek Seldaek added the Bug label Jul 21, 2023
@Seldaek Seldaek modified the milestones: 2.2, 2.6 Jul 21, 2023
@Seldaek Seldaek merged commit 3d5f475 into composer:2.2 Jul 21, 2023
19 of 21 checks passed
@Seldaek
Copy link
Member

Seldaek commented Jul 21, 2023

Thanks, merged in 2.2 and 2.6 (ce876e7)


// Removing any previously junction to ensure clean execution.
if (!is_dir($junction) || $this->isJunction($junction)) {
@rmdir($junction);
Copy link

Choose a reason for hiding this comment

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

@Attia-Ahmed that looks strange to me, rmdir() if it is not a directory? Do we want to remove the file if it's not a directory, then we need unlink() for example, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@voku Thank you for your observation on the use of rmdir(). In this specific scenario, we're dealing with Windows junctions, which can sometimes be corrupted or broken. Standard PHP functions like is_dir(), is_file(), and is_link() have proven unreliable for detecting the existence such corrupted junctions. However, rmdir() has been effective in removing both valid and broken junctions, which is why it's used here, even when is_dir() returns false.

It's important to note that the original removeJunction function in the same class also relies on rmdir(). However, it throws an exception if is_dir() fails, making it unsuitable for this particular case.

If you'd like to explore alternative solutions, I'd invite you to run the testOverrideJunctions test, which will fail if we change this behavior. Feel free to give it a try and suggest a more convenient approach that ensures the test works as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants