MatmaRex / patchdemo

This repository has been moved to GitLab: https://gitlab.wikimedia.org/repos/ci-tools/patchdemo
https://gitlab.wikimedia.org/repos/ci-tools/patchdemo
MIT License
25 stars 21 forks source link

Add mediawiki/vendor repository #573

Closed MusikAnimal closed 1 year ago

MusikAnimal commented 1 year ago

I am working on https://gerrit.wikimedia.org/r/c/mediawiki/extensions/UrlShortener/+/930716 which requires a new package in mediawiki/vendor that is not yet merged and must undergo security review. it'd be awesome if in the meantime we could demo it!

I'm not sure if this is as simple as allowlisting the repository? It seems like there are potential security issues just as with #19. There was talk at #112 about having a separate repository with the list of trusted users who can change sensitive things like this. Maybe if we move forward with that, the same repo could be checked for vendor changes.

MatmaRex commented 1 year ago

I'd rather not use mediawiki/vendor here, and just install things with Composer. I think all you need is to add UrlShortener to the list here: https://github.com/MatmaRex/patchdemo/blob/master/repository-lists/composerinstall.yaml and Patch Demo will run composer install in the extension's directory when creating a wiki.

MusikAnimal commented 1 year ago

Perfect! I have filed https://github.com/MatmaRex/patchdemo/pull/574

Assuming that works, we're all set for UrlShortener and I assume this issue can be closed.

Thanks as always!

MusikAnimal commented 1 year ago

@MatmaRex Would it be possible to make Patch Demo ignore any Depends-On patches that are in mediawiki/vendor? That is needed for the tests to pass, but at the same time it causes Patch Demo to reject the patch.

I know this is sort of specialized handling, but it would seem not uncommon to depend on mediawiki/vendor patches while the dependent patch is still waiting for security review.

The workaround of course is to remove the Depends-On solely for Patch Demo, then add it right back. Not a huge deal, but it is somewhat tedious and pollutes the patchsets.

MatmaRex commented 1 year ago

Yes, I think that would be a good idea. I've also encountered this with people adding Depends-On pointing to config patches etc.