Skip to content

Commit

Permalink
bug #145 Prevent branch switching with GitFileReader (sstok)
Browse files Browse the repository at this point in the history
This PR was merged into the 1.3-dev branch.

Discussion
----------

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tickets       | 
| License       | MIT

The GitFileReader uses a temporary directory to clone to and read from, this clone is always singular to reduce too much temp directories.

But when using the GitFileReader it's possible this might cause some side-effects when one file reads from branch A but that file uses the GitFileReader to read from branch B, which would cause a switch (without switching back) and file A not being able to load any other files from branch A (as was expected).

Hopefully this will also fix a rather hard to find bug where configuration loading fails randomly.



Commits
-------

6cd1acf Prevent branch switching with GitFileReader
  • Loading branch information
sstok committed Apr 20, 2024
2 parents 3234d67 + 6cd1acf commit e7b926e
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 6 deletions.
4 changes: 2 additions & 2 deletions src/Service/Git/GitFileReader.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public function getFile(string $branch, string $path): string
throw GitFileNotFound::atBranch($branch, $path);
}

return $this->gitTempRepository->getLocal(mb_substr($this->gitBranch->getGitDirectory(), 0, -5), $branch) . \DIRECTORY_SEPARATOR . $path;
return $this->gitTempRepository->getLocal(mb_substr($this->gitBranch->getGitDirectory(), 0, -5), $branch, boundToBranch: true) . \DIRECTORY_SEPARATOR . $path;
}

/** @param string $path Path relative to repository root */
Expand Down Expand Up @@ -109,6 +109,6 @@ public function getFileAtRemote(string $remote, string $branch, string $path): s
throw GitFileNotFound::atRemote($remote, $branch, $path);
}

return $this->gitTempRepository->getRemote($this->gitConfig->getLocal('remote.' . $remote . '.url'), $branch) . \DIRECTORY_SEPARATOR . $path;
return $this->gitTempRepository->getRemote($this->gitConfig->getLocal('remote.' . $remote . '.url'), $branch, boundToBranch: true) . \DIRECTORY_SEPARATOR . $path;
}
}
16 changes: 12 additions & 4 deletions src/Service/Git/GitTempRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,22 @@ public function __construct(
private readonly Filesystem $filesystem
) {}

public function getLocal(string $directory, ?string $branch = null): string
/**
* @param bool $boundToBranch bind to the temp-location to $branch (to prevent switching during
* the process)
*/
public function getLocal(string $directory, ?string $branch = null, bool $boundToBranch = false): string
{
return $this->getRemote('file://' . $directory, $branch);
return $this->getRemote('file://' . $directory, $branch, $boundToBranch);
}

public function getRemote(string $repositoryUrl, ?string $branch = null): string
/**
* @param bool $boundToBranch bind to the temp-location to $branch (to prevent switching during
* the process)
*/
public function getRemote(string $repositoryUrl, ?string $branch = null, bool $boundToBranch = false): string
{
$tempdir = $this->filesystem->storageTempDirectory('repo_' . sha1($repositoryUrl), false, $exists);
$tempdir = $this->filesystem->storageTempDirectory('repo_' . sha1($repositoryUrl . ($boundToBranch ? '~' . $branch : '')), false, $exists);

if (! $exists) {
$this->process->mustRun(['git', 'clone', '--no-checkout', '--origin', 'origin', $repositoryUrl, $tempdir]);
Expand Down
9 changes: 9 additions & 0 deletions tests/Functional/Service/Git/GitTempRepositoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,15 @@ public function it_creates_temporary_repository_for_specific_branch(): void
self::assertFileExists($location . '/foo.txt');
}

/** @test */
public function it_creates_temporary_repository_for_specific_branch_without_switching(): void
{
$location = $this->gitTempRepository->getLocal($this->rootRepository, 'master');
$location2 = $this->gitTempRepository->getLocal($this->rootRepository, '_hubkit', true);

self::assertNotSame($location, $location2);
}

/** @test */
public function it_updates_temporary_repository_for_specific_branch(): void
{
Expand Down

0 comments on commit e7b926e

Please sign in to comment.