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 error handling for command dotnet csharpier #58

Merged
merged 1 commit into from
Nov 30, 2023

Conversation

arg0d
Copy link
Collaborator

@arg0d arg0d commented Nov 28, 2023

In Rust, command exit status has to be handled manually. Not handling exit status would cause the generator to generate invalid code when dotnet is installed, but csharpier is not installed.

Switch from using dotnet csharpier to using dotnet-csharpier. The former produces really verbose output by dotnet when csharpier is not installed or dotnet tools directory is not in PATH, while the latter returns a neat command not found error if CSharpier is not yet installed.

@arg0d arg0d force-pushed the kristupas/fix-csharpier-not-installed branch from 8de8eb0 to f8b0b98 Compare November 28, 2023 15:17
@arg0d
Copy link
Collaborator Author

arg0d commented Nov 28, 2023

@meenzen could you check if its fine to use dotnet-csharpier instead of dotnet csharpier.

@arg0d arg0d requested a review from Lipt0nas November 28, 2023 15:18
@arg0d
Copy link
Collaborator Author

arg0d commented Nov 28, 2023

It seemed like error handling was adequate at first from CI run, but the errors were only handled when dotnet itself is not present in PATH.

The verbose output I'm talking about:

> dotnet csharpier
Could not execute because the specified command or file was not found.
Possible reasons for this include:
  * You misspelled a built-in dotnet command.
  * You intended to execute a .NET program, but dotnet-csharpier does not exist.
  * You intended to run a global tool, but a dotnet-prefixed executable with this name could not be found on the PATH.

@meenzen
Copy link
Contributor

meenzen commented Nov 28, 2023

Calling the dotnet-csharpier command directly has slightly different behavior, it always calls the global-tool. If you have csharpier installed as a local tool (using a tool manifest) then the dotnet-csharpier command does not work.

Running dotnet csharpier will automatically choose the correct tool for the working directory.

I can't think of a better solution than using dotnet-csharpier in this case though, so this is a limitation we'll have to accept for now.

In Rust, command exit status has to be handled manually. Not handling
exit status would cause the generator to generate invalid code when
`dotnet` is installed, but `csharpier` is not installed.

Switch from using `dotnet csharpier` to using `dotnet-csharpier`. The
former produces really verbose output by `dotnet` when `csharpier` is
not installed or dotnet tools directory is not in PATH, while the latter
returns a neat `command not found` error if CSharpier is not yet
installed.

Signed-off-by: Kristupas Antanavicius <kristupas.antanavicius@nordsec.com>
@arg0d arg0d force-pushed the kristupas/fix-csharpier-not-installed branch from f8b0b98 to 0bc97d3 Compare November 29, 2023 09:23
@arg0d
Copy link
Collaborator Author

arg0d commented Nov 29, 2023

If using dotnet-csharpier works for your current use case, then I'm inclined to merge this.

@meenzen
Copy link
Contributor

meenzen commented Nov 29, 2023

I'm fine with that.

Obligatory LGTM

@arg0d arg0d merged commit 7d49e0e into main Nov 30, 2023
4 checks passed
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