projectfluent / python-fluent

Python implementation of Project Fluent
https://projectfluent.org/python-fluent/
Other
210 stars 26 forks source link

fluent-runtime requires an outdated version of fluent-syntax #176

Closed alexgibson closed 1 year ago

alexgibson commented 1 year ago

The fluent-runtime library seems to pin fluent-syntax to 0.17: https://github.com/projectfluent/python-fluent/blob/master/fluent.runtime/tox.ini#L11

This prevents us from updating / using some other fluent packages, which require 0.18.

See: https://github.com/mozilla/bedrock/issues/12209

eemeli commented 1 year ago

Sorry for the delay in getting to this.

The config you're linking to is for local testing during development, specifically for verifying that the runtime package remains compatible with v0.17 of the syntax package. The published version of the library has a dependency fluent.syntax>=0.17,<0.19.

@alexgibson Are you really sure that this is the source of your problems? Bedrock has itself a pinned dependency on fluent.syntax 0.17.0 with a comment that this is "to avoid compatibility issues", but I can't find mention of what those compatibility issues might be.

alexgibson commented 1 year ago

I believe the conflict is due to two other packages:

  fluent.syntax<0.19,>=0.18.0 (from compare-locales==8.2.1->-r requirements/dev.in (line 25))
  fluent.syntax<0.19,>=0.18.0 (from moz-fluent-linter==0.3.4->-r requirements/dev.in (line 26))
eemeli commented 1 year ago

I'm sorry, I'm not seeing the conflict here. Doesn't 0.18.1 satisfy all of the conditions?

alexgibson commented 1 year ago

Perhaps I’m misunderstanding here, but Fluent runtime strictly requires 0.17, whereas the other libraries require 0.18 at minimum. Hence the conflict?

alexgibson commented 1 year ago

At least this is what our requirements compiler seems to think:

There are incompatible versions in the resolved dependencies:
  fluent-syntax==0.17.0 from https://files.pythonhosted.org/packages/f4/8e/2bb1faa3ec29f52a5609bc1712227cfed8a808671448e2b450c65411135c/fluent.syntax-0.17.0-py2.py3-none-any.whl (from -r requirements/prod.txt (line 255))
  fluent.syntax<0.19,>=0.18.0 (from compare-locales==8.2.1->-r requirements/dev.in (line 25))
  fluent.syntax<0.19,>=0.18.0 (from moz-fluent-linter==0.3.4->-r requirements/dev.in (line 26))
  fluent.syntax<0.19,>=0.17 (from fluent-runtime==0.3.1->-r requirements/prod.txt (line 251))

If you don't think this is the case, I can try again and see if we still hit a conflict.

alexgibson commented 1 year ago

@eemeli Sorry i think i understand and you're right. I need to figure out why 0.17 is pinned in our dependencies. But re-reading the error above, it doesn't seem to be because of the reason I filed the issue for.

Thanks for pointing it out!

stevejalim commented 1 year ago

Just popping in here with some info. I couldn't recall why i had downpinned fluent.syntax to 0.17, so I removed it as an explicit dependency - and then got the kind of conflicts Alex showed, above. So I then upgraded compare-locales from a hard pin of 7.6.0 to 8.2.1. The requirements compiled fine, but then CI blew up for us with this:

+ moz-l10n-lint l10n/l10n-pontoon.toml
Traceback (most recent call last):
  File "/venv/bin/moz-l10n-lint", line 5, in <module>
    from compare_locales.lint.cli import main
  File "/venv/lib/python3.9/site-packages/compare_locales/lint/cli.py", line 10, in <module>
    from compare_locales.lint.linter import L10nLinter
  File "/venv/lib/python3.9/site-packages/compare_locales/lint/linter.py", line 10, in <module>
    from compare_locales import parser, checks
  File "/venv/lib/python3.9/site-packages/compare_locales/parser/__init__.py", line 23, in <module>
    from .fluent import (
  File "/venv/lib/python3.9/site-packages/compare_locales/parser/fluent.py", line 12, in <module>
    from fluent.syntax.visitor import Visitor
ModuleNotFoundError: No module named 'fluent.syntax.visitor'
make: *** [Makefile:127: test-image] Error 1

Exited with code exit status 2

So, I suspect that was the reason behind the 0.17 hard pin.

Any tips for fixing this are v welcome!

eemeli commented 1 year ago

Check your actual local fluent-syntax version? fluent.syntax.visitor was first included in 0.18.0.

stevejalim commented 1 year ago

Hi @eemeli - thanks for the tip.

I've tweaked a few things and I found that if I don't specify fluent-syntax at all, fluent-runtime wants to install 0.17.0 but if i spec fluent-syntax==0.18.1 then our requirements do compile happily enough.

I wonder, is it worth increasing the mininum version of fluent-syntax specced in fluent-runtime up to 0.18? I have a feeling that in our case, pip/pip-compile was trying to get the lowest version acceptable to fluent-runtime

https://github.com/projectfluent/python-fluent/blob/61328fabaa90f5635dd94493110ad5e8cb018282/fluent.runtime/setup.py#L31

(Not sure if that would break other poeple's uses, though)

eemeli commented 1 year ago

Good that you figured it out! I'm clearly still not familiar enough with Python's dependency management.

Once #180 lands, we'll need to bump up both packages to 0.19.0 and require that as the new minimum.

stevejalim commented 1 year ago

Thanks @eemeli - I appreciate the heads-up!