Skip to content

Comments

startami: strip trailing '})' from amicmd to avoid eval syntax error#332

Open
laurentroque wants to merge 1 commit intopcdshub:masterfrom
laurentroque:fix-startami-amicmd
Open

startami: strip trailing '})' from amicmd to avoid eval syntax error#332
laurentroque wants to merge 1 commit intopcdshub:masterfrom
laurentroque:fix-startami-amicmd

Conversation

@laurentroque
Copy link
Contributor

Description

Strip trailing config-artifact characters (e.g., })) from the amicmd string constructed in scripts/startami before it is executed.

Motivation and Context

startami -c cxi_0.cnf could fail with:
eval: syntax error near unexpected token ')'

On CXI hosts, amicmd was being parsed from the ami_client line in cxi_0.cnf and could end up with a trailing }), causing bash eval to error. This change sanitizes the constructed command to avoid the syntax error.

How Has This Been Tested?

  • cxi-monitor: /cds/home/i/iroque/programs/engineering_tools/scripts/startami -c cxi_0.cnf (reaches expected restart prompt; no eval syntax error)
  • cxi-control: /cds/home/i/iroque/programs/engineering_tools/scripts/startami -c cxi_0.cnf (successfully launches online_ami; no eval syntax error)
  • xcs-control: /cds/home/i/iroque/programs/engineering_tools/scripts/startami (successfully launches online_ami; no eval syntax error)

Where Has This Been Documented?

  • Inline comment added next to the new amicmd sanitisation line in scripts/startami.
  • Jira ticket: ECS-9707

@laurentroque laurentroque requested a review from a team as a code owner January 30, 2026 22:55

amicmd=$(grep ami_client /reg/g/pcds/dist/pds/"$HUTCH"/scripts/"$CONFIG" | grep -v '#' | awk 'BEGIN { FS = ":" }; { print $4}' | sed s/ami_GUI_path//g | sed s/\'+proxy_cds/"$proxy_cds"/g | sed s:\'+expname:"$EXPNAME"/:g | sed s/+\'//g | sed s/\'\}\)//g)
# Strip trailing cnf/python artifacts (e.g. "})") so eval doesn't choke
amicmd=$(printf "%s" "$amicmd" | sed 's/[[:space:]]*[})][})]*$//')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some nitpicky questions, if this works I think it's correct to merge without addressing these.
I'm mostly trying to update my own sed/regex knowledge.

  • Why is this a new command using printf instead of joining onto the | sed train on line 74?
  • (Conversely, if new commands are preferred for readability, maybe we should unwrap more of them)
  • Why printf with no formatting over echo?
  • What's [[:space:]]* doing in the sed here? Are you expecting e.g. })? Your PR description only mentions })
  • It looks like you're using a literal [})] followed by a [})]*, but in regex we have a + operator for matching 1 or more instances of a symbol. This could have been [})]\+

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants