-
-
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
Pick higher revision number to guarantee successful file retrieval #11350
Conversation
|
I think it deserves a backport to LTS. Say if you need it rebased. |
As an afterthought, the revision is useless in the task of retrieving a single file. Thus the patch is really only 3 lines. diff --git a/src/Composer/Repository/Vcs/SvnDriver.php b/src/Composer/Repository/Vcs/SvnDriver.php
index f2bfad0f7..371faaf13 100644
--- a/src/Composer/Repository/Vcs/SvnDriver.php
+++ b/src/Composer/Repository/Vcs/SvnDriver.php
@@ -180,15 +180,13 @@ public function getFileContent(string $file, string $identifier): ?string
Preg::match('{^(.+?)(@\d+)?/$}', $identifier, $match);
if (!empty($match[2])) {
$path = $match[1];
- $rev = $match[2];
} else {
$path = $identifier;
- $rev = '';
}
try {
$resource = $path.$file;
- $output = $this->execute('svn cat', $this->baseUrl . $resource . $rev);
+ $output = $this->execute('svn cat', $this->baseUrl . $resource);
if (!trim($output)) {
return null;
} |
The revision being useless to retrieve a single file looks like a weird argument to me. the whole point of a VCS system is to track changes of files over revisions. Saying that getting a file at any revision is equivalent implies that you never modify any file after adding them in your VCS. |
Except when you meet a VCS that follows different versioning model, than what you are used to in Git. |
@AnrDaemon but ignoring revisions entirely does not seem the right way to fix this issue to me, as it ignores the fact that you wanted an older version of the file. |
Where do I want an older version, since at every point in code it refers to current one? |
To make it perfectly clear: in Subversion, every directory is a repository in itself. Thus, 'trunk', 'tags', 'branches' - merely a point of view. For VCS, they are all separate directories, with their own revision history. |
I'd rather not merge this as bugfix as I have no way to really try this out.. but let's do it in 2.6 as the change seems reasonable to me. Thanks |
I will provide a test case once I get a few free hours. And I'm still leaning to the idea that specifying |
Yes I believe you might be right there, but that's a bigger change that I am not so keen on making in this codepath that few people use as it might lead to regressions we won't notice for a while. |
SvnDriver::getTags
andSvnDriver::getBranches
both fail to compose valid reference tocomposer.json
if repository root was moved after their creation. Pick highest available revision from the list to raise chances for success.