Skip to content
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

Merge pull request #77608 from phi-gamma/olm-remove-typing #77608

Merged
merged 1 commit into from Jan 13, 2020

Conversation

@phi-gamma
Copy link
Contributor

@phi-gamma phi-gamma commented Jan 13, 2020

Typing has been upstreamed into CPython and will cause errors
if loaded with a 3.7 interpreter. Cf.
python/typing#573

Olm works without typing as tested via weechat-matrix.

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions (Fedora 31)
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
@phi-gamma phi-gamma requested review from FRidh and jonringer as code owners Jan 13, 2020
@Mic92
Copy link
Contributor

@Mic92 Mic92 commented Jan 13, 2020

@GrahamcOfBorg build python3Packages.python-olm python2Packages.python-olm

@phi-gamma
Copy link
Contributor Author

@phi-gamma phi-gamma commented Jan 13, 2020

@Mic92: I had no idea Python2 was still a thing on nixos. That one I didn’t
test at all. weechat builds against Python3 so it’s fine, dunno about other
downstream olm users though.

@Mic92
Copy link
Contributor

@Mic92 Mic92 commented Jan 13, 2020

I would say either mark disable the python2 build disabled = !isPy3k; or include typing conditionally for python2, if python2 is still supported by this library and including typing make the build fixed.

Typing has been upstreamed into CPython and will cause errors
if loaded with a 3.7 interpreter. Cf.
python/typing#573

Under Python3 libolm works without typing as tested via
weechat-matrix.

typing is still pulled in when installed for Python2.
@phi-gamma phi-gamma force-pushed the phi-gamma:olm-remove-typing branch from ca39b30 to 0169b72 Jan 13, 2020
@phi-gamma
Copy link
Contributor Author

@phi-gamma phi-gamma commented Jan 13, 2020

Skimming the other users of libolm I couldn’t figure out whether
they still supported py2 so I made typing depend on py2 as
per your suggestion.

@Mic92
Copy link
Contributor

@Mic92 Mic92 commented Jan 13, 2020

@GrahamcOfBorg build python3Packages.python-olm python2Packages.python-olm

@Mic92 Mic92 changed the title python3Packages.python-olm: remove dependency on the typing package Merge pull request #77608 from phi-gamma/olm-remove-typing Jan 13, 2020
@Mic92 Mic92 merged commit 0129b95 into NixOS:master Jan 13, 2020
3 checks passed
3 checks passed
python2Packages.python-olm, python3Packages.python-olm on aarch64-linux Success
Details
python2Packages.python-olm, python3Packages.python-olm on x86_64-darwin Success
Details
python2Packages.python-olm, python3Packages.python-olm on x86_64-linux Success
Details
@phi-gamma
Copy link
Contributor Author

@phi-gamma phi-gamma commented Jan 13, 2020

Thanks, that was really fast!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.