Move seednode JVM arguments to global configuration#7598
Move seednode JVM arguments to global configuration#7598alvasw wants to merge 5 commits intobisq-network:masterfrom
Conversation
A Gradle upgrade was required to support Bitcoin Coin v29.+, but during the upgrade the mainClassName wasn't correctly migrated to application.mainClass.
The app start scripts were copied from the build directories to the root directory and had their paths updated via search-and-replace by us. However, with recent updates to Gradle, this approach no longer works, so the broken scripts have been removed.
The previous approach of copying app start scripts from build directories to the root and adjusting them no longer works. This commit introduces wrapper scripts that invoke the custom <app>:startBisqApp Gradle task.
The AppStartPlugin currently overrides heap settings for the seednode, and the bisq seednode service unit overrides both heap settings and other JVM arguments. These overrides are not applied in the dev setup, which can lead to inconsistencies and potentially hide bugs during local testing. This change moves the seednode JVM arguments to the global configuration to ensure consistent behavior across environments.
📝 WalkthroughWalkthroughThis pull request refactors the Bisq build system by migrating from legacy Gradle Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
scripts/statsnode (1)
2-5: Quote shell variables to handle paths with spaces.Same issue as in
scripts/bridge- unquoted variables will break if the script path contains spaces or special characters.🔧 Proposed fix
#!/bin/bash -absolute_script_path=$(realpath $0) +absolute_script_path=$(realpath "$0") scripts_dir_path=$(dirname $absolute_script_path) -cd ${scripts_dir_path}/.. +cd "${scripts_dir_path}/.." ./gradlew statsnode:startBisqApp --args "\"$@\""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/statsnode` around lines 2 - 5, The script uses unquoted expansions that will break on paths with spaces; update the assignments and cd to quote variable expansions and command arguments: call realpath with the quoted positional parameter (for example use realpath on "$0"), assign scripts_dir_path using dirname on the quoted absolute_script_path, and use a quoted cd target (scripts_dir_path plus /.. wrapped in quotes) so absolute_script_path, scripts_dir_path, realpath, dirname and the cd invocation handle paths with spaces and special characters.scripts/bridge (1)
2-5: Quote shell variables to handle paths with spaces.Unquoted variables on lines 2 and 5 will break if the script path contains spaces or special characters.
🔧 Proposed fix
#!/bin/bash -absolute_script_path=$(realpath $0) +absolute_script_path=$(realpath "$0") scripts_dir_path=$(dirname $absolute_script_path) -cd ${scripts_dir_path}/.. +cd "${scripts_dir_path}/.." ./gradlew bridge:startBisqApp --args "\"$@\""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/bridge` around lines 2 - 5, The script uses unquoted shell variable expansions for absolute_script_path, scripts_dir_path and when changing directories (realpath, dirname, cd) which will break for paths with spaces; update the assignments and the cd invocation to quote variable expansions and command substitutions (e.g., wrap command substitutions and "$absolute_script_path" / "$scripts_dir_path" in double quotes) so realpath, dirname and cd handle paths with spaces and special characters reliably.scripts/apitest (1)
2-6: Quote variables to handle paths with spaces.Same issue as other wrapper scripts - unquoted variables will cause failures if paths contain spaces.
🔧 Proposed fix
#!/bin/bash -absolute_script_path=$(realpath $0) -scripts_dir_path=$(dirname $absolute_script_path) +absolute_script_path=$(realpath "$0") +scripts_dir_path=$(dirname "$absolute_script_path") -cd ${scripts_dir_path}/.. +cd "${scripts_dir_path}/.." ./gradlew apitest:startBisqApp --args "\"$@\""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/apitest` around lines 2 - 6, The script uses unquoted variable expansions (absolute_script_path, scripts_dir_path, and the ./gradlew invocation) which will break on paths with spaces; update the expansions to use quoted variables (e.g., quote $0 when calling realpath, use "${absolute_script_path}" and "${scripts_dir_path}" in dirname and cd, and quote the args passed to ./gradlew) so all path and argument values are properly handled.scripts/desktop.bat (1)
1-2: Usecall gradleworgradlew.batfor reliable batch script execution.On Windows, invoking another batch script without
callcan cause the current script to not return. Also, the triple-quote pattern is unusual for argument forwarding.🔧 Proposed fix
+@echo off cd %~dp0.. -gradlew desktop:startBisqApp --args """%*""" +call gradlew.bat desktop:startBisqApp --args "%*"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/desktop.bat` around lines 1 - 2, Replace the direct invocation of the Gradle wrapper with a proper batch call and fix argument quoting: use CALL to invoke the wrapper (e.g., call gradlew or call gradlew.bat) instead of running it directly so the script returns control, and change the triple-quote argument forwarding ("""%*""") to a single Windows-safe quoted expansion like "%*" so forwarded args are passed correctly; adjust the existing lines around cd %~dp0.. and the gradlew desktop:startBisqApp --args invocation accordingly.scripts/seednode (1)
2-6: Quote variables to handle paths with spaces.The script will break if the script path or repository path contains spaces or special characters. Variables should be quoted to ensure robustness.
🔧 Proposed fix for quoting
#!/bin/bash -absolute_script_path=$(realpath $0) -scripts_dir_path=$(dirname $absolute_script_path) +absolute_script_path=$(realpath "$0") +scripts_dir_path=$(dirname "$absolute_script_path") -cd ${scripts_dir_path}/.. +cd "${scripts_dir_path}/.." ./gradlew seednode:startBisqApp --args "\"$@\""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/seednode` around lines 2 - 6, Quote all variable expansions and command arguments to handle paths with spaces: assign absolute_script_path with absolute_script_path="$(realpath "$0")", compute scripts_dir_path with scripts_dir_path="$(dirname "$absolute_script_path")", change directory with cd "${scripts_dir_path}/..", and call the Gradle wrapper using "./gradlew" and preserve positional args by passing --args "\"$@\"" (ensure the wrapper path and any variable expansions are quoted). This ensures realpath, dirname, cd, ./gradlew, and "$@" usages are robust to spaces/special characters.scripts/desktop (1)
2-6: Quote variables to handle paths with spaces.Same issue as in
scripts/seednode- unquoted variables will cause failures if paths contain spaces.🔧 Proposed fix
#!/bin/bash -absolute_script_path=$(realpath $0) -scripts_dir_path=$(dirname $absolute_script_path) +absolute_script_path=$(realpath "$0") +scripts_dir_path=$(dirname "$absolute_script_path") -cd ${scripts_dir_path}/.. +cd "${scripts_dir_path}/.." ./gradlew desktop:startBisqApp --args "\"$@\""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/desktop` around lines 2 - 6, Wrap all variable expansions to handle paths with spaces: call realpath with the quoted positional parameter (use absolute_script_path="$(realpath "$0")"), assign scripts_dir_path="$(dirname "$absolute_script_path")", use cd "$scripts_dir_path/.." to change directories safely, and ensure the Gradle invocation quotes the arguments expansion (adjust ./gradlew desktop:startBisqApp --args to pass quoted args, e.g. --args "$@" or an equivalent single-quoted string if Gradle expects one) so that absolute_script_path, scripts_dir_path, and the argument expansion are all properly quoted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@desktop/build.gradle`:
- Around line 20-22: Remove the redundant application block in
desktop/build.gradle (the one containing mainClass =
'bisq.desktop.app.BisqAppMain') that duplicates the earlier application { ... }
definition; keep the original application block (the first occurrence that
already defines mainClass) and delete the second/duplicate block to eliminate
the merge artifact.
In `@scripts/bridge.bat`:
- Around line 1-2: Replace the non-drive-switching directory change and fragile
gradlew invocation in bridge.bat: change the "cd %~dp0.." usage to a drive-aware
command (use cd /d with the "%~dp0.." path), invoke the gradlew batch with
"call" so control returns to the caller, fix the argument quoting to correctly
pass all arguments (use "%*" or proper quoted expansion), and add simple error
handling to bail out with a non-zero exit code if gradlew fails; target the
existing "cd %~dp0.." and "gradlew bridge:startBisqApp --args """%*""""
fragments when making these edits.
In `@scripts/daemon`:
- Around line 2-6: The script breaks argument forwarding and path handling:
quote the invocations and robustly build a single --args value from "$@" using
shell escaping (e.g. build a variable by iterating over "$@" with printf '%q' to
produce a consolidated_args string) then invoke gradlew with
--args="$consolidated_args"; also quote $0 and $absolute_script_path when
calling realpath/dirname (use "realpath \"$0\"" / quote expansions) and make the
cd to ${scripts_dir_path}/.. robust by using cd ... || exit 1 so the script
exits on failure.
In `@scripts/restapi`:
- Around line 2-6: Replace the broken argument forwarding that uses --args
"\"$@\"" with a safe single-string expansion --args="$*" so Gradle receives all
CLI args as one string; also ensure you protect variable expansions (use quoted
expansions for absolute_script_path and scripts_dir_path where used) and make
the directory change robust by using cd ${scripts_dir_path}/.. || exit 1 before
invoking ./gradlew restapi:startBisqApp so the script fails fast on cd errors.
In `@scripts/seednode.bat`:
- Around line 1-2: The launcher uses an unsafe change-directory and fragile arg
forwarding: replace the first line that currently reads "cd %~dp0.." with a cd
that uses the /d flag and quotes the parent-directory expansion (so it works
across drives) and add a quick error check/exit on failure; replace the second
line that currently uses the triple-quote pattern "gradlew seednode:startBisqApp
--args """%*""" with a standard quoted expansion for forwarding arguments (use
--args "%*") to correctly preserve arguments and quotes.
---
Nitpick comments:
In `@scripts/apitest`:
- Around line 2-6: The script uses unquoted variable expansions
(absolute_script_path, scripts_dir_path, and the ./gradlew invocation) which
will break on paths with spaces; update the expansions to use quoted variables
(e.g., quote $0 when calling realpath, use "${absolute_script_path}" and
"${scripts_dir_path}" in dirname and cd, and quote the args passed to ./gradlew)
so all path and argument values are properly handled.
In `@scripts/bridge`:
- Around line 2-5: The script uses unquoted shell variable expansions for
absolute_script_path, scripts_dir_path and when changing directories (realpath,
dirname, cd) which will break for paths with spaces; update the assignments and
the cd invocation to quote variable expansions and command substitutions (e.g.,
wrap command substitutions and "$absolute_script_path" / "$scripts_dir_path" in
double quotes) so realpath, dirname and cd handle paths with spaces and special
characters reliably.
In `@scripts/desktop`:
- Around line 2-6: Wrap all variable expansions to handle paths with spaces:
call realpath with the quoted positional parameter (use
absolute_script_path="$(realpath "$0")"), assign scripts_dir_path="$(dirname
"$absolute_script_path")", use cd "$scripts_dir_path/.." to change directories
safely, and ensure the Gradle invocation quotes the arguments expansion (adjust
./gradlew desktop:startBisqApp --args to pass quoted args, e.g. --args "$@" or
an equivalent single-quoted string if Gradle expects one) so that
absolute_script_path, scripts_dir_path, and the argument expansion are all
properly quoted.
In `@scripts/desktop.bat`:
- Around line 1-2: Replace the direct invocation of the Gradle wrapper with a
proper batch call and fix argument quoting: use CALL to invoke the wrapper
(e.g., call gradlew or call gradlew.bat) instead of running it directly so the
script returns control, and change the triple-quote argument forwarding
("""%*""") to a single Windows-safe quoted expansion like "%*" so forwarded args
are passed correctly; adjust the existing lines around cd %~dp0.. and the
gradlew desktop:startBisqApp --args invocation accordingly.
In `@scripts/seednode`:
- Around line 2-6: Quote all variable expansions and command arguments to handle
paths with spaces: assign absolute_script_path with
absolute_script_path="$(realpath "$0")", compute scripts_dir_path with
scripts_dir_path="$(dirname "$absolute_script_path")", change directory with cd
"${scripts_dir_path}/..", and call the Gradle wrapper using "./gradlew" and
preserve positional args by passing --args "\"$@\"" (ensure the wrapper path and
any variable expansions are quoted). This ensures realpath, dirname, cd,
./gradlew, and "$@" usages are robust to spaces/special characters.
In `@scripts/statsnode`:
- Around line 2-5: The script uses unquoted expansions that will break on paths
with spaces; update the assignments and cd to quote variable expansions and
command arguments: call realpath with the quoted positional parameter (for
example use realpath on "$0"), assign scripts_dir_path using dirname on the
quoted absolute_script_path, and use a quoted cd target (scripts_dir_path plus
/.. wrapped in quotes) so absolute_script_path, scripts_dir_path, realpath,
dirname and the cd invocation handle paths with spaces and special characters.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
.gitignoreMakefilebtcnodemonitor/build.gradlebuild-logic/app-start-plugin/src/main/kotlin/bisq/gradle/app_start_plugin/AppStartPlugin.ktbuild-logic/commons/src/main/groovy/bisq.application.gradlecli/build.gradledaemon/build.gradledesktop/build.gradleinventory/build.gradlerestapi/build.gradlescripts/apitestscripts/apitest.batscripts/bridgescripts/bridge.batscripts/btcnodemonitorscripts/btcnodemonitor.batscripts/cliscripts/cli.batscripts/daemonscripts/daemon.batscripts/desktopscripts/desktop.batscripts/inventoryscripts/inventory.batscripts/restapiscripts/restapi.batscripts/seednodescripts/seednode.batscripts/statsnodescripts/statsnode.batseednode/bisq.envseednode/build.gradlestatsnode/build.gradle
💤 Files with no reviewable changes (1)
- .gitignore
| application { | ||
| mainClass = 'bisq.desktop.app.BisqAppMain' | ||
| } |
There was a problem hiding this comment.
Remove duplicate application block.
This second application block is redundant - the mainClass is already defined in the first block at lines 9-14. This appears to be a merge artifact and should be removed.
🔧 Proposed fix
-application {
- mainClass = 'bisq.desktop.app.BisqAppMain'
-}
-
sourceSets.main.resources.srcDirs += ['src/main/java'] // to copy fxml and css files📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| application { | |
| mainClass = 'bisq.desktop.app.BisqAppMain' | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@desktop/build.gradle` around lines 20 - 22, Remove the redundant application
block in desktop/build.gradle (the one containing mainClass =
'bisq.desktop.app.BisqAppMain') that duplicates the earlier application { ... }
definition; keep the original application block (the first occurrence that
already defines mainClass) and delete the second/duplicate block to eliminate
the merge artifact.
| cd %~dp0.. | ||
| gradlew bridge:startBisqApp --args """%*""" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Windows cmd.exe batch scripts, does cd %~dp0..switch drives, or iscd /d "%~dp0.." required for cross-drive directory changes?
💡 Result:
cd %~dp0.. does not switch the active drive if %~dp0.. is on a different drive; it only sets that other drive’s “current directory” while leaving you on the original drive. To change both drive and directory in one step, use cd /d "%~dp0.." (or pushd "%~dp0..", which also handles UNC paths). [1][2]
So for cross-drive directory changes in batch scripts, cd /d is required (unless you already happen to be on the same drive). [1][2]
Sources: [1] https://learn.microsoft.com/en-us/windows-server/administration/windows-commands/cd [2] https://stackoverflow.com/questions/18309941/what-does-it-mean-by-command-cd-d-dp0-in-windows
Use drive-safe directory change in batch launcher
cd %~dp0.. does not switch to a different drive; it only sets that drive's current directory while keeping you on the original drive. If the caller is on another drive, Gradle runs from the wrong location.
Proposed fix
-cd %~dp0..
-gradlew bridge:startBisqApp --args """%*"""
+@echo off
+cd /d "%~dp0.." || exit /b 1
+call gradlew.bat bridge:startBisqApp --args "%*"Use cd /d to switch both drive and directory in a single command. The fix also adds error handling, properly invokes the batch file with call, and corrects argument quoting.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cd %~dp0.. | |
| gradlew bridge:startBisqApp --args """%*""" | |
| `@echo` off | |
| cd /d "%~dp0.." || exit /b 1 | |
| call gradlew.bat bridge:startBisqApp --args "%*" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/bridge.bat` around lines 1 - 2, Replace the non-drive-switching
directory change and fragile gradlew invocation in bridge.bat: change the "cd
%~dp0.." usage to a drive-aware command (use cd /d with the "%~dp0.." path),
invoke the gradlew batch with "call" so control returns to the caller, fix the
argument quoting to correctly pass all arguments (use "%*" or proper quoted
expansion), and add simple error handling to bail out with a non-zero exit code
if gradlew fails; target the existing "cd %~dp0.." and "gradlew
bridge:startBisqApp --args """%*"""" fragments when making these edits.
| absolute_script_path=$(realpath $0) | ||
| scripts_dir_path=$(dirname $absolute_script_path) | ||
|
|
||
| cd ${scripts_dir_path}/.. | ||
| ./gradlew daemon:startBisqApp --args "\"$@\"" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and examine the actual scripts/daemon file
find . -name daemon -type fRepository: bisq-network/bisq
Length of output: 77
🏁 Script executed:
# Once found, read the script to verify it matches the snippet
head -20 scripts/daemonRepository: bisq-network/bisq
Length of output: 231
🏁 Script executed:
# Test the current quoting behavior as described in the review
#!/bin/bash
set -euo pipefail
set -- '--host=127.0.0.1' '--password=two words'
cmd=(./gradlew daemon:startBisqApp --args "\"$@\"")
printf 'Current quoting behavior:\n'
printf 'argc=%d\n' "${`#cmd`[@]}"
i=0
for a in "${cmd[@]}"; do
printf '%d: <%s>\n' "$((i++))" "$a"
doneRepository: bisq-network/bisq
Length of output: 195
🏁 Script executed:
# Test what the suggested fix would produce
#!/bin/bash
set -euo pipefail
set -- '--host=127.0.0.1' '--password=two words'
quoted_args=()
for arg in "$@"; do
quoted_args+=("$(printf '%q' "$arg")")
done
cmd=(./gradlew daemon:startBisqApp --args="${quoted_args[*]}")
printf '\nSuggested fix behavior:\n'
printf 'argc=%d\n' "${`#cmd`[@]}"
i=0
for a in "${cmd[@]}"; do
printf '%d: <%s>\n' "$((i++))" "$a"
doneRepository: bisq-network/bisq
Length of output: 183
🏁 Script executed:
# Check how gradlew handles --args with fragmented input
# Simulate what actually gets passed to gradlew with current code
#!/bin/bash
set -euo pipefail
# Current approach simulation
set -- '--host=127.0.0.1' '--password=two words'
echo "=== Current Code Simulation ==="
echo "What gradlew receives:"
# This mimics: ./gradlew daemon:startBisqApp --args "\"$@\""
# The shell expands this to multiple arguments passed to gradlew
args_value="\"$@\""
echo "The --args value passed to gradlew:"
printf '%s\n' "$args_value"
echo -e "\n=== How it's parsed by gradlew ==="
# gradlew will receive --args as one argument, and the quoted string as another
# If args_value contains the quotes, gradlew sees: --args and then the quoted content
echo "gradlew sees 2+ separate arguments here, not --args=<value>"Repository: bisq-network/bisq
Length of output: 287
🏁 Script executed:
# Check if there's any documentation or usage of this script
grep -r "scripts/daemon" . --include="*.md" --include="*.txt" --include="*.rst" 2>/dev/null || echo "No documentation references found"Repository: bisq-network/bisq
Length of output: 94
🏁 Script executed:
# Check if there are any tests for this script
find . -name "*daemon*test*" -o -name "*test*daemon*" -type f 2>/dev/null | head -5Repository: bisq-network/bisq
Length of output: 43
🏁 Script executed:
# Check gradle documentation for --args usage
cat build.gradle 2>/dev/null | grep -A 5 -B 5 "\-\-args" || echo "No --args pattern found in build.gradle"Repository: bisq-network/bisq
Length of output: 100
Fix argument forwarding; current quoting breaks multi-argument input.
The current line 6 (--args "\"$@\"") causes argument fragmentation when multiple arguments are passed. Gradlew receives --args as a separate argument followed by incomplete quoted strings, rather than a single --args=<value> argument. This breaks any invocation with arguments containing spaces or special characters.
The suggested fix properly escapes arguments using printf '%q' and passes them as --args=<consolidated_args>. Additionally, add quotes around variable expansions ($0 and $absolute_script_path) to handle paths with spaces, and use cd ... || exit 1 for robustness.
Suggested fix
-absolute_script_path=$(realpath $0)
-scripts_dir_path=$(dirname $absolute_script_path)
+absolute_script_path="$(realpath "$0")"
+scripts_dir_path="$(dirname "$absolute_script_path")"
-cd ${scripts_dir_path}/..
-./gradlew daemon:startBisqApp --args "\"$@\""
+cd "${scripts_dir_path}/.." || exit 1
+quoted_args=()
+for arg in "$@"; do
+ quoted_args+=("$(printf '%q' "$arg")")
+done
+./gradlew daemon:startBisqApp --args="${quoted_args[*]}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| absolute_script_path=$(realpath $0) | |
| scripts_dir_path=$(dirname $absolute_script_path) | |
| cd ${scripts_dir_path}/.. | |
| ./gradlew daemon:startBisqApp --args "\"$@\"" | |
| absolute_script_path="$(realpath "$0")" | |
| scripts_dir_path="$(dirname "$absolute_script_path")" | |
| cd "${scripts_dir_path}/.." || exit 1 | |
| quoted_args=() | |
| for arg in "$@"; do | |
| quoted_args+=("$(printf '%q' "$arg")") | |
| done | |
| ./gradlew daemon:startBisqApp --args="${quoted_args[*]}" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/daemon` around lines 2 - 6, The script breaks argument forwarding and
path handling: quote the invocations and robustly build a single --args value
from "$@" using shell escaping (e.g. build a variable by iterating over "$@"
with printf '%q' to produce a consolidated_args string) then invoke gradlew with
--args="$consolidated_args"; also quote $0 and $absolute_script_path when
calling realpath/dirname (use "realpath \"$0\"" / quote expansions) and make the
cd to ${scripts_dir_path}/.. robust by using cd ... || exit 1 so the script
exits on failure.
| absolute_script_path=$(realpath $0) | ||
| scripts_dir_path=$(dirname $absolute_script_path) | ||
|
|
||
| cd ${scripts_dir_path}/.. | ||
| ./gradlew restapi:startBisqApp --args "\"$@\"" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's read the actual scripts/restapi file
cat -n scripts/restapi 2>/dev/null || echo "File not found, searching..."Repository: bisq-network/bisq
Length of output: 274
🏁 Script executed:
#!/bin/bash
# Search for the restapi script file
fd -type f -name "restapi" | head -20Repository: bisq-network/bisq
Length of output: 231
🏁 Script executed:
#!/bin/bash
# Run the test from the review to verify Bash expansion behavior
bash -lc 'set -- --host 127.0.0.1; for t in "\"$@\""; do printf "<%s>\n" "$t"; done'Repository: bisq-network/bisq
Length of output: 199
🏁 Script executed:
#!/bin/bash
# Let's also test the proposed fix behavior
bash -lc 'set -- --host 127.0.0.1; for t in "$*"; do printf "<%s>\n" "$t"; done'Repository: bisq-network/bisq
Length of output: 195
🏁 Script executed:
#!/bin/bash
# And test the correct "$@" behavior for comparison
bash -lc 'set -- --host 127.0.0.1; for t in "$@"; do printf "<%s>\n" "$t"; done'Repository: bisq-network/bisq
Length of output: 197
--args "\"$@\"" breaks multi-argument forwarding because Gradle's --args expects a single string value
The current pattern produces malformed token boundaries. With arguments --host 127.0.0.1, "\"$@\"" expands to two tokens: "--host and 127.0.0.1" (with unpaired quotes). When Gradle's option parser processes --args "\"--host 127.0.0.1\", it only recognizes --args with value "\"--host, leaving the second argument as garbage.
The fix should use --args="$*" to concatenate all arguments into a single IFS-separated string, which Gradle's --args expects:
Proposed fix
-absolute_script_path=$(realpath $0)
-scripts_dir_path=$(dirname $absolute_script_path)
+absolute_script_path="$(realpath "$0")"
+scripts_dir_path="$(dirname "$absolute_script_path")"
-cd ${scripts_dir_path}/..
-./gradlew restapi:startBisqApp --args "\"$@\""
+cd "${scripts_dir_path}/.." || exit 1
+./gradlew restapi:startBisqApp --args="$*"The additional quoting changes prevent word splitting and glob expansion on variable expansion, and the cd || exit 1 adds error handling.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| absolute_script_path=$(realpath $0) | |
| scripts_dir_path=$(dirname $absolute_script_path) | |
| cd ${scripts_dir_path}/.. | |
| ./gradlew restapi:startBisqApp --args "\"$@\"" | |
| absolute_script_path="$(realpath "$0")" | |
| scripts_dir_path="$(dirname "$absolute_script_path")" | |
| cd "${scripts_dir_path}/.." || exit 1 | |
| ./gradlew restapi:startBisqApp --args="$*" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/restapi` around lines 2 - 6, Replace the broken argument forwarding
that uses --args "\"$@\"" with a safe single-string expansion --args="$*" so
Gradle receives all CLI args as one string; also ensure you protect variable
expansions (use quoted expansions for absolute_script_path and scripts_dir_path
where used) and make the directory change robust by using cd
${scripts_dir_path}/.. || exit 1 before invoking ./gradlew restapi:startBisqApp
so the script fails fast on cd errors.
| cd %~dp0.. | ||
| gradlew seednode:startBisqApp --args """%*""" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Windows batch file cd /d directory change with spaces and drive letters best practices
💡 Result:
Best practices for cd /d in Windows batch (spaces + drive letters)
-
Use
cd /dwhenever the target might be on another drive (it changes drive + directory in one step). Example:cd /d "D:\Work\Project"
Without
/d,cdmay update the directory forD:but leave you on the current drive. [1] -
Quote paths (especially if they may contain spaces):
cd /d "C:\Program Files\My App"
While
cdcan handle spaces without quotes when command extensions are enabled, quoting is the safest and most portable habit (e.g., across different cmd settings and when variables are involved). [1][3] -
Prefer
pushd/popdovercdwhen you need to return later (and for UNC paths):pushd "\\server\share\Some Folder" rem do work here popd
With command extensions enabled,
pushdcan accept a UNC path and will temporarily map a drive letter;popdreturns and removes that temporary mapping. [2] -
If you use
setlocal/endlocal, don’t expect acdinside to “stick” afterendlocal. Eithercdafterendlocal, or use apushd ./popdworkaround to preserve the directory change acrossendlocalboundaries. [4] -
If your batch file should run relative to its own location, start with:
cd /d "%~dp0"
(This sets working directory to the script’s drive+folder.) [1]
-
Check for failures when changing directories (especially network paths):
pushd "\\server\share\path" || exit /b 1
(
pushdsets a nonzero errorlevel on failure—don’t keep going if the directory didn’t change.) [5]
Sources: Microsoft cd docs [1], Microsoft pushd docs [2], Microsoft KB on quoting paths with spaces [3], setlocal/endlocal directory behavior discussion [4], pushd error handling guidance [5].
🌐 Web query:
Windows batch file argument forwarding triple quotes triple-quote pattern
💡 Result:
In cmd.exe / batch, the “triple-quote” pattern usually refers to nesting quotes so cmd /c (or cmd /k) can accept a single command string and you can still quote an executable/batch path that contains spaces:
cmd /c ""C:\path with spaces\tool.cmd" "arg 1" "arg 2""- The outer
"…"are forcmd /c’s single command-string argument. - The inner quotes are for the script path (or exe path) with spaces. (ss64.com)
For forwarding all arguments from one batch to another, use CALL plus %*:
call "%~dp0child.cmd" %*If you must pass a literal " inside an argument to another batch file, you generally need to double it as "" (cmd’s batch-argument escaping rule), e.g.:
call child.cmd "We had 3"" of rain."because "" is how cmd.exe represents an embedded quote when passing arguments to a batch file. (stackoverflow.com)
Caveat: perfect argument round-tripping through batch files is fundamentally unreliable for some characters because cmd.exe re-interprets the command line rather than treating arguments as opaque literals. (stackoverflow.com)
Citations:
- 1: https://ss64.com/nt/syntax-esc.html?utm_source=openai
- 2: https://stackoverflow.com/questions/562038/escaping-double-quotes-in-batch-script?utm_source=openai
- 3: https://stackoverflow.com/questions/51759330/perfectly-forwarding-arguments-in-batch?utm_source=openai
🏁 Script executed:
# Check if the file exists and verify its current state
find . -name "seednode.bat" -type f | head -5Repository: bisq-network/bisq
Length of output: 83
🏁 Script executed:
cat -n ./scripts/seednode.batRepository: bisq-network/bisq
Length of output: 132
Harden Windows path/arg handling in the launcher.
Line 1 should use the /d flag with quoting and error checking when changing to the parent directory on possibly different drives. Line 2's triple-quote pattern ("""%*""") is fragile for argument forwarding and should use standard quote-wrapped expansion instead.
💡 Suggested fix
-cd %~dp0..
-gradlew seednode:startBisqApp --args """%*"""
+cd /d "%~dp0.." || exit /b 1
+gradlew.bat seednode:startBisqApp --args="%*"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cd %~dp0.. | |
| gradlew seednode:startBisqApp --args """%*""" | |
| cd /d "%~dp0.." || exit /b 1 | |
| gradlew.bat seednode:startBisqApp --args="%*" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/seednode.bat` around lines 1 - 2, The launcher uses an unsafe
change-directory and fragile arg forwarding: replace the first line that
currently reads "cd %~dp0.." with a cd that uses the /d flag and quotes the
parent-directory expansion (so it works across drives) and add a quick error
check/exit on failure; replace the second line that currently uses the
triple-quote pattern "gradlew seednode:startBisqApp --args """%*""" with a
standard quoted expansion for forwarding arguments (use --args "%*") to
correctly preserve arguments and quotes.
1 participant
The AppStartPlugin currently overrides heap settings for the seednode,
and the bisq seednode service unit overrides both heap settings and other JVM
arguments.
These overrides are not applied in the dev setup, which can lead to
inconsistencies and potentially hide bugs during local testing.
This change moves the seednode JVM arguments to the global configuration
to ensure consistent behavior across environments.
Summary by CodeRabbit
New Features
Chores