Open wgevaert opened 1 year ago
I have the same issue here. When do we have news about this issue!?
Can someone look into this?
Hi, @wgevaert I've tested your code locally and it doesn't work I have the same error here and more...
@reedy This solves my problem with duplicate includes.
see above
Fatal error: Cannot redeclare <some function/class> (previously declared in <directory>/extensions/<name>/<file>:<line>) in <directory>/extensions/<name>/<file> on line <line>
tested by modifying my composer.json file that way:
diff --git a/composer.json b/composer.json
index 9cd6b1ed536..27d9750e402 100644
--- a/composer.json
+++ b/composer.json
@@ -20,6 +20,11 @@
"wiki": "https://www.mediawiki.org/"
},
"prefer-stable": true,
+ "repositories": [
+ {
+ "url": "https://github.com/wgevaert/composer-merge-plugin.git",
+ "type": "git"
+ }],
"require": {
"composer/semver": "3.3.2",
"cssjanus/cssjanus": "2.1.1",
@@ -57,7 +62,7 @@
"wikimedia/cdb": "2.0.0",
"wikimedia/cldr-plural-rule-parser": "2.0.0",
"wikimedia/common-passwords": "0.5.0",
- "wikimedia/composer-merge-plugin": "2.1.0",
+ "wikimedia/composer-merge-plugin": "dev-bugfix-don-t-include-required",
"wikimedia/html-formatter": "3.0.1",
"wikimedia/ip-set": "3.1.0",
"wikimedia/ip-utils": "4.0.0",
The only issue I see with this patch is that it silently fixes problems occured somewhere else. Maybe adding a warning on duplicate values would resolve the concerns by the package maintainers. I hope after summer break we will get some feedback
I do give a "warning" here, but currently it is "info" (only seen when running composer in verbose mode). Should I change to "warning"? It might be spammy.
Sorry, I was not clear. With this, I was referring to the change to composer.
Change #251 should be merged as it is, I think.
After thinking about it for a bit, I asked myself if there are other extensions (not SMW) that cause this problem. Otherwise, one could discuss the way how SMW is installed. Why is it different from other extensions, that you just download and where you install the dependencies via composer? Or if it insists on being installed via composer, why can it not be stored in the vendor folder?
After thinking about it for a bit, I asked myself if there are other extensions (not SMW) that cause this problem. Otherwise, one could discuss the way how SMW is installed. Why is it different from other extensions, that you just download and where you install the dependencies via composer? Or if it insists on being installed via composer, why can it not be stored in the vendor folder?
SMW is not the only extension that can be installed using composer, see this page
E.g. the following can also be installed using composer: https://packagist.org/packages/mediawiki/semantic-result-formats https://packagist.org/packages/mediawiki/chameleon-skin (in skins folder, not extensions folder) https://packagist.org/packages/mediawiki/simple-batch-upload https://packagist.org/packages/mediawiki/semantic-scribunto https://packagist.org/packages/mediawiki/sub-page-list https://packagist.org/packages/mediawiki/semantic-extra-special-properties https://packagist.org/packages/mediawiki/parser-hooks https://packagist.org/packages/mediawiki/page-forms https://packagist.org/packages/professional-wiki/network https://packagist.org/packages/professional-wiki/modern-timeline https://packagist.org/packages/mediawiki/vector-skin (in skins folder instead of extensions folder) https://packagist.org/packages/mediawiki/semantic-meta-tags https://packagist.org/packages/mediawiki/data-transfer https://packagist.org/packages/mediawiki/cargo https://packagist.org/packages/mediawiki/bootstrap-components
To name a few.
Thank you. This means that there is a critical mass to care about. However, https://phabricator.wikimedia.org/T250406 is not decided.
With this set up, the following
composer.local.json
s give problems:{ "require": { "mediawiki/user-functions": "dev-REL1_35" }, "extra": { "merge-plugin": { "include": [ "extensions/*/composer.json" ] } } }
From the perspective of what is officially supported, I believe the "problem" is the inclusion of "mediawiki/user-functions"
to fetch extensions. Per RFC T467 this is not officially supported. If you remove that, and make sure to have a Git clone of the extension, then everything should automatically work.
Having said that, there's clearly a specific subset of extensions that have never stopped advertising and internally making use of Composer to manage their own installation. I think for people that prefer that, that can probably unofficially work. But, then you must remove the use of "extensions/*/composer.json"
and instead fully embrace the "Composer way", i.e. use "require"
to list out each extension you want to install (if it is published there), and then for extensions that aren't published there (i.e. most) you clone them from Git and list them by name under "include" — not with a wildcard.
There is no concept native to Composer of magically by-wildcard depending on packages that happen to exist on disk. If you want to use Composer to install extensions, then I think you have to go all-in on that and list out each extension want to install (via "require" or "include", no wildcard) in your mediawiki/composer.local.json
file.
There is one other thing that stands out to me, which I don't fully understand:
Make a
composer.json
similar to this:{ … "extensions/*/composer.json" … }
With this set up, the following
composer.local.json
give problems:{ … "extensions/*/composer.json" … }
Do I understand correctly that this first composer.json
file is the mediawiki root composer.json? I have not seen before that someone would modify this, and certainly not include extensions/*/
there as well. If I understood this correctly, may I ask what the motivation is for adding a wildcard in both files? It believe we usually put the wildcard (only) in composer.local.json
.
fwiw, I agree with this from @Krinkle:
you must remove the use of "extensions/*/composer.json" and instead fully embrace the "Composer way", i.e. use "require" to list out each extension you want to install
@Krinkle, I understand that various problems can be resolved with this PR. I also do not fully understand all the problems described here. However, without any extensions downloaded the following composer.local.json leads to
{
"require": {
"mediawiki/semantic-media-wiki": "~4.1"
},
"extra": {
"merge-plugin": {
"include": [
"extensions/*/composer.json"
]
}
}
}
Fatal error: Cannot redeclare <some function/class> (previously declared in <directory>/extensions/<name>/<file>:<line>) in <directory>/extensions/<name>/<file> on line <line>
when running the updater or visiting the wiki.
@physikerwelt Yes, my recommendation is to follow either the officially supported way, or to use Composer. Right now it is mixing them, which leads to the error.
The supported way, as documented in composer.local.json-sample
, is for the local file to contain only the following:
{
"extra": {
"merge-plugin": {
"include": [
"extensions/*/composer.json"
]
}
}
}
This means you control what's installed by cloning extensions, and should work for all extensions, including SMW.
The Composer way (which is not officially supported, but should work!), is to specify each extension you want to install separately. This is happens naturally when you use "require"
as wildcards are not valid there. For non-composer extensions, you'd list them in the "include"
like so:
{
"require": {
"mediawiki/semantic-media-wiki": "~4.1"
},
"extra": {
"merge-plugin": {
"include": [
"extensions/AbuseFilter/composer.json",
"extensions/AntiSpood/composer.json"
]
}
}
}
I haven't reviewed the patch in detail, but while I believe it would likely be safe and not break any existing installs, my worry is that removes a useful ability to detect mistakes, unless this is the only possible way to trigger that error? Otherwise, it would silence the one way to detect such an issue.
The related change to warn about duplicate files has been merged https://github.com/composer/composer/pull/11109 . However, the problem seems to be still there. When thinking about this again, I am wondering why SMW installs itself to extensions and not to vendor.
When thinking about this again, I am wondering why SMW installs itself to extensions and not to vendor.
Because it is a mediawiki extension. See the composer-installer plugin: packages with "type": "mediawiki-extension"
are installed into the extension folder, and mediawiki-skin
to the skin folder.
When thinking about this again, I am wondering why SMW installs itself to extensions and not to vendor.
Because it is a mediawiki extension. See the composer-installer plugin: packages with
"type": "mediawiki-extension"
are installed into the extension folder, andmediawiki-skin
to the skin folder.
This is how it is implemented, I was wondering why it was done.
While SMW has quite an extensive installation guide, there is no mention of installing it from git. But it works by cloning https://github.com/SemanticMediaWiki/SemanticMediaWiki/. So for me, there is no issue anymore.
This solves several issues, including but not limited to the following: Make a
composer.json
similar to this:With this set up, the following
composer.local.json
s give problems:Gives the error
Fatal error: Cannot redeclare <some function/class> (previously declared in <directory>/extensions/<name>/<file>:<line>) in <directory>/extensions/<name>/<file> on line <line>
, as described in this issue on this repo and this issue in the composer repoGives the error
which is also discussed in this issue on this repo (Which also says "[..] it would be good if there was some way to indicate not to merge the configs of any packages that are already included by the main composer.json file since their configs will already be evaluated by Composer when they're required in the main composer.json file.", which is what this pull request does). Solving the issue by using the
merge-replace: false
option gives the sameFatal error: Cannot redeclare
problem described above.This pull request solves all these issues.