-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
ffsync: Update to syncstorage-rs #5942
Conversation
7e3a1d2
to
16da8d9
Compare
63370be
to
4d86ff5
Compare
b3924af
to
05adf10
Compare
I suggest you bring back discussion here when being PR specific (#5939 (comment)) I'll have to do a test run of my own to try to capture where the issue is. Something else must be missin. |
3ecfecc
to
0a6ff26
Compare
Thanks @th0ma7, I'll continue discussions here. I've been looking more into the current error being experienced which is:
Based on the above you mentioned that I may be missing:
Adding this either to the spk or the cross folder Makefiles does not resolve the issue. Following this I searched for where else in the codebase may have this implemented. I found it in These lines are of course present in the |
@mreid-tt with fixed python311 dependencies I get a different error:
So it looks like PyO3 is too old and cuts off the latest digit of 3.11 => 3.1 (PyO3 supports python >= 3.6) |
indeed, PyO3 v0.14.5 was released one month before Python 3.10 (the first release with two digit minor version). |
@hgy59, I have created two issues in the upstream repository based on our experiences with this PR: #1502: Compilation issues with version of PyO3 Do let me know if there are any other issues with the package before merging. Also in our wiki for the Makefile variables, I propose we add the following lines to the table:
I am not sure which section they should be added to (cross/ Makefiles?), so your guidance would be appreciated. |
Yes, this goes to the cross/Makefile section. |
Finally the naming should be consistently "Mozilla Sync Storage" instead of "Firefox Sync Server" (package description, wizard pages, et. al. ) |
I'm not sure this is a change we should make. Looking at the Mozilla Services Documentation, I see that there are a few services including:
Additionally, in the same documentation it describes the topic further:
So, my understanding is that sync server (in this rust implementation) is a combination of a token server and a storage service, so calling the whole the name of one part seems incorrect. Additionally, the main documentation page for setting one up describes it as a Sync-1.5 Server (as well as Firefox Sync server). Based on the above, to me the most accurate name would be Mozilla (or Firefox) Sync-1.5 Server. As such, I would propose we keep the naming as-is. |
Ok name it "Mozilla Sync Server" and leave the 1.5. |
@hgy59, I have revised the package naming and the wizards to "Mozilla Sync Server" and removed the 1.5 from the name. I also updated the package description to use the wording from the upstream website. I found that the description is a bit tricky (must escape special characters, but not enclose in double-quotes). I believe we are ready for final approval. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@hgy59, thanks for the green light. For publishing, based on the changes which affect other packages, can this be done via GitHub or do you have to publish it manually? |
Yes it can, look in the wiki online all the howto is the well written by @publicarray |
And forgot to mention, good job guys! |
OK, I've started a build action so we'll see what happens. EDIT: I ran it and it was successful and even though the package generated contained both a build of ffsync and python311, only the ffsync seems to be on the server at https://synocommunity.com/package/ffsync. So I guess it works! |
Description
This PR migrates the existing Firefox Sync Server (syncserver) to Mozilla Sync Storage (syncstorage-rs) built with Rust. This build will replace the package and all earlier versions which do not currently work will be removed.
Fixes #5939
Closes #2093
Closes #2747
Closes #2748
Closes #2816
Closes #2828
Closes #2873
Closes #2885
Closes #2979
Closes #2985
Closes #3124
Supersedes #3849
Download note
When obtaining a build from the 'Checks' tab, be aware that it will encompass builds for two other packages impacted by this pull request (PR). This is standard behaviour for build dependencies, and you can safely disregard these supplementary files.
Checklist
all-supported
completed successfullyType of change