Fail check/ts-build-current when bundle files cannot be built

[pavoljuhas]

The check/ts-build-current CI job could pass if check/ts-build
failed before writing the bundle files.

[pavoljuhas]

cc: @seunomonije (related to #7687)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.37%. Comparing base (8137600) to head (d70f4c0).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7696      +/-   ##
==========================================
- Coverage   99.38%   99.37%   -0.01%     
==========================================
  Files        1089     1089              
  Lines       97550    97550              
==========================================
- Hits        96948    96945       -3     
- Misses        602      605       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[viathor]

Optional: Would it be appropriate to use set -e in this script?

[mhucka]

I'm not Pavol, but I just had an experience involving set -e so feeling compelled to chime in. Using set -e would obviate the need for the exit statement in this one place, but it would also cause the script to exit if git diff on line 27 found any differences. (The use of set -e causes errors inside $(…) commands to "bubble up", in a sense.) That in turn would prevent the rest of the logic in the script from working as desired.

[pavoljuhas]

In this PR I'd like to do a minimum change to prevent failing builds going unnoticed and do a more complete fix later. As for the set -e the rules how it applies are a bit byzantine (e.g., true && false does exit, but false && true does not) to a level it is easier for me to reason about the script without errexit.

BTW, I am going to hold on with this PR a bit, because it would start failing the CI for unrelated contributions. The fix for the ts-build is in works at #7697 so I will wait for it with the merge here.

[viathor]

@mhucka IIUC, exiting when there are untracked files is the intended behavior. Indeed, it is what we do explicitly in the conditional, so set -e may simplify the script.

@pavoljuhas It was only a suggestion.

[viathor]

@mhucka IIUC, exiting when there are untracked files is the intended behavior. Indeed, it is what we do explicitly in the conditional, so set -e may simplify the script.

@pavoljuhas It was only a suggestion.

[mhucka]

Re the comment above:

@mhucka IIUC, exiting when there are untracked files is the intended behavior. Indeed, it is what we do explicitly in the conditional, so set -e may simplify the script.

True, but right now, the script prints an explanatory message. With set -e, it would exit before it got to the part where it prints the message:

if [[ -n "$untracked" ]]; then
    echo -e "\033[31mERROR: Uncommitted changes to bundle file(s) found! Please commit these files:\033[0m"
    for generated in $untracked
    do
        echo -e "\033[31m ${generated}\033[0m"
    done

As far as I can see, the behavior of git diff --name-only HEAD is to print a list of files without any further explanation. So while both approaches would exit with an exit code of 1, IMHO there is some value in explaining to the user why that happened.