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

Atom client requires vhdl_ls.toml #65

Closed
slaweksiluk opened this issue Jan 17, 2020 · 18 comments · Fixed by #66
Closed

Atom client requires vhdl_ls.toml #65

slaweksiluk opened this issue Jan 17, 2020 · 18 comments · Fixed by #66

Comments

@slaweksiluk
Copy link

I use rust_hdl from docker and after some update language server stopped working in Atom. I was using it without any vhdl_ls.tom for simple syntax checking. Now it seems toml project file is necessary?
Zrzut ekranu z 2020-01-17 13-12-17

@Xcodo
Copy link
Contributor

Xcodo commented Jan 17, 2020

This is because of the changes in #51.

I will see if I can patch the Atom extension.

To get you going you can download a pre-built version of the executable from https://github.com/kraigher/rust_hdl/releases/latest (on linux you may need to chmod the vhdl_ls executable).

@Xcodo
Copy link
Contributor

Xcodo commented Jan 17, 2020

Sorry, now I see that the build definition comes from the Dockerfile itself. I'll fix it here.

@slaweksiluk
Copy link
Author

atom automatically downloaded new docker image with rust hdl, but I still receive similar error (but expected .toml path is different now):
Zrzut ekranu z 2020-01-20 14-46-52

Should I reinstall rust hdl somehow?

@kraigher
Copy link
Member

It seems like the vhdl_ls binary is running from /app/ and not /app/bin because it wants to find /app/../vhdl_libraries/

I assumed this was fixed in the new docker file:
https://github.com/kraigher/rust_hdl/blob/master/Dockerfile

@kraigher
Copy link
Member

@Xcodo it looks like the vhdl_ls binary itself is named /app/bin and not located in /app/bin/ which explains the problem. I will see if I can fix it.

@Xcodo
Copy link
Contributor

Xcodo commented Jan 21, 2020

Thanks. I thought I'd understood the documentation but clearly not.

Before my change the last line didn't have the binary name included

ENTRYPOINT ["/app"]

@kraigher
Copy link
Member

I pushed a fix now which I am reasonably comfortable works. @slaweksiluk can you try it?

@kraigher
Copy link
Member

@Xcodo Before your PR I believe the binary was named app. app was not a folder holding the binary.

@kraigher
Copy link
Member

That fooled me when reviewing because I thought /app/bin/ was the parent folder of the binary but in fact is bin was the binary.

@Xcodo
Copy link
Contributor

Xcodo commented Jan 21, 2020

Ah right, I understand now. Thanks for fixing it.

@slaweksiluk
Copy link
Author

Thank you all for efforts. I'm now receiving different error message:

logger.ts:19 VHDL (vhdl_ls) stderr docker: Error response from daemon: OCI runtime create failed: container_linux.go:346: starting container process caused "exec: \"/app/bin/$CRATE\": stat /app/bin/$CRATE: no such file or directory": unknown.
logger.ts:19 VHDL (vhdl_ls) rpc.onClose The RPC connection closed unexpectedly

Can I debug it somehow at my side?

@kraigher kraigher reopened this Jan 22, 2020
@kraigher
Copy link
Member

Looks like $CRATE variable was not expanded but taken verbatim. I pushed a fix to try to remedy it.

@slaweksiluk
Copy link
Author

Still no luck:

logger.ts:19 VHDL (vhdl_ls) stderr docker: Error response from daemon: OCI runtime create failed: container_linux.go:346: starting container process caused "exec: \"/bin/sh\": stat /bin/sh: no such file or directory": unknown.

@kraigher
Copy link
Member

Strange, @Xcodo @mbrobbel maybe one of you guys have an idea what the problem is? I have "dry-coded" my changes without being able to run the image and see if it works.

@mbrobbel
Copy link
Contributor

@kraigher the problem is that this change c2c6356 modifies the entrypoint to use shell form, which won't work because there is no shell in this image. You can instead choose to hardcode the binary name with exec form.

diff --git a/Dockerfile b/Dockerfile
index 6acdd6e..4e39b95 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -7,6 +7,6 @@ RUN cargo build --manifest-path $CRATE/Cargo.toml --release --features "packaged

 FROM scratch
 ARG CRATE
-COPY --from=builder /volume/target/x86_64-unknown-linux-musl/release/$CRATE /app/bin/$CRATE
+COPY --from=builder /volume/target/x86_64-unknown-linux-musl/release/$CRATE /app/bin/x
 COPY --from=builder /volume/vhdl_libraries /app/vhdl_libraries
-ENTRYPOINT /app/bin/$CRATE
+ENTRYPOINT ["/app/bin/x"]

@kraigher
Copy link
Member

@mbrobbel I will try that, I tried using $CRATE in the entry point but it was not expanded and just used verbatim.

@kraigher
Copy link
Member

I pushed a commit now which hardcodes then binary name to avoid $CRATE not being expanded but still avoids using the shell which is not part of the container.

@slaweksiluk
Copy link
Author

This is working now. Thanks

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 a pull request may close this issue.

4 participants