Skip to content

Handle quotes in gradle arguments #128548

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

idegtiarenko
Copy link
Contributor

When executing .ci/scripts/run-gradle.sh -p benchmarks/ run --args 'org.elasticsearch.benchmark._nightly -rf json -rff build/result.json' I was getting following error:

FAILURE: Build failed with an exception.
* What went wrong:
Problem configuring task :benchmarks:run from command line.
> Unknown command-line option '-r'.

...

https://gradle-enterprise.elastic.co/s/37zyldvhgnbze

I believe this is happening due to quotes in the command. unlike $@, "$@" should preserve quotes.

@idegtiarenko idegtiarenko requested a review from breskeby May 28, 2025 07:53
@idegtiarenko idegtiarenko requested a review from a team as a code owner May 28, 2025 07:53
@idegtiarenko idegtiarenko added >non-issue :Delivery/Build Build or test infrastructure Team:Delivery Meta label for Delivery team v9.1.0 labels May 28, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-delivery (Team:Delivery)

@@ -41,4 +41,4 @@ else
fi

set -e
$GRADLEW -S --max-workers=$MAX_WORKERS $@
$GRADLEW -S --max-workers=$MAX_WORKERS "$@"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please let me know if additional testing (besides main pipeline) is required for this change

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable to me. Have you tested running the pipeline that caused this issue to verify it fixes your problem?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. have you manually triggered the pipeline that failed from the patch branch to verify the fix works here too?

@@ -41,4 +41,4 @@ else
fi

set -e
$GRADLEW -S --max-workers=$MAX_WORKERS $@
$GRADLEW -S --max-workers=$MAX_WORKERS "$@"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. have you manually triggered the pipeline that failed from the patch branch to verify the fix works here too?

@@ -1,7 +1,7 @@
steps:
- label: periodic-micro-benchmarks
command: |
.ci/scripts/run-gradle.sh -p benchmarks/ run --args 'org.elasticsearch.benchmark._nightly -rf json -rff build/result.json'
.ci/scripts/run-gradle.sh :benchmarks:run --args 'org.elasticsearch.benchmark._nightly -rf json -rff build/result.json'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@breskeby
Copy link
Contributor

I've asked @brianseeders to have a look at this too as his bash foo is beyond mine and he authored that initially i think

Copy link
Contributor

@brianseeders brianseeders left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that should be quoted. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Build Build or test infrastructure >non-issue Team:Delivery Meta label for Delivery team v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants