firefox syncserver service: fix PYTHONPATH #28188
Conversation
|
@Nadrieril, thanks for your PR! By analyzing the history of the files in this pull request, we identified @nbp, @fpletz and @wkennington to be potential reviewers. |
| @@ -124,7 +124,13 @@ in | |||
| description = "Firefox Sync Server"; | |||
| wantedBy = [ "multi-user.target" ]; | |||
| path = [ pkgs.pythonPackages.pasteScript pkgs.coreutils ]; | |||
| environment.PYTHONPATH = "${pkgs.pythonPackages.syncserver}/lib/${pkgs.pythonPackages.python.libPrefix}/site-packages"; | |||
There was a problem hiding this comment.
Why is PYTHONPATH used? I suggest creating a single Python environment.
There was a problem hiding this comment.
I don't know what you mean by that. How would I do it ? I'm not very familiar with python+nix
| extraLibs = [ pkgs.pythonPackages.syncserver ]; | ||
| ignoreCollisions = true; | ||
| }; | ||
| in "${penv}/${pkgs.python.sitePackages}/"; |
There was a problem hiding this comment.
I think it would be simpler to add python.withPackages (ps: [ps.syncserver]) to path instead? Not 100% sure if it would work though.
There was a problem hiding this comment.
Oh I didn't know about withPackages. How would I use it ? Would ${python.withPackages (ps: [ps.syncserver ps.paster])}/bin/paster work ?
There was a problem hiding this comment.
Actually syncserver has a two versions of wsgiproxy in its dependencies, which causes a conflict with python.withPackages
There was a problem hiding this comment.
Indeed, paster and syncserver are two separate Python programs. They should not share any PYTHONPATH at all. Therefore, exporting PYTHONPATH is tricky.
Could you try FRidh@9687f59
There was a problem hiding this comment.
Well, paster needs to find syncserver somehow. I tested your patch, I get "pkg_resources.DistributionNotFound: The 'syncserver' distribution was not found and is required by the application" from paster
There was a problem hiding this comment.
can't you get rid of ignorecollisions now that you override the dependency ?
There was a problem hiding this comment.
Made a mistake there, because we should not ignore collisions
FRidh@938562f
There was a problem hiding this comment.
hmm, I looked more precisely, and webtest is only a build-time dependency of pyramid... the conflict may be from somewhere else and may be due to a packager mixing build-time and runtime deps
There was a problem hiding this comment.
Indeed, webtest is a testing framework, it should never be propagated. But a number of packages do not make the distinction (e.g. https://github.com/mozilla-services/pyramid_hawkauth/blob/master/setup.py)
There was a problem hiding this comment.
This is the correct fix: #28193
Nadrieril
force-pushed
the
ffsync-fix-pythonpath
branch
from
68fb3fe
to
d6c1d2f
Compare
|
I squashed your commits and pushed. This works on my server. |
Along with #28187 and #28186, this is enough to make the sync server work again (see #25530).
I have been using the service with two devices for a week now, and as far as I can tell it works well.
(nix.useSandbox on NixOS,
or option
build-use-sandboxinnix.confon non-NixOS)
nix-shell -p nox --run "nox-review wip"./result/bin/)