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.

Sign up for GitHub

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

Jump to bottom

autotools-language-server: init at 0.0.19 #316901

Merged
merged 10 commits into from
Jun 18, 2024
Merged

autotools-language-server: init at 0.0.19 #316901

merged 10 commits into from
Jun 18, 2024

Conversation

doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Jun 3, 2024 •
edited
Loading

Motivation for this change

Numerous other Python updates, and this new package.

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:
  • 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/)
  • 22.05 Release Notes (or backporting 21.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.

@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Jun 3, 2024
@ofborg ofborg bot requested a review from fabaff June 3, 2024 13:18
@doronbehar doronbehar force-pushed the pkg/autotools-language-server branch from 03ddbf0 to 2d5ed1c Compare June 13, 2024 06:49
@doronbehar
Copy link
Contributor Author

Thanks for the review!

@doronbehar doronbehar force-pushed the pkg/autotools-language-server branch from 2d5ed1c to 710f407 Compare June 13, 2024 07:00
@doronbehar doronbehar force-pushed the pkg/autotools-language-server branch from 710f407 to 306018c Compare June 14, 2024 07:50
@doronbehar doronbehar merged commit 7eabbc2 into NixOS:master Jun 18, 2024
24 checks passed
@doronbehar doronbehar deleted the pkg/autotools-language-server branch June 18, 2024 06:52
Janik-Haag
Janik-Haag reviewed Jun 26, 2024
View reviewed changes
pkgs/top-level/python-packages.nix
@@ -15402,6 +15404,20 @@ self: super: with self; {

tree-sitter = callPackage ../development/python-modules/tree-sitter { };

tree-sitter0_21 = callPackage ../development/python-modules/tree-sitter0_21 { };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is a bit late, but why are you copying and pasting the file instead of using overrideAttrs and just modifying the original package, that would make this a lot more maintainable and would ensure that the two versions nix expressions don't diverge from each other?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is a bit late, but why are you copying and pasting the file instead of using overrideAttrs and just modifying the original package, that would make this a lot more maintainable and would ensure that the two versions nix expressions don't diverge from each other?

The argument to make the expressions different is exactly related to the potential divergence: I anticipate that in the future the two expressions will keep on diverging and diverging, especially since the developers of tree-sitter don't officially support this outdated 0.21 version.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you should at least make yourself the maintainer rather then fabaff. I also don't know how to feel about keeping the older version alive, I guess it's fine as long as their is still upstream support, but once upstream drops support we kinda wann drop it as well I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you should at least make yourself the maintainer rather then fabaff. I also don't know how to feel about keeping the older version alive, I guess it's fine as long as their is still upstream support, but once upstream drops support we kinda wann drop it as well I think.

You are correct, this is not ideal, but this is related to the choice of python3.pkgs.tree-sitter-languages v.s python3.pkgs.tree-sitter-make. The later is not packaged in Nixpkgs yet, and for a good reason and the former has this issue with the latest version of python3.pkgs.tree-sitter.

TBH only now I managed to explain it good enough :) The situation perhaps should have been explained in some comments.

@Janik-Haag Janik-Haag mentioned this pull request Jun 26, 2024
Open
13 tasks
@ghpzin ghpzin mentioned this pull request Jul 27, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Janik-Haag Janik-Haag Janik-Haag left review comments

@Moraxyc Moraxyc Moraxyc left review comments

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@fabaff fabaff Awaiting requested review from fabaff

Assignees
No one assigned
Labels
6.topic: python 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@doronbehar @SuperSandro2000 @Moraxyc @Janik-Haag