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

fix: clipboard package not working with CGO_ENABLED=0 [ROAD-1185] #147

Merged
merged 13 commits into from
Oct 13, 2022

Conversation

j-luong
Copy link
Contributor

@j-luong j-luong commented Oct 4, 2022

Description

This PR replaces the clipboard package due to compatibility issues when building with CGO_ENABLED=0. Before merging we will need to verify that the changes work on all platforms.

Validation procedure

  • Use goreleaser to build LS binaries in golang docker container:
docker run --rm -it -v ${PWD}:/app -w /app golang
go install github.com/goreleaser/goreleaser@latest
goreleaser build --rm-dist --snapshot
  • Wire built binary as custom LS path in VSCode
  • Open extension and try authenticating from scratch
  • Press “Copy authentication link to clipboard”
  • Auth link should be copied to clipboard (paste to verfiy)

Checklist

  • Tests added and all succeed
  • Linted
  • Validated on darwin_arm64
  • Validated on darwin_amd64
  • Validated on linux_386
  • Validated on linux_amd64
  • Validated on linux_arm64 (with xsel installed)
  • Validated on windows_386
  • Validated on windows_amd64
  • License file updated, if new 3rd-party dependency is introduced

@j-luong j-luong marked this pull request as ready for review October 10, 2022 08:38
@j-luong j-luong requested a review from a team as a code owner October 10, 2022 08:38
@j-luong j-luong force-pushed the fix/clipboardNotWorking branch from bc911db to 0dbce8b Compare October 10, 2022 12:31
@j-luong j-luong force-pushed the fix/clipboardNotWorking branch 2 times, most recently from c0d4475 to dedf640 Compare October 10, 2022 12:49
@j-luong
Copy link
Contributor Author

j-luong commented Oct 10, 2022

There's a package requirement to have xsel or xclip for Linux, if these are missing, testing on linux_arm64, copyAuthLink fails with: Snyk encountered an error: No clipboard utilities available. Please install xsel, xclip, wl-clipboard or Termux:API add-on for termux-clipboard-get/set.

Screenshot from 2022-10-10 16-20-07

@bastiandoetsch
Copy link
Collaborator

Would you please the documentation to

  • readme
  • gitbook

Also, please create a followup story to

  • display the needed dependency on vscode linux
  • add to vscode documentation

@j-luong j-luong force-pushed the fix/clipboardNotWorking branch from 91592f5 to ee57b50 Compare October 11, 2022 12:42
@j-luong
Copy link
Contributor Author

j-luong commented Oct 11, 2022

@bastiandoetsch as we're going down the 2 packages option, would the doc updates still be required? Linux shouldn't have a dependency requirement with golang.design/x/clipboard.

There's actually a dependency on CGO_ENABLED=1 for Linux in the golang.design/x/clipboard package. This means that we can't use 2 different packages.

Will revert back to having a requirement for Linux and update the docs as suggested

@j-luong
Copy link
Contributor Author

j-luong commented Oct 12, 2022

Docs (README, GitBook/VSCode) have been updated.

Copy link
Contributor

@michelkaporin michelkaporin left a comment

Choose a reason for hiding this comment

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

🚀

README.md Outdated Show resolved Hide resolved
@j-luong j-luong force-pushed the fix/clipboardNotWorking branch from cb7c9f7 to 617dbcd Compare October 13, 2022 11:14
@j-luong j-luong merged commit 98daf2c into main Oct 13, 2022
@j-luong j-luong deleted the fix/clipboardNotWorking branch October 13, 2022 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants