Enabled download of includes over HTTP (#605) #958

docdnp · 2023-08-13T21:46:58Z

Added option --git-over-http (default: false)
When active: Include-Parser uses git spare-checkout instead of git archive

closes #605

* Added option --git-over-http (default: false) * When active: Include-Parser uses git spare-checkout instead of git archive

firecow · 2023-08-14T05:40:50Z

@docdnp Isn't it possible to "auto-detect" that the checkout is a "http" checkout?

That way we can avoid having a specific cli option that http users needs to remember.

docdnp · 2023-08-14T07:20:43Z

@docdnp Isn't it possible to "auto-detect" that the checkout is a "http" checkout?

That way we can avoid having a specific cli option that http users needs to remember.

Well, yes.. could do it via: git config --get remote.origin.url

@firecow Do you know why the jest tests failed? I'm a bit surprised and TS isn't actually what I'm familiar with.

* CLI option still available but hidden

docdnp · 2023-08-14T07:41:06Z

@firecow In case you insist on the removal of the CLI option I'll do it. I don't see what reason might break the autodetection. The more I think about it, it should be better removed. Just let me know.

firecow · 2023-08-14T12:48:58Z

@docdnp Let's auto detect it.

I'll check out the jest tests.

I think we might have a broken test, sometimes 👍

* only auto-detection left over

firecow · 2023-08-14T19:32:10Z

In git-data.ts the "remote" is already parsed.

I suggest you edit the regexp to this

(?<schema>https|git|http:)(?::\/\/|@)(?<host>[^:/]*)(:(?<port>\d+)\/|:|\/)(?<group>.*)\/(?<project>[^ .]+)(?:\.git)?.*

And add a

this.remote.schema = gitRemoteMatch.groups.schema;

You will then be able to use the schema as your condition instead of git config --get remote.origin.url

The reason your jest test is failing is because you don't have a "bash spy" setup in the include-invalid-project-file-ref test-case. Reuse the git-data.ts object, and you won't have to add bash spy's in various pieces of test-case code.

docdnp · 2023-08-14T19:42:52Z

OK. Sounds good. I'll check it out. I've been already playing around with include-invalid-project-file-ref but stumbled over the attempt of the test setup to make some calls to your (firecow) gitlab repository.

Thanks for the hint. I'll come back in case of any further questions or obstacles.

docdnp · 2023-08-14T21:42:27Z

I've added <protocol> instead of <schema> in git-data.ts and changed the regular expression to

/(?:.*?)\s+(?<protocol>.*?)(?::\/\/|@)(?<host>[^:\/]*)(:(?<port>\d+)\/|:|\/)(?<group>.*)\/(?<project>[^ .]+)(?:\.git)?.*/

Double-checked the regexp in perl

#!/usr/bin/perl -n
# file: regexp.pl

    /   (?:.*?)\s+
        (?<protocol>.*?)
        (?::\/\/|@)
        (?<host>[^:\/]*)
        (:(?<port>\d+)\/|:|\/)
        (?<group>.*)
        \/
        (?<project>[^ .]+)
        (?:\.git)?.*
    /x && print   "Protocol: ", $+{protocol},  "\nHost    : ", $+{host}, 
                "\nGroup   : ", $+{group},     "\nProject : ", $+{project}, "\n"

using git remote -v | head -1 | regexp.pl. I was a bit surprised that [^:/] works in TS. Perl complains, and needs [^:\/] in line 7 after the host. I think it could be even simplified when used against git config --get remote.origin.url instead of git remote -v, like this:

#!/usr/bin/perl -n
# file: regexp.pl

    /   (?<protocol>.*?)
        (?::\/\/|@)
        (?<host>[^:\/]*)
        (:(?<port>\d+)\/|:|\/)
        (?<group>.*)
        \/
        (?<project>[^ .]+)
        (?:\.git)?
    /xx && print   "Protocol: ", $+{protocol},  "\nHost    : ", $+{host}, 
                "\nGroup   : ", $+{group},     "\nProject : ", $+{project}, "\n"

Don't know whether gitRemoteMatch matches exactly, but wouldn't assume it.

Nevertheless do I doubt that the errors won't appear now anymore, but who knows how surprised I will be if you're right.
Actually I wish you're right, even if I don't understand it. ;-)

firecow · 2023-08-15T05:56:47Z

Use the regex i pasted

…gexp

docdnp · 2023-08-15T12:20:55Z

OK. Done. But had to correct your suggestion, as it didn't catch pure 'http://' origins. Here a way to test it out...

Test data is:

# file: git.remote.out
origin	https://github.com/docdnp/gitlab-ci-local.git (fetch)
origin	[email protected]:docdnp/gitlab-ci-local.git (fetch)
origin	http://github.com/docdnp/gitlab-ci-local.git (fetch)

Script is:

#!/usr/bin/perl -n
# file: regexp.pl

$reschema = $ENV{use_regexp_firecow} == 1 ? qr((?<schema>https|git|http:)) : qr((?<schema>git|https?));

/   $reschema
    (?::\/\/|@)
    (?<host>[^:\/]*)
    (:(?<port>\d+)\/|:|\/)
    (?<group>.*)
    \/
    (?<project>[^ .]+)
    (?:\.git)?
/xx && print    "Schema : '", $+{schema},   "'\nHost   : '", $+{host}, 
                "'\nGroup  : '", $+{group},    "'\nProject: '", $+{project}, "'\n",

Test is:

docdnp:gitlab-ci-local $ cat git.remote.out | use_regexp_firecow=1 ./regexp.pl
Schema : 'https'
Host   : 'github.com'
Group  : 'docdnp'
Project: 'gitlab-ci-local'
Schema : 'git'
Host   : 'github.com'
Group  : 'docdnp'
Project: 'gitlab-ci-local'
docdnp:gitlab-ci-local $ cat git.remote.out | use_regexp_firecow=0 ./regexp.pl
Schema : 'https'
Host   : 'github.com'
Group  : 'docdnp'
Project: 'gitlab-ci-local'
Schema : 'git'
Host   : 'github.com'
Group  : 'docdnp'
Project: 'gitlab-ci-local'
Schema : 'http'
Host   : 'github.com'
Group  : 'docdnp'
Project: 'gitlab-ci-local'

As far as I see in the latest test report my assumption regarding the reason of the test failures was correct, as the error logs don't show anything related to the regexp or my changes.

Summary of all failing tests
 FAIL  tests/test-cases/include-invalid-project-file-ref/integration.include-invalid-project-ref-file.test.ts
  ● include-invalid-project-file-ref
  [...]
      Project include could not be fetched { project: firecow/gitlab-ci-local-includes, ref: HEAD, file: .gitlab-modue.yml }
    - Error: git archive | tar -f - -xC failed horribly
    -   this is a core error
    -   read it carefully
    + Error: Command failed with exit code 2: git archive --remote=ssh://[email protected]:22/firecow/gitlab-ci-local-includes.git HEAD .gitlab-modue.yml | tar -f - -xC .gitlab-ci-local/includes/gitlab.com/firecow/gitlab-ci-local-includes/HEAD
    + Host key verification failed.
    
    + fatal: the remote end hung up unexpectedly
    + tar: This does not look like a tar archive
    + tar: Exiting with failure status due to previous errors

I assume that their is a basic problem related to credentials or so:

    + Host key verification failed.
    + fatal: the remote end hung up unexpectedly

Now I'm trying to reproduce it with my own gitlab repository. Or is there any other way to test this stuff locally to decrease the feedback cycle?

* The test attempted to compare an expected error text with the assertion error's message. * The assertion error's message might have changed previously, so the test failed * Analysis of the current logs of failing tests in the pipeline shows that different reasons might lead to an expected failure when calling 'git archive': * the server doesn't exist or isn't reachable (which could timeout), then error message contains - ssh: Could not resolve hostname <non-existing-hostname>: Name or service not known - fatal: the remote end hung up unexpectedly * the credentials for the server are wrong, then error message contains: - [email protected]: Permission denied (publickey). - fatal: the remote end hung up unexpectedly * the repository doesn't exist on the server, then error message contains: - ERROR: The project you were looking for could not be found or you don't have permission to view it. - fatal: the remote end hung up unexpectedly * the file doesn't exist on the server, then error message contains: => remote: fatal: pathspec '.gitlab-modue.yml' did not match any files * Thus it seems OK to test against the 'fatal.*' only

firecow · 2023-08-15T17:07:40Z

Yeah, I just call node src/index.js --cwd /home/mjn/myproject or npx jest tests/test-cases/include-invalid-project-file-ref/ to run a single test case.

docdnp · 2023-08-15T18:46:30Z

Yeah. That's the way I'm doing local tests. Actually I see other tests failing than in the pipeline. That's a bit confusing as I don't know whether it depends on the changes I made or on other issues or whether the tests are a bit fragile.

You opened a pull request on the same issue? Why?

Fix development README.md

firecow · 2023-08-15T19:36:51Z

Just to test whats going on

docdnp · 2023-08-15T22:11:44Z

Now I'm quiet sure, that we're suffering of brittle tests. Made an analysis on 10 consecutive test runs. My personal opinion is at the end of this comment. Here the results in short:

Testrun 01:     Test Suites: 3 failed, 94 passed, 97 total      Tests:       4 failed, 212 passed, 216 total
Testrun 02:     Test Suites: 3 failed, 94 passed, 97 total      Tests:       4 failed, 212 passed, 216 total
Testrun 03:     Test Suites: 1 failed, 96 passed, 97 total      Tests:       2 failed, 214 passed, 216 total
Testrun 04:     Test Suites: 5 failed, 92 passed, 97 total      Tests:       5 failed, 211 passed, 216 total
Testrun 05:     Test Suites: 3 failed, 94 passed, 97 total      Tests:       4 failed, 212 passed, 216 total
Testrun 06:     Test Suites: 4 failed, 93 passed, 97 total      Tests:       4 failed, 212 passed, 216 total
Testrun 07:     Test Suites: 2 failed, 95 passed, 97 total      Tests:       2 failed, 214 passed, 216 total
Testrun 08:     Test Suites: 4 failed, 93 passed, 97 total      Tests:       5 failed, 211 passed, 216 total
Testrun 09:     Test Suites: 2 failed, 95 passed, 97 total      Tests:       3 failed, 213 passed, 216 total
Testrun 10:     Test Suites: 3 failed, 94 passed, 97 total      Tests:       3 failed, 213 passed, 216 total

I wanted also to know in how many test runs a failing test occured:

File: tests/test-cases/parallel-matrix/integration.parallel-matrix.test.ts
- Testruns: 04 05
File: tests/test-cases/artifacts-reports-dotenv/integration.artifacts-reports-dotenv.test.ts
- Testruns: 01 02 04 05 06 08 09 10
File: tests/test-cases/network-alias-build/integration.network-alias-build.test.ts
- Testruns: 01 02 03 04 05 06 07 08 09 10
File: tests/test-cases/artifacts-with-cache/integration.artifacts-with-cache.test.ts
- Testruns: 01 02 04 06 08
File: tests/test-cases/cache-docker/integration.cache-docker.test.ts
- Testruns: 04 06 07 08 10

Here some more details on the failing tests involved:

Testrun 01:
 FAIL  tests/test-cases/artifacts-reports-dotenv/integration.artifacts-reports-dotenv.test.ts (6.035 s)
 FAIL  tests/test-cases/artifacts-with-cache/integration.artifacts-with-cache.test.ts (5.261 s)
 FAIL  tests/test-cases/network-alias-build/integration.network-alias-build.test.ts (11.885 s)
Test Suites: 3 failed, 94 passed, 97 total
Tests:       4 failed, 212 passed, 216 total
Snapshots:   4 passed, 4 total
Time:        176.629 s

Testrun 02:
 FAIL  tests/test-cases/artifacts-reports-dotenv/integration.artifacts-reports-dotenv.test.ts (6.228 s)
 FAIL  tests/test-cases/artifacts-with-cache/integration.artifacts-with-cache.test.ts (5.24 s)
 FAIL  tests/test-cases/network-alias-build/integration.network-alias-build.test.ts (11.964 s)
Test Suites: 3 failed, 94 passed, 97 total
Tests:       4 failed, 212 passed, 216 total
Snapshots:   4 passed, 4 total
Time:        185.873 s

Testrun 03:
 FAIL  tests/test-cases/network-alias-build/integration.network-alias-build.test.ts (12.336 s)
Test Suites: 1 failed, 96 passed, 97 total
Tests:       2 failed, 214 passed, 216 total
Snapshots:   4 passed, 4 total
Time:        176.578 s

Testrun 04:
 FAIL  tests/test-cases/artifacts-reports-dotenv/integration.artifacts-reports-dotenv.test.ts (6.201 s)
 FAIL  tests/test-cases/artifacts-with-cache/integration.artifacts-with-cache.test.ts (5.263 s)
 FAIL  tests/test-cases/cache-docker/integration.cache-docker.test.ts (5.247 s)
 FAIL  tests/test-cases/network-alias-build/integration.network-alias-build.test.ts (11.853 s)
 FAIL  tests/test-cases/parallel-matrix/integration.parallel-matrix.test.ts
Test Suites: 5 failed, 92 passed, 97 total
Tests:       5 failed, 211 passed, 216 total
Snapshots:   4 passed, 4 total
Time:        187.093 s

Testrun 05:
 FAIL  tests/test-cases/artifacts-reports-dotenv/integration.artifacts-reports-dotenv.test.ts (6.038 s)
 FAIL  tests/test-cases/network-alias-build/integration.network-alias-build.test.ts (11.84 s)
 FAIL  tests/test-cases/parallel-matrix/integration.parallel-matrix.test.ts
Test Suites: 3 failed, 94 passed, 97 total
Tests:       4 failed, 212 passed, 216 total
Snapshots:   4 passed, 4 total
Time:        184.324 s

Testrun 06:
 FAIL  tests/test-cases/artifacts-reports-dotenv/integration.artifacts-reports-dotenv.test.ts (6.26 s)
 FAIL  tests/test-cases/artifacts-with-cache/integration.artifacts-with-cache.test.ts (5.324 s)
 FAIL  tests/test-cases/cache-docker/integration.cache-docker.test.ts (5.247 s)
 FAIL  tests/test-cases/network-alias-build/integration.network-alias-build.test.ts (11.915 s)
Test Suites: 4 failed, 93 passed, 97 total
Tests:       4 failed, 212 passed, 216 total
Snapshots:   4 passed, 4 total
Time:        180.757 s

Testrun 07:
 FAIL  tests/test-cases/cache-docker/integration.cache-docker.test.ts (5.222 s)
 FAIL  tests/test-cases/network-alias-build/integration.network-alias-build.test.ts (11.92 s)
Test Suites: 2 failed, 95 passed, 97 total
Tests:       2 failed, 214 passed, 216 total
Snapshots:   4 passed, 4 total
Time:        179.786 s

Testrun 08:
 FAIL  tests/test-cases/artifacts-reports-dotenv/integration.artifacts-reports-dotenv.test.ts (6.185 s)
 FAIL  tests/test-cases/artifacts-with-cache/integration.artifacts-with-cache.test.ts (5.222 s)
 FAIL  tests/test-cases/cache-docker/integration.cache-docker.test.ts (5.225 s)
 FAIL  tests/test-cases/network-alias-build/integration.network-alias-build.test.ts (12.01 s)
Test Suites: 4 failed, 93 passed, 97 total
Tests:       5 failed, 211 passed, 216 total
Snapshots:   4 passed, 4 total
Time:        187.418 s

Testrun 09:
 FAIL  tests/test-cases/artifacts-reports-dotenv/integration.artifacts-reports-dotenv.test.ts (6.123 s)
 FAIL  tests/test-cases/network-alias-build/integration.network-alias-build.test.ts (11.999 s)
Test Suites: 2 failed, 95 passed, 97 total
Tests:       3 failed, 213 passed, 216 total
Snapshots:   4 passed, 4 total
Time:        178.611 s

Testrun 10:
 FAIL  tests/test-cases/artifacts-reports-dotenv/integration.artifacts-reports-dotenv.test.ts (6.105 s)
 FAIL  tests/test-cases/cache-docker/integration.cache-docker.test.ts (5.177 s)
 FAIL  tests/test-cases/network-alias-build/integration.network-alias-build.test.ts (11.316 s)
Test Suites: 3 failed, 94 passed, 97 total
Tests:       3 failed, 213 passed, 216 total
Snapshots:   4 passed, 4 total
Time:        173.606 s

Do you know what's going on or have any hint, where I maybe don't test correctly? I'm always cautios regarding critique on when I begin to work in foreign codebases and assume in the first place, that I'm the one who maybe doesn't know exactly what he's doing.

But, to be honest, I don't believe anymore in the code changes that enable sparse checkouts are in any way connected to the failing tests. What is your opinion on that? I would suggest the following:

Let us finish the pull request
Let us later on figure out what the heck is so fragile with those tests

Are you OK with that?

firecow · 2023-08-16T04:17:39Z

I also think we have a brittle test in master.

I've only been able to get the parallel matrix test to fail occasionally though. As far as I see, thats the only one flapping in Github actions as well

Try using these jest cli options

https://jestjs.io/docs/cli#--runinband
https://jestjs.io/docs/cli#--testtimeoutnumber
https://jestjs.io/docs/cli#--maxconcurrencynum

But yeah, lets finish this.

Remove your changes to package.json, package-lock.json and the test file, and we are good to go, i think.

firecow · 2023-08-16T08:11:22Z

parallel matrix concurrent have been disabled fixing the flapping test case
https://github.com/firecow/gitlab-ci-local/actions/runs/5875837064

docdnp · 2023-08-16T16:28:57Z

OK. Testing without concurrency and a longer timeout works stable. You mean I should remove tsnode from package.json? And are you sure about reverting the test case? Actually the changes made the test run through.

I'll do the changes a bit later this evening. Just give me a short answer.

firecow · 2023-08-17T06:03:14Z

Yeah, the test file changes are irrelevant...

Just let ts-node stay if you want, just thought devs installed that globally.

docdnp · 2023-08-17T10:39:44Z

Did what you said. The test integration.include-invalid-project-ref-file.test.ts fails again... as I assumed.

firecow · 2023-08-17T16:58:09Z

https://github.com/firecow/gitlab-ci-local/pull/960/files Have a look at this.

docdnp · 2023-08-17T19:55:21Z

Well, thank you.. I think I've got it running now. Actually it was just a damn "/" missing in the "target" path. "tar" wasn't happy about this and thus the test failed. My fault. :-/

firecow

Looks great. Good job 👍

docdnp · 2023-08-17T20:14:45Z

Great. 👍 Thank you for the hints to the right direction.

firecow · 2023-08-17T20:24:27Z

No problem at all 👍 Thank you for the contribution

codespearhead · 2024-10-01T21:06:57Z

Should this be removed then?

gitlab-ci-local/README.md

Lines 210 to 212 in cec7ccd

    
           ## Quirks 
        
           git+http isn't properly supported https://github.com/firecow/gitlab-ci-local/issues/605 and has certain quirks

Enabled download of includes over HTTP

0f4d758

* Added option --git-over-http (default: false) * When active: Include-Parser uses git spare-checkout instead of git archive

docdnp changed the title ~~Enabled download of includes over HTTP~~ Enabled download of includes over HTTP (#605) Aug 13, 2023

fixed linter problems

91f5321

autodetecting whether sparse checkout should be preferred

20dc3de

* CLI option still available but hidden

docdnp and others added 5 commits August 14, 2023 17:42

Removed CLI option regardin sparse checkout

f3762da

* only auto-detection left over

Merge branch 'master' into master

1f1c8f2

Merge branch 'master' into master

072be86

refactored: parser-includes relies only on git-data

d877a21

Merge branch 'master' of github.com:docdnp/gitlab-ci-local

1f15e31

refactored: git-data detects schema on basis of firecow's proposed re…

46e0add

…gexp

Merge pull request #1 from firecow/master

8615667

Fix development README.md

preparing back-merge as discussed with firecow

ee03df1

firecow and others added 2 commits August 17, 2023 08:18

Merge branch 'master' into master

f97b3d2

Merge branch 'master' of github.com:docdnp/gitlab-ci-local

1c94c08

docdnp added 2 commits August 17, 2023 20:08

using clearer if clauses for schema default ports

1ae41be

fixed target path for tar call

b0ec96f

firecow approved these changes Aug 17, 2023

View reviewed changes

firecow merged commit 3d36a44 into firecow:master Aug 17, 2023

Enabled download of includes over HTTP (#605) #958

Enabled download of includes over HTTP (#605) #958

Uh oh!

Conversation

docdnp commented Aug 13, 2023 • edited by firecow Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

firecow commented Aug 14, 2023

Uh oh!

docdnp commented Aug 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

docdnp commented Aug 14, 2023

Uh oh!

firecow commented Aug 14, 2023

Uh oh!

firecow commented Aug 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

docdnp commented Aug 14, 2023

Uh oh!

docdnp commented Aug 14, 2023

Uh oh!

firecow commented Aug 15, 2023

Uh oh!

docdnp commented Aug 15, 2023

Uh oh!

firecow commented Aug 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

docdnp commented Aug 15, 2023

Uh oh!

firecow commented Aug 15, 2023

Uh oh!

docdnp commented Aug 15, 2023

Uh oh!

firecow commented Aug 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

firecow commented Aug 16, 2023

Uh oh!

docdnp commented Aug 16, 2023

Uh oh!

firecow commented Aug 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

docdnp commented Aug 17, 2023

Uh oh!

firecow commented Aug 17, 2023

Uh oh!

docdnp commented Aug 17, 2023

Uh oh!

firecow left a comment

Choose a reason for hiding this comment

Uh oh!

docdnp commented Aug 17, 2023

Uh oh!

firecow commented Aug 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codespearhead commented Oct 1, 2024

Uh oh!

Uh oh!

docdnp commented Aug 13, 2023 •

edited by firecow

Loading

docdnp commented Aug 14, 2023 •

edited

Loading

firecow commented Aug 14, 2023 •

edited

Loading

firecow commented Aug 15, 2023 •

edited

Loading

firecow commented Aug 16, 2023 •

edited

Loading

firecow commented Aug 17, 2023 •

edited

Loading

firecow commented Aug 17, 2023 •

edited

Loading