openeuropa / composer-artifacts

Composer plugin that allows to download a specified artifact instead of the project source.
European Union Public License 1.2
10 stars 3 forks source link

WIP: Gitlab provider #18

Open drzraf opened 5 years ago

drzraf commented 5 years ago

OPENEUROPA-XXX

Description

Introduced a GitLab provider.

Change log

Notes

PRE vs POST artifacts (#14 )

  1. Binding toPRE_PACKAGE_* events means that artifacts tarball represent everything needed.
  2. Binding toPOST_PACKAGE_* events means that artifacts tarball complement the regular tarball (distUrl) and as such, the artifacts tarball must be extracted by the Provider on top of the distUrl tarball (possibly overriding files originating from the regular distUrl tarball)

Ideally, when a user only bind the GitLab provider on PRE_PACKAGE_* events, we could consider simply changing $this->package->setDistUrl() as Github provider does. But the issue lays in passing the GitLab API authentication token (HTTP header) needed to fetch private artifacts which AFAIK is something Composer downloader does not support.

drupol commented 5 years ago

Hi Raphaël,

Quite a lot of stuff for Gitlab, great work!

Quick review:

drzraf commented 5 years ago

What still lack:

  1. Custom extraction directory (eg: extracts artifacts inside ./static for example) and one question: Should we allow to extract artifacts *outside* package directory?
  2. Convenience test to see, in POST_PACKAGE_events if package actually contains a .gitlab-ci.yml (artifacts can't exist without it, but I'm not sure because the name of this file can be changed by project owner). [Some heuristic could even be created to extract a suitable job / stage from parsing the yaml.
  3. Tests. But I don't completely understand the way they work, they are slow to run and I didn't find a way to run only some of them. The sample GitLab payload provided should allow for multiple tests to be created. Do you think you could provide some help with this?
    • normal : artifacts are extracted
    • override : one of the files contained by the artifacts tarball override a file from the package
    • custom directory extraction
    • A build for specific job, ref (stage, tag) is not found
    • The artifact tarball does not exist anymore (they expire after a defined time)
  4. Until an hypothetical composer 2.x release allows us to, we are stuck using a custom downloader (https://github.com/composer/composer/issues/7929)
drzraf commented 5 years ago

I mocked Gitlab class where needed and added the ability to run a mock instead of the real class.

I tried to improve some bits from the unique test I created but can't understand why project is not installed when running through phpunit, to begin with. It probably has something to do with events dispatched by the TestApplication that could differ from a real run of composer install? Any idea?

drupol commented 5 years ago

I will carefully review this next week, first Fosdem this week, then on Monday evening, I'll give it a shot.

drzraf commented 5 years ago

Hi @drupol. A friendly reminder that I'd be glad to see this in :)

drupol commented 5 years ago

Haven't forgot you mate :)

drupol commented 5 years ago

Hi Rafael,

I'm setting up a new project based on this one, it will by default includes the plugins.

I suggest you to close this PR for now.

drzraf commented 5 years ago

Hi @drupol , you probably have your reasons but it's surprising.

(@ademarco / @dxvargas ?)

drupol commented 5 years ago

Hi @drupol , you probably have your reasons but it's surprising.

No it's no, I'd like to push this forward, but I have no time dedicated to this project at the Commission, so I'm doing the fork in my free time.

* Could you maintain an identical, or at least similar, (copyleft) licensing (eg: GPL)?

Yes.

* Is this a fork while this repository is "maintained" on its own?

Yes, this repo will still be maintained by the European Commission.

* Or is this repository going to be archived?

I don't know what will happen. I don't think it's gonna be archived, it's created and used by the EC, so, there is no reason for that.

* If you're not part of _OpenEuropa_ yourself while the organization is not maintaining actively the project, why not just move it so avoid (confusing) bicephal situation.

The OpenEuropa project created this project and uses it every day, so, there is no reason to move it, it's working well and it does the job :-)

* At least clarifying the reason of the fork (technical, licensing, political, personal) would be a nice clarification (for me, but maybe also for future contributors)

The issue about the provider is open since a couple of months now, and there is nobody to dedicate time in this project anymore.

On my side and as an OpenSource lover, I'm willing to push this forward, this is why the fork.

I don't think that the PR you did some times ago will ever be included in this project because the European Commission doesn't use Gitlab.

drzraf commented 5 years ago

Makes perfect sense. As long as the fork goes for a GPL-compatible copyleft license (as the initial codebase) I'm happy to join you there. In term of technical vision, do you confirm that one composer plugin supporting several "artifact providers" is more adequate than one composer plugin per provider?

drupol commented 5 years ago

Yes, this is why the fork make sense it's technically full of sense.

drzraf commented 1 year ago

@brummbar : If I rebase the PR, would you merge it?