Add a --configuration option to the index binary#628
Conversation
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
🤖 Augment PR SummarySummary: Adds a dedicated Changes:
Technical Notes: Configuration output is prettified to stdout and the command exits early after reading the configuration JSON. 🤖 Was this summary useful? React with 👍 or 👎 |
| if (app.contains("verbose")) { | ||
| sourcemeta::core::prettify(raw_configuration, std::cerr); | ||
| std::cerr << "\n"; | ||
| if (app.contains("configuration")) { |
There was a problem hiding this comment.
--configuration returns before the --url override block later in index_main, so invocations like ... --url <...> --configuration will print the non-overridden URL (and other derived values) rather than the effective configuration; the Makefile now calls this combination in sandbox-index. Is that the intended behavior for the new option?
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| sourcemeta::core::prettify(raw_configuration, std::cerr); | ||
| std::cerr << "\n"; | ||
| if (app.contains("configuration")) { | ||
| sourcemeta::core::prettify(raw_configuration, std::cout); |
There was a problem hiding this comment.
In --configuration mode the JSON is written to stdout, but the program also prints the version banner to stdout at startup, so the overall stdout stream isn’t valid JSON (the new tests work around this via tail -n +2). If --configuration is meant for scripting/pipe use, this mixed output could be surprising.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
2 issues found across 9 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/index/index.cc">
<violation number="1" location="src/index/index.cc:105">
P2: `--configuration` should be a read-only operation, but it still constructs `Output` before returning, which creates/scans the output directory. Consider moving the configuration-print/exit path before `Output` initialization so it doesn’t touch the filesystem when only dumping the configuration.</violation>
</file>
<file name="test/cli/index/enterprise/sourcemeta-std.sh">
<violation number="1" location="test/cli/index/enterprise/sourcemeta-std.sh:17">
P2: The pipeline only checks `tail`'s exit status, so a failure in the index command can be masked. Run the command separately (or capture its output) so `errexit` stops the test on failure.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if (app.contains("verbose")) { | ||
| sourcemeta::core::prettify(raw_configuration, std::cerr); | ||
| std::cerr << "\n"; | ||
| if (app.contains("configuration")) { |
There was a problem hiding this comment.
P2: --configuration should be a read-only operation, but it still constructs Output before returning, which creates/scans the output directory. Consider moving the configuration-print/exit path before Output initialization so it doesn’t touch the filesystem when only dumping the configuration.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/index/index.cc, line 105:
<comment>`--configuration` should be a read-only operation, but it still constructs `Output` before returning, which creates/scans the output directory. Consider moving the configuration-print/exit path before `Output` initialization so it doesn’t touch the filesystem when only dumping the configuration.</comment>
<file context>
@@ -102,9 +102,10 @@ static auto index_main(const std::string_view &program,
- if (app.contains("verbose")) {
- sourcemeta::core::prettify(raw_configuration, std::cerr);
- std::cerr << "\n";
+ if (app.contains("configuration")) {
+ sourcemeta::core::prettify(raw_configuration, std::cout);
+ std::cout << "\n";
</file context>
| } | ||
| EOF | ||
|
|
||
| "$1" "$TMP/one.json" "$TMP/output" --configuration | tail -n +2 > "$TMP/output.txt" |
There was a problem hiding this comment.
P2: The pipeline only checks tail's exit status, so a failure in the index command can be masked. Run the command separately (or capture its output) so errexit stops the test on failure.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At test/cli/index/enterprise/sourcemeta-std.sh, line 17:
<comment>The pipeline only checks `tail`'s exit status, so a failure in the index command can be masked. Run the command separately (or capture its output) so `errexit` stops the test on failure.</comment>
<file context>
@@ -0,0 +1,54 @@
+}
+EOF
+
+"$1" "$TMP/one.json" "$TMP/output" --configuration | tail -n +2 > "$TMP/output.txt"
+
+cat << EOF > "$TMP/expected.txt"
</file context>
Signed-off-by: Juan Cruz Viotti jv@jviotti.com