TezosTaqueria / taqueria

*BETA* Taqueria provides a seamless development workflow to easily build, test and deploy your Tezos applications.
https://taqueria.io
Apache License 2.0
64 stars 20 forks source link

[Bug]: LIGO plugin - incorrectly interpreting docker output #254

Closed mweichert closed 2 years ago

mweichert commented 2 years ago

What happened?

The LIGO plugin executes the LIGO CLI via docker. However, the LIGO plugin does not differentiate the stdout/stderr generated from the use of the docker command versus the stdout/stderr generated by the LIGO CLI.

Currently, if the LIGO CLI writes to stdout but not stderr, but docker writes to stderr, we'll throw an error an assume that compilation has failed. That's because both processes are sharing the same stderr. By segmenting these, we can handle different outputs appropriately.

This is an issue currently when running the ligolang:ligo image on M1 processors. As the LIGO team hasn't build an image for the arm64/v8 architecture, docker emits a WARNING on stderr. Because we treat all stderr output as a fatal error, we're incorrectly stating that there was a compilation error.

What should have happened?

We need to segment the stdout and stderr for docker and the LIGO cli. We should likewise do the same for process return codes.

This should probably be generalized and provided by the SDK, not something that the LIGO plugin is responsible for. Rather, the LIGO plugin should just be responsible for handling the errors/exceptions thrown by the SDK when used to invoke docker.

Steps to reproduce the bug

On a mac with the M1 processor, try compiling a valid LIGO contract using Taqueria 0.0.8. You'll get a message that compilation has failed, and yet, the artifact was generated successfully.

Relevant log output

No response

Screenshots and images

Screenshot and images

Code of Conduct

mweichert commented 2 years ago

This might be best to implement once #257, #258, and #255 are complete.

russellmorton commented 2 years ago

@mweichert to link issue and close this issue as a duplicate.

russellmorton commented 2 years ago

Resolved. Confirmed with @mweichert