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

Contents of junctioned (path) repositories still deleted #9583

Open
AndreKR opened this issue Jan 1, 2021 · 4 comments
Open

Contents of junctioned (path) repositories still deleted #9583

AndreKR opened this issue Jan 1, 2021 · 4 comments
Labels
Milestone

Comments

@AndreKR
Copy link

AndreKR commented Jan 1, 2021

My `composer.json`:
{
	"name": "myapp/myapp",
	"description": "",
	"license": "proprietary",
	"require": {
		"php": "^7.2",
		"creations/df": "dev-master",
		"ext-pgsql": "*"
	},
	"autoload": {
		"psr-4": {
			"myapp\\": "src",
			"myapp\\tests\\": "tests"
		}
	}
}
Output of `composer diagnose`:
 Checking composer.json: WARNING
 require.creations/df : unbound version constraints (dev-master) should be avoided
 Checking platform settings: The Windows OneDrive folder is not supported on PHP versions below 7.2.23 and 7.3.10.
 Upgrade your PHP (7.3.1) to use this location with Composer.
 
 
 Checking git settings: OK
 Checking http connectivity to packagist: OK
 Checking https connectivity to packagist: OK
 Checking github.com rate limit: OK
 Checking disk free space: OK
 Checking pubkeys:
 Tags Public Key Fingerprint: 57815BA2 7E54DC31 7ECC7CC5 573090D0  87719BA6 8F3BB723 4E5D42D0 84A14642
 Dev Public Key Fingerprint: 4AC45767 E5EC2265 2F0C1167 CBBB8A2B  0C708369 153E328C AD90147D AFE50952
 OK
 Checking composer version: You are not running the latest stable version, run `composer self-update` to update (1.10.19 => 2.0.8)
 Composer version: 1.10.19
 PHP version: 7.3.1
 PHP binary path: C:\dev-tools\php-7.3.1-nts-Win32-VC15-x64\php.exe
 OpenSSL version: OpenSSL 1.1.1a  20 Nov 2018

This is a continuation of #4955 because I'm still seeing this issue.
A similar behavior was already observed in #4955 and fixed in the meantime, but it has regressed in 1.9.0. The last version not affected by this is 1.8.3.

When composer removes a junctioned dependency, instead of removing the junction, it descends into the junctioned repository and deletes everything recursively. That includes the .git directory with unpushed branches, reflog, everything, which makes this bug particularly severe.

This happened to me many times before, but this time I got "lucky". Composer was unable to delete a file and aborted, which left the junction behind and allowed me to do some debugging.

I copied the isJunction() function from src/Composer/Util/Filesystem.php to a script and called it with the path of the junction. It returns true. So either the detection failed only one time, or it's not this function that causes the bug to happen.

(I copied the function from my local composer.phar, but it seems identical to the current master on GitHub.)

Here's the output, which I find interesting because apparently the deletion doesn't happen during the "Removing ..." step, but during the "Installing ..." step:

Package operations: 0 installs, 1 update, 0 removals
  - Removing creations/df (dev-master), source is still present in C:\htdocs\myapp\vendor/creations/df
  - Installing creations/df (dev-master eb99b82):

  [RuntimeException]
  Could not delete C:\htdocs\myapp\vendor/creations/df\.git\objects\pack\pack-00de1b8825c71d6a7959975b1f703f8832c61538.idx:
  This can be due to an antivirus or the Windows Search Indexer locking the file while they are analyzed

I will now purposefully lock a file in the junctioned repository (by running notepad > lockme.txt on the command line) and do some more debugging...

@AndreKR
Copy link
Author

AndreKR commented Jan 1, 2021

I don't know enough about Composers internal workings to say which of the following things are supposed to happen and which are not, so I will just report what I saw.

After resolving dependencies, an update was scheduled:

Package operations: 0 installs, 1 update, 0 removals
Updates: creations/df:dev-master eb99b82

I'm not quite sure why it thinks an update is necessary in this case, but I think that doesn't matter: Even when an update is necessary, it should remove the junction and reinstall it directly instead of deleting the contents of the junctioned repository, right?

Then in DownloadManager::update(), this path is taken:

// upgrading from a dist stable package to a dev package, force source reinstall
if ($target->isDev() && 'dist' === $installationSource) {
$downloader->remove($initial, $targetDir);
$this->download($target, $targetDir);
return;
}

And then in PathDownloader::remove(), this path is taken:
https://github.com/composer/composer/blob/1.10.19/src/Composer/Downloader/PathDownloader.php#L167-L173

This is because $path is the path to the junction and $package->getDistUrl() is the path to the junctioned repository. realpath($path) is the path to the junctioned repository because realpath() resolves junctions. Thus they are identical so the return is executed and the junction is not removed.

Then DownloadManager::download() is called by the code linked above (first code snippet) and that in turn calls VcsDownloader::download() which calls $this->filesystem->emptyDirectory($path).

emptyDirectory() calls Filesystem::remove() on the contents of the junctioned repository, but never on the function itself. So even though Filesystem::remove() would remove the junction properly, since it never sees the junction, the junction is again not removed.

@AndreKR
Copy link
Author

AndreKR commented Jan 1, 2021

So I was curious why it even thinks an update is needed in the first place.

There is a file vendor/composer/installed.json. In there, Composer keeps the commit hash of the junctioned repository when it was last installed. If that doesn't match the current commit hash of the junctioned repository, Composer schedules an update. So whenever the junctioned repository changes, Composer thinks an update is necessary and completely deletes the junctioned repository (by the process described above).

While it is true that Composer gets some (wrong) information from vendor/composer/installed.json, not every change to the junctioned repository triggers a reinstall. In most cases it will just report "Source already present". There is one specific change that reproducibly triggers the issue, changing "sentry/sentry": "1.10.*" to "sentry/sentry": "3.1.*" in the junctioned repository's requirements, but only if the first install had been made with the "sentry/sentry": "1.10.*".

Wild guess: installed.json and/or the current state of vendor is somehow (erroneously?) taken into account when resolving the dependencies, so that the junctioned repository with the composer.json change does not satisfy the requirements anymore or at least loses to the VCS version. The VCS version is then installed and that's where the issue happens (note how the error happens in VcsDownloader::download() while PathDownloader just says "Source already present").

This guess is corroborated by the fact that when using 1.8.3 (the last version not affected by this issue) it deletes the junction and freshly clones the repository.

@Seldaek Seldaek added this to the Bugs milestone Jan 17, 2021
@Seldaek Seldaek added the Bug label Jan 17, 2021
@Seldaek Seldaek changed the title Contents of junctioned repositories still deleted Contents of junctioned (path) repositories still deleted Jan 17, 2021
@Seldaek
Copy link
Member

Seldaek commented Jan 17, 2021

Can you check if 2.0 is affected? Because we are not really fixing edge case stuff in 1.x at this point..

@Seldaek
Copy link
Member

Seldaek commented May 23, 2022

Ping? Would be good to know if this can still be reproduced, as I lack a way to reproduce this reliably.

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

No branches or pull requests

2 participants