Match group substituting

BeyondEvil commented 1 year ago

Describe the bug

Match group substituting in output runner fails under some conditions (see repro instructions).

Reproduction instructions

Steps to reproduce the behavior:

Check out this branch https://github.com/BeyondEvil/bazel-watcher/tree/beyondevil/regex-group-bug
ibazel test //internal/ibazel/output_runner:output_runner_test
See error

Expected behavior

I'm expecting the $1 to be replaced with output_runner producing the output:

*snip*
INFO: Build completed successfully, 1 total action
Do you want to execute this command?
echo target=output_runner
[y/N]y
iBazel [1:36AM]: Executing command: echo
iBazel [1:36AM]: Workspace path: /Users/jimbrannlund/dev/bazel-watcher
iBazel [1:36AM]: Executing command: `echo target=$1`
target=output_runner

but it's not performing the substitution:

*snip*
INFO: Build completed successfully, 1 total action
Do you want to execute this command?
echo target=$1
[y/N]y
iBazel [1:36AM]: Executing command: echo
iBazel [1:36AM]: Workspace path: /Users/jimbrannlund/dev/bazel-watcher
iBazel [1:36AM]: Executing command: `echo target=$1`
target=$1

[
    {
        "regex": "^Target \\/\\/internal\\/ibazel\\/(.+):output_runner_test up-to-date:$",
        "command": "echo",
        "args":    ["target=$1"]
    }
]

It also fails for:

"args": ["target/$1"]
"args": ["target $1"]

Version (please complete the following information):

OS: M1 Mac OSX 12.3 (Monterey)
iBazel - Version v0.17.0
Bazelisk version: development Build label: 5.3.1 Build target: bazel-out/darwin_arm64-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar Build time: Mon Sep 19 17:37:11 2022 (1663609031) Build timestamp: 1663609031 Build timestamp as int: 1663609031

Additional context Side not: -run_output_interactive=false doesn't work.

❯ ibazel test -run_output_interactive=false //internal/ibazel/output_runner:output_runner_test
iBazel [1:44AM]: You are using an experimental filesystem watcher. If you would like to disable that, please set the environment variable
        IBAZEL_USE_LEGACY_WATCHER=1
iBazel [1:44AM]: Querying for files to watch...
Loading: 0 packages loaded
ERROR: Error while parsing 'buildfiles(deps(set(-run_output_interactive=false //internal/ibazel/output_runner:output_runner_test)))': syntax error at '- run_output_interactive ='
Loading: 0 packages loaded
Loading: 0 packages loaded
iBazel [1:44AM]: Bazel query failed: exit status 2
Loading: 0 packages loaded
ERROR: Error while parsing 'kind('source file', deps(set(-run_output_interactive=false //internal/ibazel/output_runner:output_runner_test)))': syntax error at '- run_output_interactive ='
Loading: 0 packages loaded
Loading: 0 packages loaded
iBazel [1:44AM]: Bazel query failed: exit status 2
iBazel [1:44AM]: Testing -run_output_interactive=false //internal/ibazel/output_runner:output_runner_test
ERROR: -run_output_interactive=false :: Invalid options syntax: -run_output_interactive=false
iBazel [1:44AM]: Build error: exit status 2

BeyondEvil commented 1 year ago

Another unexpected behaviour happens if args is:

"args":    ["$1target"]

Do you want to execute this command?
echo Target //internal/ibazel/output_runner:output_runner_test up-to-date:
[y/N]y
iBazel [1:47AM]: Executing command: echo
iBazel [1:47AM]: Workspace path: /Users/jimbrannlund/dev/bazel-watcher
iBazel [1:47AM]: Executing command: `echo Target //internal/ibazel/output_runner:output_runner_test up-to-date:`
Target //internal/ibazel/output_runner:output_runner_test up-to-date:

achew22 commented 1 year ago

@BeyondEvil thanks so much for your bug report. Would you be willing to add a failing testcase to our codebase so we can have a more precise conversation around the issue?

https://github.com/bazelbuild/bazel-watcher/blob/34c48343a7943b695c895a714af3822bb1f72d27/internal/ibazel/output_runner/output_runner_test.go#L26-L65

Thanks!

BeyondEvil commented 1 year ago

I can try (again). I already looked at those test in that file, but I have trouble deciphering what it's doing. 😏 🤷‍♂️

BeyondEvil commented 1 year ago

What's the difference between convertArg and convertArgs and when/how are they used?

BeyondEvil commented 1 year ago

I'm sorry, I'm unsuccessful in creating a failing test. :(

BeyondEvil commented 1 year ago

FYI: I'm running on v0.17.0 which is the latest version available by Homebrew, but the latest release seems to be v0.20.0 🤔

BeyondEvil commented 1 year ago

Ok, no wonder I couldn't create a failing tests for args - the issue is fixed in master (and v0.20.0) 🤦

However, it fails (and I can create a failing test for it) for commands. commit

The -run_output_interactive flag still isn't working tho:

❯ bazel-bin/cmd/ibazel/ibazel_/ibazel test -run_output_interactive=false //internal/ibazel/output_runner:output_runner_test
iBazel [11:36PM]: You are using an experimental filesystem watcher. If you would like to disable that, please set the environment variable
        IBAZEL_USE_LEGACY_WATCHER=1
iBazel [11:36PM]: Querying for files to watch...
iBazel [11:36PM]: Bazel query failed: exit status 2
iBazel [11:36PM]: Error querying for source files: exit status 2
iBazel [11:36PM]: Bazel query failed: exit status 2
iBazel [11:36PM]: Error querying for source files: exit status 2
iBazel [11:36PM]: Testing -run_output_interactive=false //internal/ibazel/output_runner:output_runner_test
ERROR: -run_output_interactive=false :: Invalid options syntax: -run_output_interactive=false
iBazel [11:36PM]: Error running Bazel exit status 2

BeyondEvil commented 1 year ago

So, to summarize.

Latest version via Homebrew is v0.17.0 where this issue is present for both command and args
Issue is fixed for args but not command in v0.20.0
Interactive flag fails for v0.20.0

BeyondEvil commented 1 year ago

It seems NPM installs an even older version:

"@bazel/ibazel": "^0.16.2",

BeyondEvil commented 1 year ago

ping @achew22

achew22 commented 1 year ago

Yep, the binaries that are uploaded to the release pages are up to date. There is some issue with the NPM release process that I've not been able to work out after a solid few hours of trying. I don't use ibazel through the npm ecosystem, it was contributed by someone else, but would generally recommend downloading from these release pages and not from anywhere else.

BeyondEvil commented 1 year ago

Yep, the binaries that are uploaded to the release pages are up to date. There is some issue with the NPM release process that I've not been able to work out after a solid few hours of trying. I don't use ibazel through the npm ecosystem, it was contributed by someone else, but would generally recommend downloading from these release pages and not from anywhere else.

Cool, good to know!

What about the interactive flag-issue I'm seeing? @achew22

achew22 commented 1 year ago

To make the flag work, you would need to add it to the list of flags that we pass through to bazel https://github.com/bazelbuild/bazel-watcher/blob/master/cmd/ibazel/main.go#L37. Seems like you're using one that ibazel doesn't know about.

BeyondEvil commented 1 year ago

Seems like you're using one that ibazel doesn't know about.

Not sure what you mean, but the flag is mentioned in the ibazel docs https://github.com/bazelbuild/bazel-watcher/blob/master/README.md#output-runner

Adding the flag --run_output_interactive=false will automatically run the command without prompting for confirmation.

And here's the code: https://github.com/bazelbuild/bazel-watcher/blob/3a6b20ac429a1ad35e4a2a6c7542cefd39ab4c8b/internal/ibazel/output_runner/output_runner.go#L40

@achew22

github-actions[bot] commented 1 year ago

Stale issue message

github-actions[bot] commented 1 year ago

Stale issue message

bazelbuild / bazel-watcher

Match group substituting #561