git-arc: Do not echo unescaped literals to jq
ClosedPublic
Actions

Authored by jlduran on Sep 24 2024, 12:39 AM.

Details

Reviewers

markj
imp
0mp

Commits

rG3ab8697d9e45: git-arc: Do not echo unescaped literals to jq

Summary

Store the json blobs in temporary files instead of variables, and feed
them to jq for parsing.

When echoing "\n", the new line will become a literal new line, and
therefore, invalid json input:

jq: parse error: Invalid string: control characters from U+0000 through U+001F must be escaped ...

To reproduce:

% git arc patch -c D12345

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 59576
Build 56463: arc lint + arc unit

Event Timeline

jlduran created this revision.Sep 24 2024, 12:39 AM

Herald added a subscriber: imp. · View Herald TranscriptSep 24 2024, 12:39 AM

jlduran requested review of this revision.Sep 24 2024, 12:39 AM

Harbormaster completed remote builds in B59576: Diff 143656.Sep 24 2024, 12:39 AM

I'm not a big fan of making temporary files, but I don't know how else to fix it.

jlduran edited the summary of this revision. (Show Details)Sep 24 2024, 12:40 AM

I think this looks good

This revision is now accepted and ready to land.Sep 24 2024, 12:41 AM

Thank you.. Hmm I am pretty sure I included @emaste in the review. Well, still learning git-arc.

Looks ok to me, thank you.

I think I know how to achieve the same result using printf, and escaping literals with %s.
Regardless, I only have one more patch for git-arc (git arc list) that may benefit of review_data being in a file. If it turns out otherwise, I can "revert" to using variables.
I'm done trying out this great article on the topic.
Thank you for the reviews!

In D46767#1066872, @jlduran_gmail.com wrote:

I think I know how to achieve the same result using printf, and escaping literals with %s.
Regardless, I only have one more patch for git-arc (git arc list) that may benefit of review_data being in a file. If it turns out otherwise, I can "revert" to using variables.
I'm done trying out this great article on the topic.
Thank you for the reviews!

I think using files is fine. We don't care about the cost of creating a few extra temporary files here.

jlduran mentioned this in D47274: git-arc: Use printf instead of echo.Oct 24 2024, 3:33 PM

0mp accepted this revision.Oct 25 2024, 1:20 PM

0mp added a subscriber: 0mp.

0mp added inline comments.

tools/tools/git/git-arc.sh
552	Note: backslashes are not necessary after a pipe. However, this style bit is already inconsistent in this file, so it's just a note and not a request for changes :)

Closed by commit rG3ab8697d9e45: git-arc: Do not echo unescaped literals to jq (authored by jlduran, committed by 0mp). · Explain WhyOct 25 2024, 1:24 PM

This revision was automatically updated to reflect the committed changes.

0mp added a commit: rG3ab8697d9e45: git-arc: Do not echo unescaped literals to jq.

des added a subscriber: des.Oct 25 2024, 2:06 PM

des added inline comments.

tools/tools/git/git-arc.sh
584	whenever you create a temporary file, you should set a trap to delete it on exit, e.g. trap 'rc=$?; rm -f "$review_data" "$user_data" "$diff_data" "$tmp"; trap - 0; exit $rc' 0 1 2 3 5 10 15 this is easier if you use a temporary directory instead, as you can just rm -rf the directory.