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

ycmd: correct gopls package and link path #352222

Merged
merged 4 commits into from
Jan 24, 2025

Conversation

melnary
Copy link
Member

@melnary melnary commented Oct 29, 2024

This fixes built-in Go completion support in vimPlugins.YouCompleteMe.

This essentially fixes two problems:

These two problems both contributed to gopls ending up a dangling symlink and breaking Go completion in YCM. After this it has been working correctly once again! :3

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 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
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@NixOSInfra NixOSInfra added the 12. first-time contribution This PR is the author's first one; please be gentle! label Oct 29, 2024
@melnary
Copy link
Member Author

melnary commented Oct 29, 2024

Also, I did kinda notice that the YouCompleteMe plugin and its respective server generally haven't really been well maintained in quite a bit. From what it looks like, this issue in particular has been present for almost 4 years. ^^;
I can add myself as a maintainer and test and update the packages further, if there's interest. I haven't yet checked if the other built-in completion engines work yet, at least for now. ^_^ (Of course only with the other maintainers' approval, I don't want to step on anyone's toes haha)

@siriobalmelli
Copy link
Contributor

I've since moved away from ycmd; in all fairness I'm no longer a maintainer here and would approve a commit removing me from the list if you deem it necessary.

@siriobalmelli
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 352222


aarch64-darwin

✅ 2 packages built:
  • vimPlugins.YouCompleteMe
  • ycmd

@melnary
Copy link
Member Author

melnary commented Oct 30, 2024

I've since moved away from ycmd; in all fairness I'm no longer a maintainer here and would approve a commit removing me from the list if you deem it necessary.

@siriobalmelli If you'd like I can add a commit to this PR removing you from the maintainer role. That's up to you though!

Do you think it would be appropriate to add myself as package maintainer? I would like to see the plugin getting some love again! (With approval from the others, of course.)
Sorry, I'm all very new to this! :)

@wegank wegank added 12.approvals: 1 This PR was reviewed and approved by one reputable person 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package labels Oct 31, 2024
@siriobalmelli
Copy link
Contributor

I'm in favor of removing myself as maintainer and adding you instead, thank you

@github-actions github-actions bot added 6.topic: vim 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` labels Oct 31, 2024
@nix-owners nix-owners bot requested a review from figsoda October 31, 2024 19:20
@wegank wegank removed 12.approvals: 1 This PR was reviewed and approved by one reputable person 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package labels Oct 31, 2024
@ofborg ofborg bot requested review from jagajaga and MarcWeber November 1, 2024 01:55
@ofborg ofborg bot added the 11.by: package-maintainer This PR was created by the maintainer of the package it changes label Nov 1, 2024
@wegank wegank added the 12.approvals: 2 This PR was reviewed and approved by two reputable people label Dec 2, 2024
@FliegendeWurst FliegendeWurst added the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 21, 2024
@SuperSandro2000
Copy link
Member

Please rebase

This fixes built-in Go completion support in vimPlugins.YouCompleteMe.

Previously both an outdated package (gotools) and an outdated binary
path were used when installing gopls into ycmd, silently breaking after an
upstream update.

Signed-off-by: Mel <[email protected]>
@melnary melnary force-pushed the ycmd/fix-gopls-path branch from dc020a1 to 11de008 Compare January 24, 2025 04:08
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 24, 2025
@melnary
Copy link
Member Author

melnary commented Jan 24, 2025

@SuperSandro2000 Rebased + Updated my contact information while I had the chance :3
Sorry for taking so long it, vacation and lots of employment bureaucracy take up a surprising amount of time!

mkdir -p $TARGET
ln -sf ${gotools}/bin/gopls $TARGET
ln -sf ${gopls}/bin/gopls $TARGET
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ln -sf ${gopls}/bin/gopls $TARGET
ln -sf ${lib.getExe gopls} $TARGET

@GaetanLepage
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 352222


x86_64-linux

✅ 3 packages built:
  • vimPlugins.YouCompleteMe
  • vimPluginsUpdater
  • ycmd

aarch64-linux

✅ 3 packages built:
  • vimPlugins.YouCompleteMe
  • vimPluginsUpdater
  • ycmd

x86_64-darwin

✅ 3 packages built:
  • vimPlugins.YouCompleteMe
  • vimPluginsUpdater
  • ycmd

aarch64-darwin

✅ 3 packages built:
  • vimPlugins.YouCompleteMe
  • vimPluginsUpdater
  • ycmd

@SuperSandro2000 SuperSandro2000 merged commit 3bbb308 into NixOS:master Jan 24, 2025
25 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: vim 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes 12.approvals: 2 This PR was reviewed and approved by two reputable people 12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants