Remove vendor folders

[rdeutz]

Current situation

Our vendor folder contains all 3rd part packages Joomla! depends on. This gives us a lot of headache when it comes to updating stuff or requesting packages e.g. for testing. A simple composer require will update also unrelated packages and you get hundreds of changes in PR.

We discussed the situation in different groups and voted in the production department on removing the folder contends for libraries/vendor. The voting passed with a majority so we are going to remove the folder in 4.0-dev soon. To be clear this doesn't effect the 3.x series of Joomla!

A direct impact of this change is that people can't clone the repository and have an installable Joomla! They have to make additional steps "composer install" + "npm install" and then they are good to go. We understand that this might be a burden for some people and that we have to explain what are the steps before and after cloning a repository. On the other side, git isn't a software distribution channel and this only infects the core and extension development. For testing and all other things we can use nightly builds or releases on download.joomla.org.

I have also removed the contents of media/vendor, there is no vote but it doesn't makes sense to just remove the PHP libs.

We have written a shot documentation explaining the steps https://docs.joomla.org/J4.x:Setting_Up_Your_Local_Environment

@mbabker can you have a look at the build script if I made the right changes?

Note: this is the 2nd try to remove vendor folders.

[HLeithner]

Iirc in the last discussion only the php deps get removed and not the js deps?

[mbabker]

This has to be done after the git archive command is run (line 97 pre-diff). That command is what creates the temporary space in build/tmp/<timestamp> based on a clean git clone from whatever remote is given, so doing the composer install in the repo root before extracting the git archive is going to be pointless.

We should also do composer install --no-dev --optimize-autoloader so we don't get dev dependencies in build packages.

We might also want to strip the extra files as a post install action (maybe make a Composer plugin for this?) that are in our .gitignore to keep us from shipping things like testing framework files, code examples (which has already bitten us in the past with the IDNA_Convert library), and other non-production assets.

[HLeithner]

I would add --ignore-platform-reqs too, so it doesn't require php ldap extension on the building host for example.

[dgrammatiko]

+1 for the clean up action

[dgrammatiko]

We still need this json, as the one in the main repo doesn't have the dependencies for the testing. That is why the tests are failing. Karma not found

[mbabker]

Another thing that has to be figured out (just remembered this).

Right now with the libraries/vendor directory checked in when we compute the changed files between releases we can also get a list of deleted files to be removed in our post-update scripts. Without the directory checked in we need to revisit how updates for that directory tree are handled to ensure outdated files are correctly removed as needed (IIRC in 3.x this was only needed when Symfony changed from PSR-0 to PSR-4 autoloading which removed a few extra nested directories, but this will be needed for the 3.x to 4.0 upgrade path at a minimum with all the changes happening in our dependency chain).

[brianteeman]

We have written a shot documentation explaining the steps https://docs.joomla.org/J4.x:Setting_Up_Your_Local_Environment

A prominent message about this change and with this link should be added to the readme.md

[rdeutz]

Thanks for the comments so far will work on the outstanding issues.

@HLeithner you're right that's why I wrote

I have also removed the contents of media/vendor, there is no vote but it doesn't makes sense to just remove the PHP libs.

not sure if you just commented or if you have arguments against removing them.

[HLeithner]

not sure if you just commented or if you have arguments against removing them.

just a comment, ignore it

[elkuku]

Big 👍
Please merge soon 😉

[alikon]

we already have few "testers" and it is really hard to find newer, even if I can understand the "ratio" behind this PR, for me this in this way we are restricting the potential testers user base

[brianteeman]

agree 100% with @alikon

[mbabker]

And let's be honest here, how many of said testers are actually working with a git checkout where they would actually have to run a couple of fairly standard commands (at least the Composer stuff is already required if you're doing code changes to our Framework repos or some of the .org apps (maybe that's why there's only 5 or 6 of us contributing there?), and soon the NPM stuff will be a requirement for those same .org apps), and how many are relying on things like patch tester and being able to install compiled packages to do their thing?

I'm half being sarcastic, and half being serious now. This might impact 10-15 people tops. Because there are so few who are actually working on the code with a full local environment set up.

It's 2018. We can't keep going around industry standard practices for development because it might make it easier for those people who aren't contributing to core to contribute to core.

And yes, I'm really trying to bite my tongue right now.

[Bakual]

I don't even see the reason for this, even after it was explained multiple times for me. I get that it's the "industry standard". I know it is and knew for a long time already and it makes sense for most of the industry for sure. No argument here.
I just don't think it makes sense for us. Our proiect is quite unique and not everything which makes sense for others also is best practice in our case.

Imho, we don't gain anything (besides following some "standards") but we loose a lot.

[mbabker]

Clearly excluding Composer vendor from Drupal’s repo or the required build step to work from the WordPress repo to get the JavaScript in place are hurting those projects and are crystal clear evidence that we should abandon this proposal at once. /sarcasm

On Fri, Jul 27, 2018 at 8:05 AM Thomas Hunziker ***@***.***> wrote: I don't even see the reason for this, even after it was explained multiple times for me. I get that it's the "industry standard". I know it is and knew for a long time already and it makes sense for most of the industry for sure. No argument here. I just don't think it makes sense for us. Our proiect is quite unique and not everything which makes sense for others also is best practice in our case. Imho, we don't gain anything (besides following some "standards") but we loose a lot. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#21217 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAWfoRW8WKeIPX8C4QK-Wg3O4PYic_hyks5uKxAJgaJpZM4VaCf-> .

-- - Michael Please pardon any errors, this message was sent from my iPhone.

[alikon]

as usual it's a trade-off choice
i've no numbers to share with you, i can just share my little experience trying to involve more people to contribute to the project,

if it is an "headache" for skilled developers i can imagine they can survive from this as it is everyday-work

what i cannot imagine "easily" is how to teach newbies to this "right" path not all people/end-users are so skilled, imho before this right move, we should spent a little bit of time to spread the word, teach not-so-skilled people, making a youtube video and so on.... i'm not against the concept.... i'm against the way

[Bakual]

Both Drupal and Wordpress are quite different from our project. I'd say the contributors and testers (as well as users) of Drupal are much more leaning toward developer. For Wordpress maybe not so much, I don't know that.
Afaik both projects also have paid core developer team.

So while they both are an OpenSource CMS like we are, their ecosystem is quite different.

[brianteeman]

Have the conversation with Drupal about their upcoming switch from patch files to gitlab. And they're allegedly a more technical user base.

[rdeutz]

however you feel, the decision was made weeks ago and this here is only the implementation

[brianteeman]

And that decision was made by whom and where? I don't see any report about this on the jvp

[alikon]

so what , simply merge it, who cares about it ... why you make it as PR then?

[rdeutz]

And that decision was made by whom and where? I don't see any report about this on the jvp

it was made by the production department via email

so what , simply merge it, who cares about it ... why you make it as PR then?

to test the change as we always do

[mbabker]

Afaik both projects also have paid core developer team.

Both projects have corporate entities providing support for their ecosystem, whose staff may also use some of that paid time in conjunction with contributions and work on the open source CMS platforms. It is incorrect to say that WordPress and Drupal have paid core staff, Automattic and Acquia do support their respective projects but neither company has (to the best of my knowledge) any type of exclusive contract with someone to be paid full time for work on the open source CMS platforms (I personally pay more attention to WordPress as it's more relevant in my day-to-day work and the people I know are employees of Automattic and working on WordPress are working throughout their ecosystem (i.e. marketing and .org infrastructure) with very little time on the CMS itself).

I'll keep it simple from here. If command line is scary for you, you don't have to use it. We provide tools to install and update Joomla without it (in part because nobody has taken command line support in Joomla seriously before 4.0 because we're still focused on the 2008 architecture of generating HTML documents with very little focus on doing anything else), and we will continue to provide the tools to test changes without it (even if that PR testing platform doesn't launch anytime soon, it is probably an hour of my time, donated free of charge because I do not get the luxury of being paid for any of my time contributing to this project by any source, to set up a CI job somewhere that creates installable packages that can emulate what the testing platform would provide). If you're actually making changes to the CMS, the workflow is a little easier in some cases. As far as what you have to provide in a pull request, removing certain compiled files means less conflicts and less time spent dealing with those. Removing vendor files precludes the temptation of patching those files in their non-canonical location (this repository) and forces contributors to go to their canonical source to make the change (and newsflash, everything in libraries/vendor requires you to composer install something, we do in the Framework repos because we're doing better about requiring tests and you have to install PHPUnit).

Who does this pull request hurt? Anyone who crazily tried just cloning this repo and running a site from it. As far as production goes, that's impossible; our upgrade tools only support one path, com_joomlaupdate (and don't give me that crap about "Database > Fix" can take care of you, that is NOT a solution). It hurts anyone who is unwilling or unable to run a couple of industry standard (yes, that term that seems to be disliked) tools to have a fully functional environment. At least with NPM, if you have EVER worked on some kind of UI thing where you deal with minified JavaScript or LESS/SCSS compilation into "plain" CSS (outside of Joomla anyway), you have at some point in your life run npm install and some combination of npm run prod, grunt, or gulp, and if those are foreign concepts to you that's fine but it means you aren't working with the technologies that we're trying to use in core Joomla development or are pretty regular things to use anywhere in the frontend space.

So forgive me for having a "coder's mind" (something that seems to be a rarity in this project anyway) and agreeing that following best practices is actually a good thing.

[alikon]

ok, so, i can only tell you that i disagree on this choice, furhermore made in this way
but after all, who am i to disagree...

[Bakual]

As far as what you have to provide in a pull request, removing certain compiled files means less conflicts and less time spent dealing with those.

Yeah, I see that one argument as the advantage. But honestly, nobody solves conflicts in those files. All you do is recompile the file after you merged upstream. Big time saver indeed for those rare cases someone else is touching that same JS or CSS file.
On the other hand, each tester will have to do it after applying a PR (except if that testing platform every flies).

Removing vendor files precludes the temptation of patching those files

Since any change to those vendor files would be automatically reverted next time anyone runs composer install, it already is pointless to change anything in there regardless if you commit the files or not. So whether the files are in the repo or not makes absolutely no difference in that regard.

The only difference I personally see is that we don't get a PR updating 1000+ files when we periodically run composer install. With this PR, it will be only the composer lock (?) file that gets updated.
But then, nobody is reviewing those PRs anyway as it is impossible to do either way.

Who does this pull request hurt? Anyone who crazily tried just cloning this repo and running a site from it. As far as production goes, that's impossible; our upgrade tools only support one path, com_joomlaupdate (and don't give me that crap about "Database > Fix" can take care of you, that is NOT a solution).

Actually, you can run dev and testsites (not productive!) quite fine from a git clone. The database fixer takes care of most things just fine.
Every now and then you need to do a fresh install thought because as you said the upgrade path doesn't work reliable. But it works surprisingly fine and to be honest my dev and test sites usually don't have a long life to begin with. I do a fresh install more often because I need clean data than because the installation was broken.

I'm sure I'm not the only one here.

[mbabker]

With this PR, it will be only the composer lock (?) file that gets updated.
But then, nobody is reviewing those PRs anyway as it is impossible to do either way.

We shouldn't be either. At most we should just be checking that our environment hasn't broken by updating third party libraries (and I generally try to point out exactly what packages are updated and what it would impact when I do those PRs). If we have to code review external code, something is really wrong.

[HLeithner]

wouldn't it help to use absolute versions on 3rd party libs in composer and npm?

I don't like composer and I don'e like npm, at least I can work with it if there is no other solution. At least composer is not a problem on a server. Only npm sucks because normally there is no node on my servers only php and some other necessary languages.

So back to my first question. Wouldn't is solve the main problem if only one person maintains the composer and npm versions?

But once again I'm not against this step if it makes it easier for all developer, as already pointed out. All testers using com_patchtester or nightly builds with .patch files doesn't need composer and npm.

[mbabker]

wouldn't it help to use absolute versions on 3rd party libs in composer and npm?

It's a twofold thing. On one hand, the lock file should guarantee a consistent environment and the Composer/NPM manifests can declare minimum supported packages (we'd also have to do a better job declaring and maintaining those minimums, I guarantee you the CMS' minimums aren't accurate in 3.x; in the Framework repos we do a test pass with the --prefer-lowest flag to validate these). On the other hand, putting it right into the main manifest means we lock the packages consistently and only transient dependencies rely on the lock file to get to the right state (we depend on PHPUnit but don't explicitly list all its dependencies).

FWIW I don't list specific versions in my apps and use SemVer ranges, but those are also not distributed the same way Joomla is.