aws-samples / amazon-transcribe-live-call-analytics

Amazon Transcribe Live Call Analytics (LCA) Sample Solution
Other
77 stars 47 forks source link

Build artifacts fails when I run publish.sh on macOS (LCA VERSION=0.8.9). #117

Closed intptr-t closed 7 months ago

intptr-t commented 7 months ago

Problem

When I run publish.sh on macOS, I get the following error with sed.

## Variable Parameters
CFN_NEW_BUCKET_NAME="lca-`date +%Y%m%d-%H%M%S`-`cat /dev/urandom | LC_CTYPE=C tr -dc "a-z0-9" | fold -w 4 | head -n 1`"
CFN_PREFIX="lca"
AWS_REGION="ap-northeast-1"

## Run publish
./publish.sh $CFN_NEW_BUCKET_NAME $CFN_PREFIX $AWS_REGION

... (snip) ...

PACKAGING submodule-aws-qnabot
Applying patch files to simplify UX by removing some QnABot options not needed for LCA
./patches/qnabot/lambda_schema_qna.js -> submodule-aws-qnabot/lambda/schema/qna.js
./patches/qnabot/website_js_admin.vue -> submodule-aws-qnabot/website/js/admin.vue
./patches/qnabot/Makefile -> submodule-aws-qnabot/Makefile
modify QnABot version string from 'N.N.N' to 'N.N.N-LCA'
sed: 1: "submodule-aws-qnabot/pa ...": unterminated substitute in regular expression

Cause

Because sed on macOS is BSD-based, the "-i" option is handled differently. Therefore, sed -i 's/"version": *"\([0-9]*\. [0-9]*\. [0-9]*\)"/"version":"\1-LCA"/' $dir/package.json will cause an error in ./publish.sh.

Reference: https://stackoverflow.com/questions/4247068/sed-command-with-i-option-failing-on-mac-but-works-on-linux

Proposal

Since GNU-based sed and BSD-based sed are currently incompatible, conditional branching is required for completion in the shell alone.

Note The -i'' hack has been fixed in macOS 10.14 (Mojave) and later, so it does not seem to work.

Environment

macOS: Sonoma 14.1

% sw_vers
ProductName:        macOS
ProductVersion:     14.1
BuildVersion:       23B74
rstrahan commented 7 months ago

Thanks for the very clearly written issue, and proposal, @intptr-t ! You say:

Since GNU-based sed and BSD-based sed are currently incompatible, conditional branching is required for completion in the shell alone.

Do you know the right condition to use? If so, do you want to send a PR with proposed fix? If so, we can quickly validate and merge it for the next release. LMK. Thanks!

intptr-t commented 7 months ago

Thanks for the response @rstrahan. I have created a PR. I hope you will check it out.

rstrahan commented 7 months ago

Merged the PR.. Your fix will be in next release. Thanks for being an LCA contributor!