Skip to content

Fix upgrade - #219

Open
CodeShakingSheep wants to merge 25 commits into
YunoHost-Apps:testingfrom
CodeShakingSheep:no-config-override
Open

CodeShakingSheep wants to merge 25 commits into
YunoHost-Apps:testingfrom
CodeShakingSheep:no-config-override

Conversation

@CodeShakingSheep

Copy link
Copy Markdown
Member

Don't replace config file

Problem

After the last update the contextmenu extension wasn't working anymore as the config/config.inc.php file had been overridden from the template file leading to the installed extensions not being present anymore after this line: https://github.com/YunoHost-Apps/roundcube_ynh/blob/master/conf/config.inc.php#L140

Solution

Keep config/config.inc.php and don't override it with the template.

PR Status

  • Code finished and ready to be reviewed/tested
  • The fix/enhancement were manually tested (if applicable)

Automatic tests

Automatic tests can be triggered on https://ci-apps-dev.yunohost.org/ after creating the PR, by commenting "!testme", "!gogogadgetoci" or "By the power of systemd, I invoke The Great App CI to test this Pull Request!". (N.B. : for this to work you need to be a member of the Yunohost-Apps organization)

Don't replace config file
@CodeShakingSheep
CodeShakingSheep changed the base branch from master to testing August 10, 2024 20:13
@CodeShakingSheep

Copy link
Copy Markdown
Member Author

!testme

@yunohost-bot

Copy link
Copy Markdown
Contributor

🌻
Test Badge

@yunohost-bot

Copy link
Copy Markdown
Contributor

📚
Test Badge

Keep composer.json
@CodeShakingSheep

Copy link
Copy Markdown
Member Author

!testme

@yunohost-bot

Copy link
Copy Markdown
Contributor

Alrighty!
Test Badge

@yunohost-bot

Copy link
Copy Markdown
Contributor

🐛
Test Badge

Keep composer.lock
@CodeShakingSheep

Copy link
Copy Markdown
Member Author

!testme

@yunohost-bot

Copy link
Copy Markdown
Contributor

May the CI gods be with you!
Test Badge

@yunohost-bot

Copy link
Copy Markdown
Contributor

📚 🐛
Test Badge

--keep option doesn't support wildcards
@CodeShakingSheep

Copy link
Copy Markdown
Member Author

!testme

@yunohost-bot

Copy link
Copy Markdown
Contributor

📖
Test Badge

@yunohost-bot

Copy link
Copy Markdown
Contributor

Meow 🐈
Test Badge

@CodeShakingSheep

Copy link
Copy Markdown
Member Author

Meow 🐈 Test Badge

Upgrades fail because of an already existing table collected_contacts. Created a PR for it sblaisot/automatic_addressbook#31

@CodeShakingSheep
CodeShakingSheep marked this pull request as draft August 28, 2024 21:00
@ericgaspar

Copy link
Copy Markdown
Member

!testme

@yunohost-bot

Copy link
Copy Markdown
Contributor

🚀
Test Badge

@ericgaspar

Copy link
Copy Markdown
Member

!testme

@yunohost-bot

Copy link
Copy Markdown
Contributor

📖 🪱
Test Badge

@CodeShakingSheep

Copy link
Copy Markdown
Member Author

Hey @ericgaspar ,
Thanks for all your work with roundcube packaging! The RC plugin which makes the upgrade test fail is https://github.com/sblaisot/automatic_addressbook which has been last updated 10(!) years ago. Back then I created a PR in the repo (sblaisot/automatic_addressbook#31) but the project seems completely dormant.

Anyways, since Roundcube 1.5 email addresses of recipients are collected automatically, see https://kolabian.wordpress.com/2020/09/26/collected-recipients-and-trusted-senders/ and roundcube/roundcubemail#6904 . So, what the problematic plugin did became a Roundcube core feature. Therefore, I'd suggest to just remove the plugin. What do you think?

Also, I saw that you updated PHP version to 8.5 in your latest PR. That might be a bit too soon as full PHP 8.4 support will come with RC 1.7, see https://roundcube.net/news/2025/07/14/roundcube-1.7-beta-released . So, I think in 1.6 we should stick to PHP 8.3 for now.

@CodeShakingSheep

Copy link
Copy Markdown
Member Author

!testme

@yunohost-bot

Copy link
Copy Markdown
Contributor

📖 🪱
Test Badge

@CodeShakingSheep

Copy link
Copy Markdown
Member Author

!testme

@yunohost-bot

Copy link
Copy Markdown
Contributor

Remember: a failing test is just a misunderstood spicy feature!
Test Badge

@CodeShakingSheep
CodeShakingSheep marked this pull request as ready for review April 6, 2026 22:43
@CodeShakingSheep

Copy link
Copy Markdown
Member Author

@ericgaspar I updated this branch and added some fixes. This should fix #259 . Can you merge it?

@CodeShakingSheep

Copy link
Copy Markdown
Member Author

!testme

@yunohost-bot

Copy link
Copy Markdown
Contributor

Fingers crossed!
Test Badge

@CodeShakingSheep

Copy link
Copy Markdown
Member Author

Just upgraded roundcube from version 1.16.14~ynh1 with this branch. Upgrade went through smoothly, no issues with plugins afterwards.

@CodeShakingSheep

Copy link
Copy Markdown
Member Author

!testme

@yunohost-bot

Copy link
Copy Markdown
Contributor

Remember: a failing test is just a misunderstood spicy feature!
Test Badge

Comment thread scripts/upgrade Outdated

# Download, check integrity, uncompress and patch the source from app.src
ynh_setup_source --dest_dir="$install_dir/tmp_aaa"
ynh_setup_source --dest_dir="$install_dir" --full_replace=1 --keep="composer.json composer.lock config/ logs/ plugins/ vendor/"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I disagree with this approach, what was done was done because that's what official doc suggests.

I know there are rough edges around the fact that /bin/env php returns wrong PHP CLI, but this is not the way to fix it.

@CodeShakingSheep

Copy link
Copy Markdown
Member Author

!testme

@yunohost-bot

Copy link
Copy Markdown
Contributor

Remember: a failing test is just a misunderstood spicy feature!
Test Badge

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.

4 participants