www-apps/radicale: Version bump to 2.1.8 #7274
Conversation
|
Pull Request assignment Areas affected: ebuilds www-apps/radicale: @maksbotan Bugs linked: 618724 In order to force reassignment and/or bug reference scan, please append |
gentoo-repo-qa-bot
added
assigned
|
As far as i understand the migration guide (have not tried it myself), you need 1.1.6 to export the data before switching to 2.x. (which does not seem to contain the migration tool anymore) If that is correct we need to warn the user before install, maybe even interactive. |
| exeinto /usr/share/${PN} | ||
| doexe radicale.wsgi | ||
| doexe radicale.fcgi | ||
|
|
There was a problem hiding this comment.
This could be done under the global USE-flag "cgi"
There was a problem hiding this comment.
Sorry, two questions about this. First, do you mean only the .fcgi file or the .wsgi file as well? Second, I thought this was in violation of the “don’t use USE flags to control installation of small files that don’t affect dependencies” rule (I don’t remember where that rule is documented, but I remember hearing something about it on the mailing list in the past)?
There was a problem hiding this comment.
The files first looked like they would only be needed when running radicale under an external webserver, maybe that is not even true and the builtin server uses them as well. So not sure whether they can/should be left out. The rule you suggest sounds sensible, just leave it like that.
| einfo "You will also find there an example FastCGI script." | ||
| ewarn "If you were previously using Radicale 1, manual migration is needed." | ||
| ewarn "Please see <http://radicale.org/1to2/> for details." | ||
| } |
There was a problem hiding this comment.
as mentioned in a comment in the PR, this should probably happen earlier so people can migrate data before merging
There was a problem hiding this comment.
That sounds a lot like a news item. I don’t see a Github mirror of gentoo-news.git; do you know if there is one or if there is another way for a non-developer to get news items added? Particularly, how to make that happen at about the same time as the new ebuild is committed?
There was a problem hiding this comment.
Sorry, no clue. I guess it can stay there and be a little more verbose. If the migration indeed requires 1.1.6 the ewarn should instruct people to downgrade again.
We need to make sure that:
- 2.x does not corrupt/delete old data so it can not be migrated anymore
- 1.1.6 stays in the tree for a long time
Or maybe, what we need can be implemented with blockers or slots, maybe in combination with a migration USE-flag.
There was a problem hiding this comment.
I’ve requested review on the gentoo-dev mailing list of a news item to be shown to all <radicale-2 users.
There was a problem hiding this comment.
Does the migration require using tools installed by v2 or is it done somehow else?
There was a problem hiding this comment.
einfo in ebuild removed in f94c56b due to news item being more appropriate. @mgorny, to answer your question, to migrate, you must have v1 installed, then you run v1 with --export-storage, then you upgrade to v2, then you run v2 and use the directory that --export-storage created as your storage location for v2. So no special tools outside Radicale itself, but you do need to run --export-storage while you still have v1 installed.
|
Reviewers, I have requested review on gentoo-dev of a news item to notify users of the need to export their database before installing the new version. Please don’t merge this PR until the news item has met the review period and you will commit the news item along with the ebuild. |
|
@Hawk777 could you please do the migration once and describe the implications here I see multiple open questions here:
|
| # directories | ||
| diropts -m0750 | ||
| dodir ${RDIR} | ||
| fowners radicale:radicale ${RDIR} |
There was a problem hiding this comment.
Any reason to do fowners instead of passing -o to diropts?
There was a problem hiding this comment.
Not really. I just copied the existing ebuild. Fixed in 6325aab.
|
|
||
| # directories | ||
| diropts -m0750 | ||
| dodir ${RDIR} |
There was a problem hiding this comment.
keepdir implies dodir, so there is no need for explicit dodir.
There was a problem hiding this comment.
I just copied the existing ebuild. Fixed in 6325aab.
| doins config logging | ||
|
|
||
| # fcgi and wsgi files | ||
| exeinto /usr/share/${PN} |
There was a problem hiding this comment.
Executable files don't belong in /usr/share.
There was a problem hiding this comment.
Here I’m not sure what the right answer is (and again the existing ebuild put them there which I just copied). I’m not sure if radicale.fcgi is useful to invoke on its own; I vaguely remember back when I used it I think I ran it through spawn-fcgi instead, not invoking it on its own, though I’m not too familiar with the business of how Python does FastCGI. Similarly executing radicale.wsgi on its own accomplishes absolutely nothing; it’s meant to be run through a WSGI container like uWSGI. So I don’t think they should be in $PATH. Maybe /usr/libexec? What do you think? I also discovered that, with uWSGI, radicale.wsgi doesn’t actually need execute permission, but I have no idea if that’s true with other containers. Thoughts? Should I leave it executable or make it not?
There was a problem hiding this comment.
Ok, is this something users put in their scripts via full path? If they do, then it's probably not worth the effort to move it right now.
As for executable bits, I think usually they aren't necessary but again, it depends on how it's used.
There was a problem hiding this comment.
Ok, is this something users put in their scripts via full path? If they do, then it's probably not worth the effort to move it right now.
I have Radicale deployed via uWSGI, and my config file contains --wsgi-file /usr/share/radicale/radicale.wsgi. So yes.
As for executable bits, I think usually they aren't necessary but again, it depends on how it's used.
If there’s any possibility of them needed, I would prefer to keep them set, to avoid running into issues like http://redmine.lighttpd.net/issues/2374—that was the opposite problem, but conceptually similar in that it was a file permission issue and required fixing file permissions by hand after every upgrade, which was annoying. Are you OK with leaving them executable? If any changes were to be made at all, I think it would be that radicale.wsgi could be non-executable, but radicale.fcgi would still need to be executable.
Hawk777
changed the title
|
In reply to @henning-schild’s questions:
No, not unless you run v1 with
Running v2 against a storage directory in the v1 format shows up as if it had no data. If you create some data, it will make a |
|
Marked WIP due to unsquashed fixup commits which I will squash once review is complete. |
|
Ok, so here's my idea: add a Remember to check |
When the user runs v1 with |
|
That is indeed not trivial. Even v1 data could be in another place if the default config was changed. And since it already sounds complicated and there seems no easy way to get it right, i would fall back to the original version. IMHO parsing configs is no option. @mgorny ? |
|
Oh, I didn’t realize the plan was to have the |
|
I was assuming the news item was still part of the WIP and not settled yet. einfo and ewarn are contained to this one PR and are IMHO enough. But now the issue is well understood, it does not matter much to me how the information gets brought to the user. |
|
The news item was posted on gentoo-dev for review. I removed the |
|
This is not going to be very helpful if you upgrade, then reboot (or restart the service) without noticing the problem. Using default paths should at least avoid the problem for some users. |
|
Isn’t that why we have news items? A way to communicate important information to users before they start installing things, as opposed to something like |
|
It is. But it doesn't change the fact that users don't care to read them ;-). |
|
In that case the ebuild should match the default location of a v1 database in pkg_pretend, given that we can clearly distinguish that from a default v2. If a v1 exists, refuse a v2 install, no matter if a v2 exists as well. So a migrating user will have to delete/move the old database. |
|
Something along the lines of d6b8f10? |
|
Given a v2 does not have .props as well that looks good. For all the users that do not read the entire message before acting, i would mention the backup before the deletion. |
|
My experimentation suggests that v1 creates a file named Order of messages fixed in 0bf07db. |
|
LGTM, but.
|
|
Sorry, missed that the first time. Fixed in a348ee0. Let me know if I’m done and OK to squash now. |
|
Looks good, please squash. |
Hawk777
changed the title
|
Squashed. Please don’t forget the news item! |
|
Could you rebase it against |
|
Done. |
| eerror " http://radicale.org/1to2/" | ||
| eerror "After conversion, please delete the old database (including hidden .props files)," | ||
| eerror "then copy the exported database into ${RDIR}." | ||
| eerror "You should finish with a ${RDIR}/collection-root directory." |
There was a problem hiding this comment.
Something to ponder upon, also in relevance to the news item: the upstream 'migration guide' is quite poor IMHO. The export stuff is basically mentioned in middle of paragraph without even a proper example, so I would suggest giving users some explicit suggestion, preferably something they could copy&paste. E.g.:
some-command --export-storage /var/lib/radicale.new
mv /var/lib/radicale /var/lib/radicale.old
mv /var/lib/radicale.new /var/lib/radicale
This sample covers conversion, and also backup. You can suggest upgrading, testing and removing the backup if it works.
There was a problem hiding this comment.
I guess the migration instructions would be best placed in the news item, not the ebuild, right? Because that’s where most people will see them first, and the ebuild can just tell people to go read the news item? I agree the guide is not all that helpful for Gentoo users, but then I think it’s written more for people who have customized their install paths and everything. If you agree that this should go in the news item, then I’ll post a new revision with more detailed instructions in it.
As an aside, would you be willing to sponsor my continued posting access to the mailing list? I don’t know when the posting restrictions will happen, but if they do happen before this all gets sorted out, then I would not be able to post the new revisions of the news item. Alternatively I could send the news item somewhere (to you?) to pass on to the list.
There was a problem hiding this comment.
I'd personally go for both, but up to you. As for ml, don't worry about it. When infra starts doing something, I'll probably look into including all active proxy-maint members.
There was a problem hiding this comment.
How does 7817dd8 look? If you think the instructions are fine, then I will convert them to news item format and, barring any further issues, squash the branch.
|
|
||
| PYTHON_COMPAT=( python{3_4,3_5,3_6} ) | ||
|
|
||
| inherit eutils distutils-r1 user |
There was a problem hiding this comment.
Please sort lexically whenever possible.
| doins config logging | ||
|
|
||
| # fcgi and wsgi files | ||
| exeinto /usr/share/${PN} |
There was a problem hiding this comment.
Ok, is this something users put in their scripts via full path? If they do, then it's probably not worth the effort to move it right now.
As for executable bits, I think usually they aren't necessary but again, it depends on how it's used.
| # fcgi and wsgi files | ||
| exeinto /usr/share/${PN} | ||
| doexe radicale.wsgi | ||
| doexe radicale.fcgi |
There was a problem hiding this comment.
Please combine this into one doexe with two files. Otherwise, people are going to repeatedly wtf before they notice that's fcgi/wsgi.
| } | ||
|
|
||
| pkg_postinst() { | ||
| einfo "A sample WSGI script has been put into ${ROOT}usr/share/${PN}." |
Hawk777
changed the title
|
Is this OK to squash again and send the new news item to gentoo-dev? |
|
Please do. I was actually waiting for you to send the item ;-). |
Bug: https://bugs.gentoo.org/618724 Package-Manager: Portage-2.3.19, Repoman-2.3.6
Hawk777
changed the title
|
Squashed and new news item posted. I guess this needs to wait another 72 hours before merge due to the news item. |
|
Pull request CI report Report generated at: 2018-03-31 00:49 UTC Issues already there before the PR (double-check them): |
|
Please ping us when it's ready to be merged. |
|
@henning-schild @mgorny That’s >72 hours now since the news item post, with no replies. This can go ahead whenever you’re ready. |
|
I did not really follow up, did not test the ebuild or read the news item. I trust your judgement and do not want to hold that up any longer, just go ahead. Thanks for pushing that forward! |
|
Merged, thanks! |
Bug: https://bugs.gentoo.org/618724
Package-Manager: Portage-2.3.19, Repoman-2.3.6