python-language-server: remove

[JamieMagee]

Description of changes

Depends on #209582 and #209585

Related to #202572

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 23.05 Release Notes (or backporting 22.11 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
  • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
• Fits CONTRIBUTING.md.

[mdarocha]

You should probably add a change log entry about removing a package, same in the other 2 PRs

[ajs124]

it's just an aliases entry, not really the changelog/release notes @mdarocha. but yes, please add that.

although you can obviously wait until the other two are merged.

[ajs124]

IMO #209582 and #209585 could just have been included in this PR, so you could save yourself all this rebasing.

As it is now, we can just go ahead with #209582 and then this PR or you can cherry-pick the change in #209582 into this branch and close #209582.

[ajs124]

diff LGTM

[thomasjm]

Hang on, this looks confused.

On the one hand the PR description mentions a couple packages related to the old Palantir language server, while the package you're removing here is the Microsoft one.

The addition to aliases.nix appears to represent a similar confusion.

The Microsoft language server is still maintained and used, so please don't remove it. If it needs to be updated to use a later dotnet version we can deal with that in connection to #202572.

[JamieMagee]

@thomasjm let me try and address your concerns.

First, the confusion between the Palantir python-language-server and the Microsoft python-language-server. pyls-black and pyls-mypy both depend on the package called python-language-server¹². On PyPI python-language-server links to the palantir python-language-server. nixpkgs has never packaged the Palantir version of python-language-server and the Microsoft python-language-server was never published to PyPI. I assume the Microsoft version is interchangeable due to the language server protocol standard allowing for different implementations.

Second, I don't think the Microsoft python-language-server is maintained. The repository was archived in April 2022.

Are you thinking of Pylance? It is the successor to the Microsoft python-language-server. However, it's not open source. That's why the community created python-lsp-server

Footnotes

1. https://github.com/rupert/pyls-black/blob/0f2e9ed8c0fb20f7f88fe47e11b723e311b568d6/setup.cfg#L17 ↩

2. https://github.com/tomv564/pyls-mypy/blob/3b105b491a42eec304f19e6ed6da1053d12958c6/requirements.txt#L4 ↩

[thomasjm]

nixpkgs has never packaged the Palantir version of python-language-server

Incorrect: https://github.com/NixOS/nixpkgs/blob/release-20.03/pkgs/development/python-modules/python-language-server/default.nix

the Microsoft python-language-server was never published to PyPI

Naturally not, since it's not written in Python :P

I assume the Microsoft version is interchangeable due to the language server protocol standard allowing for different implementations

"Interchangeable" would be a very strong word when you consider the different behaviors and feature sets of LSP implementations.

Are you thinking of Pylance? It is the successor to the Microsoft python-language-server. However, it's not open source. That's why the community created python-lsp-server

Incorrect, python-lsp-server is a fork of the Palantir one, written in Python, and not related to the Microsoft one. (See for example python-lsp/python-lsp-server#4)

You are right that Microsoft's Python Language Server has reached EOL. But I'd encourage you to completely separate any PR affecting it from any of the Palantir stuff. I also believe that it still has users in Nixpkgs, and remains a perfectly good language server, so I wouldn't be in a rush to remove it.

[JamieMagee]

Clearly I need to improve my git searching 😅

You are right that Microsoft's Python Language Server has reached EOL. But I'd encourage you to completely separate any PR affecting it from any of the Palantir stuff.

This PR only affects the Microsoft language server. I only mentioned the Palantir language server in #209582 and #209585 because I wondered why they had a dependency on the wrong language server implementation.

I also believe that it still has users in Nixpkgs, and remains a perfectly good language server, so I wouldn't be in a rush to remove it.

It's an unmaintained package built with an unmaintained SDK and running on an unmaintained runtime. I think that is a pretty good reason to remove it.

There is a maintained alternative, python-lsp-server, that users can migrate to.

[thomasjm]

I don't disagree about the unmaintainedness, and if we're trying to get rid of the old dotnet in Nixpkgs then sounds fine to me.

But it's worth pointing out that the two are not interchangeable, and the Microsoft one presents a superior experience in some ways. You should definitely remove the alias from this PR, not least because there are implementation-specific configuration options you can pass and it would be very confusing to any users who are doing that if the backend implementation was completely swapped out.

[ajs124]

You should definitely remove the alias from this PR, not least because there are implementation-specific configuration options you can pass and it would be very confusing to any users who are doing that if the backend implementation was completely swapped out.

The alias in this PR is a throw, so it alerts the user that manual intervention is needed and suggests them that they can switch to some similar software. If it were an alias that just pointed the removed package to the other, I'd agree, but in this case, this is the way we solve this. Dropping the alias just gives a less helpful evaluation error.

However, we can obviously rephrase the text of the throw, if you disagree with it.

Personally, I'm just here because of #210452, which is related, because buildDotNet currently uses openssl_1_1, because it still supports .NET 3.1. At least that's my understanding of the situation.

[GGG-KILLER]

I also don't see any issue with the message in the throw. In my experience using deprecated packages, it was never a drop in.

As a user, you should always evaluate the new package to see if it fits your needs and if it is a drop in replacement or not.

[thomasjm]

How about changing from

throw "python-language-server is no longer maintained, use the python-lsp-server community fork instead."

to

throw "python-language-server reached end-of-life and was removed from Nixpkgs. Consider using python-lsp-server instead."

(python-lsp-server is not a community fork of the python-language-server in question)

[GGG-KILLER]

How about changing from

throw "python-language-server is no longer maintained, use the python-lsp-server community fork instead."

to

throw "python-language-server reached end-of-life and was removed from Nixpkgs. Consider using python-lsp-server instead."

(python-lsp-server is not a community fork of the python-language-server in question)

I feel like that's a bit too long, I'd use the alias 2 lines below as a reference and do something like this instead:
python-language-server has been removed as it is no longer maintained. Use e.g. python-lsp-server instead

[JamieMagee]

I've rebased and updated the alias message.