-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
add composer.json #12834
add composer.json #12834
Conversation
Fix for joomla#12830
We purposefully don't ship core's |
But it works at current time.
And it causes issues at current time.
Yes, overwriting on update is a point. Let's make |
Either way I still would not add to the package any file that encourages developers to change the libraries/vendor directory. Our upgrade process will break your local install, there's no way around it. Having Joomla in a state where developers can freely add packages to that directory is a lot more than just including the Composer manifest in the package, that's the main point I want to make here. There are a lot of other things that need to be taken into consideration before we either remove the exclusions for |
So using composer is a core hack, hm?
Yes, until I call Well if try to think further: updater can look into |
Still not that simple.
No. Altering the core distributed Composer install (in libraries/vendor) is a core hack. Providing your own Composer managed library in another path isn't. All you're really doing to use that separate library is adding another autoloader to PHP's queue to point to that path. Look, I want to find a solution that allows developers to add stuff to the main Composer path. We couldn't come up with something when we started down this road almost two years ago and we still don't have one. Until we come up with a safe solution that doesn't cause an upgrade to break sites, I can't in good faith suggest merging anything to core that makes it easier/possible to modify the Composer installation shipped with core. I'm legitimately not trying to block this because I don't think devs should be able to touch the Composer install, but right now Joomla is not in a state where it can support this type of system interaction so we need to stick with the stance of not encouraging or supporting it until we have found the right solution. If you really choose to go against this advice though and make alterations to the Composer installation, instead of having a half-complete (and duplicated) |
Can you take a look at this sketch About timeouts or anything like this: I'd like to make a point one more time: |
Irrespective of @mbabker comments above your composer.json file is incorrect
will update a joomla install to a version of simplepie that is greater than the one we currently ship with. Anything that updates a joomla install to a version different to the one than the version the joomla install ships with cant be a good idea. It would be impossible to support end users as we wouldnt know what version of the libraries they are using. |
Ok, no problems - we can go with concrete version. Other way you Joomla will rewrite autoloader, or composer will uninstall libraries. |
So honestly on this whole Composer thing, I would seriously look into how Drupal has dealt with it. https://www.drupal.org/docs/develop/using-composer/using-composer-with-drupal looks like the central resource to start from at an end user perspective, someone will need to spend some more time researching to find their workflows and upgrade processes dealing with the Composer dependencies since as that document states their vendor directory is not checked into version control, meaning if they do package the Composer installation the approaches they'll have taken will be vastly different to our current setup. The libraries now can be freely updated because our base CMS manifest doesn't use explicit version numbers except for a few specific cases. So what this basically says is that the application is expected to work with certain minimums and feasibly anything newer. As an example the Registry dependency says 1.4.5 is the minimum supported version and we have 1.5.2 in the CMS right now, our manifest says that any newer version should still be supported unless there is a bug or B/C break in future releases of the upstream package on the same major version branch (same goes for all packages; so for example with our manifest as is someone could not pull in Symfony's YAML component on a 3.x release even though from an API perspective in the Registry package it is 100% compatible). If we want to address the "issue" of developers being able to freely upgrade those dependencies, the only option is to manually lock the package versions in the Composer manifest and as part of the update process we change that as well. That's really not efficient (I don't know what best practice is for that actually as it pertains to applications versus libraries). Anyway, looping back to what this PR is all about. I still suggest it being closed for the time being. Joomla does not support developers altering the Composer installation and upgrades will break it for those who are altering it. An issue should be opened (and stay open) as a collaboration space to work through this, and honestly one should have been open since we merged the Composer structure into the 3.4 development branch something like two years ago. @alex7r As for your sketch, at a quick glance it does look like it would work, but honestly I feel it would only be efficient for command line based upgrades (which Joomla doesn't support either). Composer's processes aren't optimized to deal with the restrictions of HTTP requests, that's part of the reason I feel even if we could address this issue we couldn't add a hook to the update system to automagically try to upgrade the Composer installation. Maybe one of the third party integrations for Composer helps to deal with this, I can't say I've looked into them though. Just to give an idea, I went ahead and ran an update of all the development and production dependencies from the current staging branch with Composer's profiling enabled, the full output of that is at https://gist.github.com/mbabker/e1236435c6113058a887844a866a1f77. Now, the results of this are partially skewed because of the profiler, output verbosity, and the fact I have most of these packages in cache already (so they don't need to be downloaded which wouldn't be the case on a lot of servers). As you can see this process is already eating a lot of memory and goes just over 30 seconds, which is the default timeout if I'm not mistaken for a PHP web request. So we already have memory and timing issues to cope with running it as one single command. |
@mbabker Well, serious? May be only from Cli? The whole sketch is about doing without Cli, from Http request only. Timeout: timeout can affect you when you redirect user to this page. And if you request it with xhr or any - you can trigger it multiple times until you get 200 code response. It will update 2 of 10 and go timeout on first request, 2+2 of 10 on next and further until 10 of 10. Profit. One again points of this issue:
|
Yes, only from CLI. Composer isn't designed to be run on shared hosts from a web request, you're fighting with a lot of crap configurations. So now you have to do a lot of workarounds just to include this into core as a default behavior; things like updating one package at a time. What you're proposing might work on web when you as a developer have more fine tuned control over the server environment, but as a generalized behavior not so much. I'm not trying to talk past you, disrespect you, or anything like that. But speaking quite frankly, Joomla is not in a technical position to support developers adding their additional libraries to the One additional point. The inclusion of a So again, this is not just as simple as adding a file to the package and adding a hook to the upgrade system. There is a lot more that has to be thought through on this for it to work sufficiently enough to be included in core. Until we have addressed all of those issues sufficiently, anything else to me is just asking for trouble. |
I'm closing this PR as it will not be merged for sure. |
Ok, wait a minute. I believe you're looking quite too straight into this one. Read whole message please before thinking that something statement is wrong... Once again:
All the above means:
|
@Bakual Google - is a great company with great products. But Groups is 👎 |
Pull Request for Issue #12830.
Add composer.json so composer update would not uninstall Joomla Core