repman-io / repman

Repman - PHP Repository Manager: packagist proxy and host for private packages
https://repman.io
MIT License
519 stars 107 forks source link

Fix Git default branch #602

Open giggsey opened 2 years ago

giggsey commented 2 years ago

Newer versions of Git set the default branch to 'main', which breaks the tests (as SensioLabsSecurityChecker uses master).

The https://github.com/FriendsOfPHP/security-advisories repo still uses master, so we can't change anything within the actual service yet.

xvilo commented 2 years ago

Wonderful MR, can you do the following thing @giggsey:

There were 2 errors:
1) Buddy\Repman\Tests\Unit\Service\Security\SensioLabsSecurityCheckerTest::testUpdateWhenRepoDontExist
Symfony\Component\Process\Exception\ProcessFailedException: The command "'git' 'clone' '--depth' '1' '--branch' 'master' 'file:///tmp/repman/security-advisories-repo' '.'" failed.
Exit Code: 128(Invalid exit argument)
Working directory: /tmp/repman/security-advisories
Output:
================
Error Output:
================
Cloning into '.'...
fatal: '/tmp/repman/security-advisories-repo' does not appear to be a git repository
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
/buddy/repman/src/Service/Security/SecurityChecker/SensioLabsSecurityChecker.php:218
/buddy/repman/src/Service/Security/SecurityChecker/SensioLabsSecurityChecker.php:196
/buddy/repman/src/Service/Security/SecurityChecker/SensioLabsSecurityChecker.php:34
/buddy/repman/tests/Unit/Service/Security/SensioLabsSecurityCheckerTest.php:119
2) Buddy\Repman\Tests\Unit\Service\Security\SensioLabsSecurityCheckerTest::testUpdateWhenRepoExist
Symfony\Component\Process\Exception\ProcessFailedException: The command "'git' 'clone' '--depth' '1' '--branch' 'master' 'file:///tmp/repman/security-advisories-repo' '.'" failed.
Exit Code: 128(Invalid exit argument)
Working directory: /tmp/repman/security-advisories
Output:
================
Error Output:
================
Cloning into '.'...
fatal: '/tmp/repman/security-advisories-repo' does not appear to be a git repository
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.
/buddy/repman/src/Service/Security/SecurityChecker/SensioLabsSecurityChecker.php:218
/buddy/repman/src/Service/Security/SecurityChecker/SensioLabsSecurityChecker.php:196
/buddy/repman/src/Service/Security/SecurityChecker/SensioLabsSecurityChecker.php:34
/buddy/repman/tests/Unit/Service/Security/SensioLabsSecurityCheckerTest.php:134
ERRORS!
Tests: 496, Assertions: 1194, Errors: 2.
giggsey commented 2 years ago

From memory, the test failure error is actually misleading. It would probably have been better if I changed the line from ->run() to ->mustRun(), as the git init would have failed on the Buddy CI system.

I believe that is because the version of Git used is out of date, and does not support the initial branch param.

xvilo commented 2 years ago

Possibly it would be better to download the repository as a zip, then just unpacking it? This way Repman does not have to rely on external programs

giggsey commented 2 years ago

Possibly, but the actual running of repman relies on Git working locally anyway, so I don't see the major harm in running Git here too.

Updating the tests to the latest version of PHP 7.4 might resolve it anyway, it currently uses library/php:7.4.1

akondas commented 2 years ago

I think we can use new composer audit command. WDYT?

giggsey commented 2 years ago

Does it cache anything though? Wouldn't want to hit their API for every package within repman. Possibly using their API directly might work better and cache it similar to the git pull now

On Sat, 10 Sept 2022, 13:23 Arkadiusz Kondas, @.***> wrote:

I think we can use new composer audit command. WDYT?

— Reply to this email directly, view it on GitHub https://github.com/repman-io/repman/pull/602#issuecomment-1242717831, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACKUQUDRTJVDPJ26V2HDGTV5R4VLANCNFSM54Y5BVCA . You are receiving this because you were mentioned.Message ID: @.***>

akondas commented 2 years ago

Does it cache anything though? Wouldn't want to hit their API for every package within repman. Possibly using their API directly might work better and cache it similar to the git pull now

that's why I'm asking, because from my point of view, we can leave what we have now

giggsey commented 2 years ago

that's why I'm asking, because from my point of view, we can leave what we have now

My view is to leave it pulling from the repo for the time being. There are other areas of repman that could do with the attention that direct improve people's use of it, rather than refactoring a background task

xvilo commented 1 year ago

We should still need this merged as it indeed has issues on newer PHP docker images and with the default git of macos