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

Add BashCommand #168

Merged
merged 8 commits into from
Jan 10, 2023
Merged

Add BashCommand #168

merged 8 commits into from
Jan 10, 2023

Conversation

passsy
Copy link
Contributor

@passsy passsy commented Jan 8, 2023

A new command that allows writing a sidekick command as bash script

Usage:

runner
   ..addCommand(
      BashCommand(
        name: 'test-bash-command',
        description: 'Prints inputs of a sidekick BashCommand',
        workingDirectory: runner.repository.root,
        script: () => '''
echo "arguments: \$@"
echo "workingDirectory: \$(pwd)"
# Access paths from sidekick
${systemFlutterSdkPath()}/bin/flutter --version
      ''',
      ),
    );

Prints

❯ sk test-bash-command asdf -s qwer
arguments: asdf -s qwer
working directory: /Users/pascalwelsch/Projects/phntmxyz/sidekick
Flutter 3.3.10 • channel stable • https://github.com/flutter/flutter.git
Framework • revision 135454af32 (3 weeks ago) • 2022-12-15 07:36:55 -0800
Engine • revision 3316dd8728
Tools • Dart 2.18.6 • DevTools 2.15.0

@passsy passsy added sidekick_core https://pub.dev/packages/sidekick_core prebuild-commands All pre-build commands that ship with sidekick_core labels Jan 8, 2023
@passsy passsy requested a review from Giuspepe January 8, 2023 13:52
@Giuspepe
Copy link
Contributor

Giuspepe commented Jan 9, 2023

When I run the following command

BashCommand(
  script: () => '''
echo hello
echo "bye" >&2
exit 1
''',
  description: 'test bash error',
  name: 'test-bash',
);

I get the following output

❯ ./sk test-bash
hello
bye

Error executing script:

echo hello
echo "bye" >&2
exit 1

The captured error of running the script is:


Unhandled exception:
/var/folders/c0/vw17q1tx6k50xkmzl4wx1pbh0000gn/T/Q2wmhk/6691b1f98e96063ec2ab219e59217725.sh  
exit: 1
reason: The command [/var/folders/c0/vw17q1tx6k50xkmzl4wx1pbh0000gn/T/Q2wmhk/6691b1f98e96063ec2ab219e59217725.sh] with args [] failed with exitCode: 1 workingDirectory: /Users/pepe/repos/sidekick
<stack trace>

I would have expected that The captured error of running the script is:\n bye is printed, but the line which should contain the error is completely empty in the output

Also The command [/var/folders/c0/vw17q1tx6k50xkmzl4wx1pbh0000gn/T/Q2wmhk/6691b1f98e96063ec2ab219e59217725.sh] with args [] failed could be a confusing error message because the script has already been deleted once the exception message is printed. I think it'd be better to either throw a custom exception message with the preserved stack trace or to only delete the script if execution has been successful.

@passsy
Copy link
Contributor Author

passsy commented Jan 9, 2023

The error is empty because the BashCommand runs in terminal mode (withStdIn = true) by default. Stdin/out are wired directly to the sub-process and dart has no way to consume the streams.
It's indeed confusing, I'll change that.

@passsy
Copy link
Contributor Author

passsy commented Jan 10, 2023

Further improved the error message. Instead of directly writing it to stderr, it now throws a normal exception to be handled by the command runner.

Before:

arguments: 
workingDirectory: /Users/pascalwelsch/Projects/phntmxyz/sidekick
/var/folders/0j/p0s0zrv91tgd33zrxb88c0440000gn/T/bSyNiz/9e8a2d996b8a821db8b416451599d06a.sh: line 3: error: command not found

Error executing script 'test-bash-command' (exitCode=127):
echo "arguments: $@"
echo "workingDirectory: $(pwd)"
error 1


Error executing script 'test-bash-command' (exitCode=127)
Unhandled exception:
/var/folders/0j/p0s0zrv91tgd33zrxb88c0440000gn/T/bSyNiz/9e8a2d996b8a821db8b416451599d06a.sh  
exit: 127
reason: The command [/var/folders/0j/p0s0zrv91tgd33zrxb88c0440000gn/T/bSyNiz/9e8a2d996b8a821db8b416451599d06a.sh] with args [] failed with exitCode: 127 workingDirectory: /Users/pascalwelsch/Projects/phntmxyz/sidekick
#0      waitForEx (package:dcli/src/util/wait_for_ex.dart:67)
#1      RunnableProcess.processUntilExit (package:dcli/src/util/runnable_process.dart:460)
#2      RunnableProcess.run (package:dcli/src/util/runnable_process.dart:180)
#3      startFromArgs (package:dcli/src/functions/run.dart:174)
#4      BashCommand.run (package:sidekick_core/src/commands/bash_command.dart:83)
<asynchronous suspension>
#5      CommandRunner.runCommand (package:args/command_runner.dart:209)
<asynchronous suspension>
#6      SidekickCommandRunner.run (package:sidekick_core/sidekick_core.dart:155)
<asynchronous suspension>
#7      runSk (package:sk_sidekick/sk_sidekick.dart:48)
<asynchronous suspension>
#8      main (file:///Users/pascalwelsch/Projects/phntmxyz/sidekick/sk_sidekick/bin/main.dart:4)
<asynchronous suspension>

After:

arguments: 
workingDirectory: /Users/pascalwelsch/Projects/phntmxyz/sidekick
Unhandled exception:
Error (exitCode=0) executing script of command 'test-bash-command' with arguments: <no arguments>

'''bash
echo "arguments: $@"
echo "workingDirectory: $(pwd)"

exit 1
'''


#0      BashCommand.run (package:sidekick_core/src/commands/bash_command.dart:92)
<asynchronous suspension>
#1      CommandRunner.runCommand (package:args/command_runner.dart:209)
<asynchronous suspension>
#2      SidekickCommandRunner.run (package:sidekick_core/sidekick_core.dart:155)
<asynchronous suspension>
#3      runSk (package:sk_sidekick/sk_sidekick.dart:49)
<asynchronous suspension>
#4      main (file:///Users/pascalwelsch/Projects/phntmxyz/sidekick/sk_sidekick/bin/main.dart:4)
<asynchronous suspension>

@Giuspepe Giuspepe merged commit 914bde5 into main Jan 10, 2023
@Giuspepe Giuspepe deleted the bash-command branch January 10, 2023 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
prebuild-commands All pre-build commands that ship with sidekick_core sidekick_core https://pub.dev/packages/sidekick_core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants