Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eliminate plugins submodules #6605

Closed
mattab opened this issue Nov 6, 2014 · 28 comments
Closed

Eliminate plugins submodules #6605

mattab opened this issue Nov 6, 2014 · 28 comments
Labels
answered For when a question was asked and we referred to forum or answered it. c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. c: Tests & QA For issues related to automated tests or making it easier to QA & test issues. Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc.
Milestone

Comments

@mattab
Copy link
Member

mattab commented Nov 6, 2014

The goal of this issue is to investigate our uses of sub-modules and ultimately get rid of them to use a better way instead.

What do we use submodules for?
SM or submodules are a feature of git that we use at Piwik to keep track of

  1. Plugins officially supported by core team: CustomAlerts, PleineLune, SecurityInfo, TasksTimetable, TreemapVisualization and VisitorGenerator.
  2. tests/PHPUnit/UI is a submodule to the big screenshot tests repo holding all screenshots: https://github.com/piwik/piwik-ui-tests
  3. libs/PiwikTracker is a submodule to piwik-php-tracker repo

Why getting rid of SM is important?
Submodules are quite painful to use by themselves and especially with branches. This conflicts with our project to use a always-green master branch and a develop branch #5954 - because we need a master green at all times,

Idea: replacing submodules with Composer

What if we would simply use Composer tool instead of submodules?

As a developer, maybe Composer could help me checkout all the core plugin repositories (they can be put in the composer.json section require-dev). Each of those plugins eg. in plugins/CustomAlerts would be a clone of the git repo (see this SO answer).

Managing branches across many repositories
Using phpstorm I can easily add the repos to Settings>Git and then:

  1. If I work on core refactoring, I can use the phpstorm synchronised branch feature to create a branch at once in all repositories (core + plugins all at once)
  2. If I work on a given plugin improvement or fix, I can create a branch in this repo I'm working on.

Other ideas/brain dump

  • maybe there is value in putting on packagist as well in those six core plugins?
  • I noticed phpunit does it and has this badge to inform of status Dependency Status
  • maybe we could later also create a composer.json that would let us load all premium plugins? this would help a developer to get all actively maintained plugins at once and do product-wide refactorings easier.
  • we can see how to remove the tests/PHPUnit/UI submodule in another issue (out of scope?)

What do you think of this?

RFC @tsteur @diosmosis @mgazdzik @mnapoli @vox3r @czolnowski @sgiehl @julienmoumne @halfdan @quba @tzi

@mattab mattab added Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. c: Tests & QA For issues related to automated tests or making it easier to QA & test issues. answered For when a question was asked and we referred to forum or answered it. RFC Indicates the issue is a request for comments where the author is looking for feedback. labels Nov 6, 2014
@mattab mattab added this to the Piwik 2.10.0 milestone Nov 6, 2014
@mattab mattab removed the answered For when a question was asked and we referred to forum or answered it. label Nov 6, 2014
@mgazdzik
Copy link
Contributor

mgazdzik commented Nov 6, 2014

+1 for this. We can then manage all Piwik plugins (Core + Premium) in unified way.

@sgiehl
Copy link
Member

sgiehl commented Nov 6, 2014

+1
@mgazdzik Adding "premium" plugins to composer.json won't work. If the repos are not public, people without access might get an error.

Instead of using composer we could also create a new console command to auto install all maintained plugins (which might contain premium plugins, that are ignored if there is no access given)

@czolnowski
Copy link
Contributor

+1 for composer. It's tool which is using in many PHP projects so I think that in someway becoming standard in PHP world.

@mgazdzik
Copy link
Contributor

mgazdzik commented Nov 6, 2014

@sgiehl - on the other hand we don't want just anybody to download premium plugins I think. Also I rather ment those complete files rather for internal use of refactoring, testing, etc.

@tsteur
Copy link
Member

tsteur commented Nov 6, 2014

Can you explain step by step what is to be done and which commands are to be executed when having to use a branch for plugins? And maybe also what is to be done when merging? To see whether it makes it actually easier to work with branches...

@mattab mattab modified the milestones: Short term, Piwik 2.10.0 Nov 20, 2014
@mattab mattab added Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. and removed Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. labels Nov 20, 2014
@mnapoli
Copy link
Contributor

mnapoli commented Jan 5, 2015

Is there a reason why plugins in submodules are not in this repository?

I get that they are optional plugins, but we could have them in piwik/piwik disabled by default.

@mattab
Copy link
Member Author

mattab commented Jan 5, 2015

we don't need to bundle those plugins with core because they are optional. they are available via the Marketplace. Ultimately we would like to move more plugins to the Marketplace to keep core smaller, eg. #5341

@mnapoli
Copy link
Contributor

mnapoli commented Jan 5, 2015

Then why not removing the submodules and let developers checkout those plugins manually (or with a command or script)? PhpStorm can support all git repositories inside the project (e.g. I update with PhpStorm and it updates every plugin). The submodules brings no value except the initial installation.

@mattab
Copy link
Member Author

mattab commented Jan 5, 2015

Then why not removing the submodules and let developers checkout those plugins manually (or with a command or script)?

it will be error prone to clone all the repos.... maybe we could make it automatic? eg. maybe a new console command could init all repositories that the developer has access to and that we need to maintain as part of core?

@mnapoli
Copy link
Contributor

mnapoli commented Jan 5, 2015

For me removing submodules is a high priority because it's pissing me off every single day. Every time I pull master submodules are in detached state and I have to re-checkout master on most of them so that PhpStorm works again. It also messes up git rebase, and sometimes when I switch branch I end up committing submodule updates when I should not.

If we need to clone 8 repositories, that's doable. Here is a script:

git clone https://github.com/piwik/theme-PleineLune.git plugins/PleineLune
git clone https://github.com/piwik/plugin-SecurityInfo.git plugins/SecurityInfo
git clone https://github.com/piwik/plugin-TreemapVisualization.git plugins/TreemapVisualization
git clone https://github.com/piwik/plugin-VisitorGenerator.git plugins/VisitorGenerator
git clone https://github.com/piwik/plugin-CustomAlerts.git plugins/CustomAlerts
git clone https://github.com/piwik/plugin-TasksTimetable.git plugins/TasksTimetable
git clone https://github.com/piwik/plugin-LoginHttpAuth.git plugins/LoginHttpAuth
git clone https://github.com/piwik/plugin-QueuedTracking.git plugins/QueuedTracking

There would still be left ui-tests and the php tracker but at least that's a lot of time saved already.

And if we need to make sure those plugins are not broken, we don't need to check them out locally: we should use CI. #6542 Sounds like the solution.

@mattab
Copy link
Member Author

mattab commented Jan 5, 2015

For me removing submodules is a high priority because it's pissing me off

feeling the pain 👍

Here is a script:

maybe we have to clone using SSH not the HTTPS otherwise it won't use the SSH key (when two factor enabled?)

@mnapoli
Copy link
Contributor

mnapoli commented Jan 5, 2015

git clone git@github.com:piwik/theme-PleineLune.git plugins/PleineLune
git clone git@github.com:piwik/plugin-SecurityInfo.git plugins/SecurityInfo
git clone git@github.com:piwik/plugin-TreemapVisualization.git plugins/TreemapVisualization
git clone git@github.com:piwik/plugin-VisitorGenerator.git plugins/VisitorGenerator
git clone git@github.com:piwik/plugin-CustomAlerts.git plugins/CustomAlerts
git clone git@github.com:piwik/plugin-TasksTimetable.git plugins/TasksTimetable
git clone git@github.com:piwik/plugin-LoginHttpAuth.git plugins/LoginHttpAuth
git clone git@github.com:piwik/plugin-QueuedTracking.git plugins/QueuedTracking

@mattab
Copy link
Member Author

mattab commented Jan 5, 2015

maybe we can create some command like development:git-init or something better?

@mattab mattab modified the milestones: Piwik 2.11.0, Short term Jan 5, 2015
@mattab mattab removed the RFC Indicates the issue is a request for comments where the author is looking for feedback. label Jan 5, 2015
@sgiehl
Copy link
Member

sgiehl commented Jan 5, 2015

The reason why we had choosen submodules was to automatically include those optional plugins in every dev environment. I'm still thinking that it is necessary to do that. Otherwise no one would include them manually, even if it would be easy doable with console command.
And of course everyone would maybe use another "version" of those repos. With a submodule (or subtree) the version is given. Without that some errors might not be noticed.

Another question why did we start to move optional and required parts into submodules?
I'd prefer to do that with optional parts only. Required parts should be required using composer or something similar. The current way makes it difficult to deploy using git as some but not all submodules needs to be fetched, aswell.

I don't see so many problems with submodules that you probably do.

Small hint: To get all submodules to latest master I simply use:
git submodule foreach "git checkout master && git pull"

@mattab
Copy link
Member Author

mattab commented Jan 5, 2015

The reason why we had choosen submodules was to automatically include those optional plugins in every dev environment. I'm still thinking that it is necessary to do that. Otherwise no one would include them manually, even if it would be easy doable with console command.

for anyone who uses the Phpstorm then it can be configured to "git pull" all the repositories that Phpstorm finds in the project. So everyone will use the master branch of all plugins. If they don't use Phpstorm, maybe they could use the console ./console git:pull which could be edited to also git pull all repositories within the path?

Another question why did we start to move optional and required parts into submodules?

only the PiwikTracker submodule is actually required... there was no other easy way to stay BC while moving it to its own repository. See discussion in #6870 and matomo-org/matomo-php-tracker#1

Required parts should be required using composer or something similar.

definitely we'll do this whenever possible

The current way makes it difficult to deploy using git as some but not all submodules needs to be fetched, aswell.

it's true that the FAQ about Git deploy mentions php console git:pull but I don't think currently it will fetch the PiwikTracker submodule. I think this is a bug (maybe git:pull could automatically init the submodule if not yet init)...

@mnapoli
Copy link
Contributor

mnapoli commented Jan 5, 2015

to automatically include those optional plugins in every dev environment. I'm still thinking that it is necessary to do that.

We have to keep in mind there are "contributors" and "contributors". People working everyday on Piwik will check out those plugins, as well as other plugins too.

@sgiehl
Copy link
Member

sgiehl commented Jan 5, 2015

Hm. I'm not sure about that.
Another advantage of submodules: I'm sometimes switching between Piwik version tags to test issues. That might get more difficult when not having the submodules, as newer versions of those plugins might break older Piwik versions. With submodules automatically an older version of those plugins will be checked out.

@halfdan
Copy link
Member

halfdan commented Jan 6, 2015

We can still have some sort of version dependencies for plugins (e.g. using composer). I have to agree with what is said above that submodules cause more pain than good.

@sgiehl
Copy link
Member

sgiehl commented Jan 7, 2015

Can we use composer to require optional plugins? I don't think that is possible...

@mnapoli
Copy link
Contributor

mnapoli commented Jan 9, 2015

Following a discussion today, here is a suggestion: replacing submodules with git subtrees (≠ the whole process described in #6757)

We would still keep the current advantages of submodules:

  • checked out automatically in development
  • plugins hosted in separate repositories, allowing to be distributed on the marketplace

We would gain:

  • no more submodule update or git pull in submodules: such plugins would be like if they were included in piwik/piwik
  • branches are completely in sync: create a new branch in piwik/piwik and it is also created in subtrees
  • subtrees would be removed from the generated Piwik releases, so they would still be optional plugins on the marketplace

Cons:

To sum up, it would be just like if those plugins are inside piwik/piwik except they are also synced in a separate repo.

Thoughts?

@mattab
Copy link
Member Author

mattab commented Jan 10, 2015

+1 for using git subtrees for open source plugins that are released in the marketplace.

I would git-tree split all plugin-* repos in our Piwik Github org, except plugin-PiwikDebugger. That's even more than we currentl have submodules but I think they were not added as submodules because we forgot to?

maybe others have more thoughts?

@mnapoli mnapoli self-assigned this Feb 2, 2015
@mnapoli
Copy link
Contributor

mnapoli commented Feb 2, 2015

I have done some tests with git subtrees and the script I mentioned, it worked as expected.

You can see an example here:

I'm suggesting to try that with one of our plugins that we want always installed, for example VisitorGenerator.

To make this work we would need a server to setup a github hook that would sync the subtree on each push: @mattab I guess that's possible?

We would also need to update the package script to remove the subtrees when doing a release. That's doable too.

The only problem I've identified for now is that branches in piwik/piwik would be synced to plugins, but never deleted when they are deleted from piwik/piwik. This is an issue identified in dflydev/git-subsplit#6 and I'm sure we could implement that.

All good for everybody?

@mnapoli
Copy link
Contributor

mnapoli commented Feb 2, 2015

Oh one thing I just think about now: that would also synchronize the versions. So plugins setup as git subtrees would have the same versioning as Piwik itself :-/

I don't know if that's a good thing…

@tsteur
Copy link
Member

tsteur commented Feb 2, 2015

To make this work we would need a server to setup a github hook that would sync the subtree on each push: @mattab I guess that's possible?

We have this already for git diff emails, so no problem

So plugins setup as git subtrees would have the same versioning as Piwik itself :-/

That's not really a good thing since we want to manage and release them independently, there are less often actually changes so version number would less often increase but sometimes we want to release a new version before Piwik etc.

@mnapoli mnapoli modified the milestones: Mid term, Piwik 2.11.0 Feb 9, 2015
@mnapoli mnapoli removed their assignment Feb 9, 2015
@mnapoli
Copy link
Contributor

mnapoli commented Feb 9, 2015

A little summary:

  • git subtrees won't do it unfortunately :(
  • we don't see another "immediate" solution for submodules :(
  • if Plugins: support "require" other plugins #4485 is resolved using Composer to require plugins then maybe we'll get rid of submodules (so we keep this issue open)

@mattab mattab changed the title Eliminate submodules Eliminate plugins submodules Feb 10, 2015
@tsteur tsteur mentioned this issue May 5, 2015
@mattab
Copy link
Member Author

mattab commented Jul 25, 2015

We cannot get rid of submodules at this stage. We're considering automating tracking of submodules to latest commit, see issue #8425

@mattab mattab closed this as completed Jul 25, 2015
@mattab mattab added the wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it. label Jul 25, 2015
@mattab
Copy link
Member Author

mattab commented May 2, 2016

Maybe we could get rid of submodules using the upcoming open source tool described here: https://speakerdeck.com/fabpot/a-monorepo-vs-manyrepos ?

@mattab mattab reopened this May 2, 2016
@mattab mattab removed the wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it. label Jul 19, 2016
@mattab
Copy link
Member Author

mattab commented Oct 4, 2016

We likely won't get rid of submodules and if/when we do, we can reopen this issue or a new one

@mattab mattab closed this as completed Oct 4, 2016
@mattab mattab added the answered For when a question was asked and we referred to forum or answered it. label Oct 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
answered For when a question was asked and we referred to forum or answered it. c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. c: Tests & QA For issues related to automated tests or making it easier to QA & test issues. Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc.
Projects
None yet
Development

No branches or pull requests

7 participants