Skip to content

Conversation

@leigaol
Copy link
Contributor

@leigaol leigaol commented Apr 30, 2025

Problem

LSP depends on node 18 which depends on GLIBC >=2.28.
Amzn al2 instances do not have GLIBC >=2.28.

Solution

Use the patch of glibc if it is available.


  • Treat all work as PUBLIC. Private feature/x branches will not be squash-merged at release time.
  • Your code changes must meet the guidelines in CONTRIBUTING.md.
  • License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

@leigaol leigaol requested a review from a team as a code owner April 30, 2025 21:33
@leigaol leigaol marked this pull request as draft April 30, 2025 21:33
@github-actions
Copy link

  • This pull request modifies code in src/* but no tests were added/updated.
    • Confirm whether tests should be added or ensure the PR description explains why tests are not required.
  • This pull request implements a feat or fix, so it must include a changelog entry (unless the fix is for an unreleased feature). Review the changelog guidelines.
    • Note: beta or "experiment" features that have active users should announce fixes in the changelog.
    • If this is not a feature or fix, use an appropriate type from the title guidelines. For example, telemetry-only changes should use the telemetry type.

@Hweinstock
Copy link
Contributor

verified the build works for me in cloud desktop as well!

@leigaol leigaol marked this pull request as ready for review April 30, 2025 21:56
@Hweinstock
Copy link
Contributor

Hweinstock commented Apr 30, 2025

I've verified with the latest commit, I can still execute tools and use agentic chat in my dev desktop!

Also tested this in non-dev desktop and language server starts up normally.

@jpinkney-aws
Copy link
Contributor

Mine worked on my old dev desktop but on the new one it fails because /opt/vsc-sysroot/lib64/ld-linux-x86-64.so.2 isn't defined

@justinmk3
Copy link
Contributor

here are the 2 places that decide the path to node+args:

const serverModule = resourcePaths.lsp
const argv = [

const serverModule = resourcePaths.lsp
const serverOptions = createServerOptions({
encryptionKey: key,
executable: resourcePaths.node,
serverModule,
// TODO(jmkeyes): we always use the debug options...?
execArgv: debugOptions.execArgv,
})

each of those then passes nodePath (+args) to various functions.

Copy link
Contributor

@opieter-aws opieter-aws left a comment

Choose a reason for hiding this comment

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

The linter is failing on the fs import

@jpinkney-aws jpinkney-aws deleted the branch aws:master May 1, 2025 03:02
@justinmk3 justinmk3 reopened this May 1, 2025
@justinmk3 justinmk3 changed the base branch from feature/hybridChat to master May 1, 2025 13:37
@leigaol leigaol requested a review from a team as a code owner May 1, 2025 17:55
@opieter-aws opieter-aws merged commit 91f584f into aws:master May 1, 2025
28 of 31 checks passed
const logger = getLogger('amazonqLsp.lspClient')

export async function hasGlibcPatch(): Promise<boolean> {
return await fs.exists('/opt/vsc-sysroot/lib64/ld-linux-x86-64.so.2')
Copy link
Contributor Author

@leigaol leigaol May 1, 2025

Choose a reason for hiding this comment

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

follow up: Check if aarch64 has this issue or not.

Copy link

Choose a reason for hiding this comment

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

It looks like you've only configured this for x86_64. If you want this to work on aarch64, then you'll need to point to ld-linux-aarch64.so.1.

Please check out my overall comment.

Copy link

@murkvin murkvin left a comment

Choose a reason for hiding this comment

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

It seems that this PR takes a dependency on some resources that we drop onto hosts as a component of another project. This is fine, but we should connect to discuss and formalize the interface.

For example, we already vend a static, architecture-independent path to the linker as part of our public interface. Using this would make your code more robust and abstract away platform concerns.

I don't expect to move these files around on people's machines, but I'd rather that doing so didn't break this plugin on AL2.

const logger = getLogger('amazonqLsp.lspClient')

export async function hasGlibcPatch(): Promise<boolean> {
return await fs.exists('/opt/vsc-sysroot/lib64/ld-linux-x86-64.so.2')
Copy link

Choose a reason for hiding this comment

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

It looks like you've only configured this for x86_64. If you want this to work on aarch64, then you'll need to point to ld-linux-aarch64.so.1.

Please check out my overall comment.

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.

7 participants