Skip to content

fix scripts to run in mac os x and or linux #2260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 2, 2019
Merged

fix scripts to run in mac os x and or linux #2260

merged 2 commits into from
Dec 2, 2019

Conversation

camilamacedo86
Copy link
Contributor

@camilamacedo86 camilamacedo86 commented Nov 22, 2019

Description of the change:
fix scripts to run in mac os x and/or Linux

Motivation for the change:
Closes #2257

@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 22, 2019
@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 25, 2019
@camilamacedo86 camilamacedo86 changed the title fix scripts to run in mac os x and or linux WIP: fix scripts to run in mac os x and or linux Nov 25, 2019
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 25, 2019
sed -i " " 's|\(FROM quay.io/operator-framework/ansible-operator\)\(:.*\)\?|\1:dev|g' build/Dockerfile
else
sed -i 's|\(FROM quay.io/operator-framework/ansible-operator\)\(:.*\)\?|\1:dev|g' build/Dockerfile
fi
Copy link
Member

Choose a reason for hiding this comment

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

Can this be combined into one command, similar to the others?

@camilamacedo86 camilamacedo86 changed the title WIP: fix scripts to run in mac os x and or linux fix scripts to run in mac os x and or linux Nov 28, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 28, 2019
@camilamacedo86 camilamacedo86 added kind/bug Categorizes issue or PR as related to a bug. area/testing Issue related to testing the operator-sdk and subcomponents labels Nov 29, 2019
sed -i "s|{{ REPLACE_IMAGE }}|$DEST_IMAGE|g" deploy/operator.yaml
sed -i 's|{{ pull_policy.default..Always.. }}|Never|g' deploy/operator.yaml
if [[ "$OSTYPE" == "darwin"* ]]; then
sed -i " " "s|{{ REPLACE_IMAGE }}|$DEST_IMAGE|g" deploy/operator.yaml
Copy link
Contributor Author

@camilamacedo86 camilamacedo86 Nov 29, 2019

Choose a reason for hiding this comment

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

@joelanford just here that I could not find a solution because of "{{".

Copy link
Member

@joelanford joelanford Dec 2, 2019

Choose a reason for hiding this comment

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

EDIT:

sed -i".bak" -E -e "s|\{\{ REPLACE_IMAGE \}\}|$DEST_IMAGE|g" deploy/operator.yaml; rm -f deploy/operator.yaml.bak
sed -i".bak" -E -e 's|\{\{ pull_policy.default..Always.. \}\}|Never|g' deploy/operator.yaml; rm -f deploy/operator.yaml.bak

Copy link
Member

Choose a reason for hiding this comment

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

I think I was wrong originally when I said that | characters don't work with BSD sed. It seems that they do, in fact, work with both GNU sed and BSD sed. So I think we should use | everywhere we were using it before so that we don't have to escape the forward slashes in the regex patterns and replacements.

Comment on lines 161 to 163
sed -i 's|{{ pull_policy.default..Always.. }}|Always|g' deploy/operator.yaml
sed -i".bak" -e 's/{{ pull_policy.default..Always.. }}/Always/g' deploy/operator.yaml; rm -f deploy/operator.yaml.bak
cp deploy/operator.yaml deploy/operator-copy.yaml
sed -i "s|{{ REPLACE_IMAGE }}|$IMAGE|g" deploy/operator.yaml
Copy link
Member

Choose a reason for hiding this comment

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

The files in the ci directory do not need to be changed since:

  1. They are currently unused in this repo
  2. They will soon be transitioned into the openshift/ocp-release-operator-sdk repo
  3. When used with OpenShift CI, they do not need to support mac's sed command syntax.

Copy link
Member

@joelanford joelanford left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 2, 2019
@camilamacedo86 camilamacedo86 merged commit a499c46 into operator-framework:master Dec 2, 2019
@camilamacedo86 camilamacedo86 deleted the shell branch December 2, 2019 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing Issue related to testing the operator-sdk and subcomponents kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Commands using sed the shell scripts do not work for Mac OS
3 participants