repman icon indicating copy to clipboard operation
repman copied to clipboard

Fix Git default branch

Open giggsey opened this issue 3 years ago • 9 comments

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.

giggsey avatar Jul 27 '22 09:07 giggsey

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

  • Update the "update" command too, your tests are currently failing on this too! Check the output here: https://app.buddy.works/repman/repman/pipelines/pipeline/244546/execution/62e107710e961625d012c7b7
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.

xvilo avatar Sep 06 '22 13:09 xvilo

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.

giggsey avatar Sep 06 '22 14:09 giggsey

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

xvilo avatar Sep 06 '22 19:09 xvilo

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

giggsey avatar Sep 06 '22 19:09 giggsey

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

akondas avatar Sep 10 '22 12:09 akondas

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: @.***>

giggsey avatar Sep 10 '22 12:09 giggsey

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

akondas avatar Sep 10 '22 12:09 akondas

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

giggsey avatar Sep 22 '22 09:09 giggsey

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

xvilo avatar Jul 09 '23 19:07 xvilo