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

Rename composer.json.dist to composer.json #9279

Merged
merged 2 commits into from
Apr 3, 2024

Conversation

mvorisek
Copy link
Contributor

@mvorisek mvorisek commented Dec 19, 2023

needed for #9241, for using this project/libs as a composer/Packagist dependency in general

composer.json Outdated Show resolved Hide resolved
@alecpl
Copy link
Member

alecpl commented Dec 23, 2023

This will make working directly on source code more complicated. I normally have my composer.json file with additional packages/plugins added. I won't be able to do that. I need to investigate what options there are to fix that. One I found is use of COMPOSER env pointing to a different file.

@mvorisek
Copy link
Contributor Author

mvorisek commented Dec 23, 2023

COMPOSER env. var [1] is for root package [2] only even if the docs [1] do not mention that explicitly. Dependencies require to use composer.json strictly otherwise they cannot be installed. IFAIK there is no workaround and this PR is needed.

[1] https://getcomposer.org/doc/03-cli.md#composer
[2] https://getcomposer.org/doc/04-schema.md#root-package

@mvorisek mvorisek force-pushed the rename_composer_json_dist branch 2 times, most recently from 1a76637 to 8fc3bae Compare January 2, 2024 12:42
bin/installto.sh Outdated
@@ -79,7 +79,8 @@ if (strtolower($input) == 'y') {
}

foreach (['index.php', 'config/defaults.inc.php', 'composer.json-dist', 'jsdeps.json', 'CHANGELOG.md', 'README.md', 'UPGRADING', 'LICENSE', 'INSTALL'] as $file) {
$command = 'rsync -a --out-format=%n ' . INSTALL_PATH . "$file $target_dir/$file";
$source_file = $file === 'composer.json-dist' ? 'composer.json' : $file;
$command = 'rsync -a --out-format=%n ' . INSTALL_PATH . "$source_file $target_dir/$file";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ensures the composer.json is downloaded still to composer.json-dist as expected by https://github.com/roundcube/roundcubemail/blob/1.6.5/bin/update.sh#L177 update script.

I couldn't find more places to fix, but please review this PR carefully, there might be more install/update usecases/docs I might be not fully aware.

@mvorisek mvorisek requested a review from alecpl January 2, 2024 12:43
@alecpl
Copy link
Member

alecpl commented Jan 6, 2024

I created composer/composer#11784 but I don't expect it to be implemented soon or at all.

The package I mentioned there is not of a great quality. However, I tested the COMPOSER variable and it looks like it might actually work. But it's not an easy decision.

@mvorisek
Copy link
Contributor Author

@alecpl can I ask you to review this PR? Based on #9279 (comment) and composer/composer#11784 the composer.json name is the only one name possible to make this project working with composer natively.

@mvorisek mvorisek force-pushed the rename_composer_json_dist branch from 8fc3bae to 20e5797 Compare March 17, 2024 18:22
@mvorisek
Copy link
Contributor Author

I have rebased this PR. @alecpl, can you please check? According to the sources above, there are no other options, the only supported filename by composer is composer.json and the place that needs to be adjusted is the distrubution/upgrade.

@alecpl alecpl merged commit bdd5de5 into roundcube:master Apr 3, 2024
15 checks passed
@mvorisek mvorisek deleted the rename_composer_json_dist branch April 3, 2024 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants