-
Notifications
You must be signed in to change notification settings - Fork 21
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
Add macOS support; input validation; an example #13
base: master
Are you sure you want to change the base?
Changes from 3 commits
65d82ba
e38c6be
80820d0
e45087e
5737075
c68c657
b61da0c
89853d6
6b53197
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -194,20 +194,20 @@ copy_playbook() { | |||||
shift 4; local extra_deps=("${@}") | ||||||
|
||||||
echo "Adding playbook..." | ||||||
cp -L --preserve=timestamps "$playbook_file" "$tmpdir/playbook.yml" | ||||||
cp -L -p "$playbook_file" "$tmpdir/playbook.yml" | ||||||
l50 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
if [ -d "$playbook_path/roles" ]; then | ||||||
echo "Adding roles..." | ||||||
cp -Lr --preserve=timestamps "$playbook_path/roles" "$tmpdir" | ||||||
cp -Lrp "$playbook_path/roles" "$tmpdir" | ||||||
fi | ||||||
|
||||||
if [ -n "$requirements_file" ]; then | ||||||
echo "Adding requirements..." | ||||||
cp -L --preserve=timestamps "$requirements_file" "$tmpdir/requirements.yml" | ||||||
cp -L -p "$requirements_file" "$tmpdir/requirements.yml" | ||||||
l50 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
fi | ||||||
|
||||||
if [ -n "$vars_file" ]; then | ||||||
echo "Adding vars..." | ||||||
cp -L --preserve=timestamps "$vars_file" "$tmpdir/vars.yml" | ||||||
cp -L -p "$vars_file" "$tmpdir/vars.yml" | ||||||
l50 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
fi | ||||||
|
||||||
if [ ${#extra_deps[@]} -gt 0 ]; then | ||||||
|
@@ -218,7 +218,7 @@ copy_playbook() { | |||||
fi | ||||||
|
||||||
echo "Adding ansible config..." | ||||||
cp -L --preserve=timestamps "$ETC_PATH/ansible.cfg" "$tmpdir" | ||||||
cp -L -p "$ETC_PATH/ansible.cfg" "$tmpdir" | ||||||
l50 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
} | ||||||
|
||||||
add_python_requirements() { | ||||||
|
@@ -290,20 +290,34 @@ install_roles() { | |||||
|
||||||
add_entrypoint() { | ||||||
echo "Adding entrypoint..." | ||||||
cp -L --preserve=timestamps "$LIB_PATH/run-playbook.sh" "$tmpdir" | ||||||
chmod 0755 "$tmpdir/run-playbook.sh" | ||||||
if [ -e "$LIB_PATH/run-playbook.sh" ]; then | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This check should not be here, instead we must evaluate that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm afraid I don't understand your feedback here, but I'm happy to implement whatever proposed changes you have. |
||||||
cp -L -p "$LIB_PATH/run-playbook.sh" "$tmpdir" | ||||||
chmod 0755 "$tmpdir/run-playbook.sh" | ||||||
else | ||||||
echo "File not found: $LIB_PATH/run-playbook.sh" | ||||||
fi | ||||||
} | ||||||
|
||||||
compress_output() { | ||||||
local output_file=$1 | ||||||
local output_file=$1 | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Don't change the indentation. See |
||||||
|
||||||
echo "Packaging files to the output..." | ||||||
echo "Packaging files to the output..." | ||||||
l50 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
# First add the header to the final script | ||||||
cat "$LIB_PATH/bin-header.sh" > "$output_file" | ||||||
# First add the header to the final script | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This check should not be here, instead we must evaluate that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm afraid I don't understand your feedback here, but I'm happy to implement whatever proposed changes you have. |
||||||
if [ -e "$LIB_PATH/bin-header.sh" ]; then | ||||||
cat "$LIB_PATH/bin-header.sh" > "$output_file" | ||||||
else | ||||||
echo "File not found: $LIB_PATH/bin-header.sh" | ||||||
fi | ||||||
|
||||||
SED_BINARY='' | ||||||
# Do some build-time metadata insertion | ||||||
sed -i'' \ | ||||||
if [[ "$(uname)" == 'Darwin' ]]; then | ||||||
SED_BINARY=`which gsed` | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here you're adding a hidden dependency on gnu-sed which implies the user will have it installed. I know that the only thing that kept you from using sed -i.bkp -e '...' "$output_file"; rm "${output_file}.bkp" And we avoid the conditional and the hidden dependency. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added docs for installing gnu grep if on a Mac to the README. It saves on the temp file. |
||||||
else | ||||||
SED_BINARY=`which sed` | ||||||
fi | ||||||
$SED_BINARY -i'' \ | ||||||
-e "s/UNCOMPRESS_SKIP=.*/UNCOMPRESS_SKIP=$(( $(wc -l < "$output_file") + 1 ))/" \ | ||||||
-e "s/\$VERSION/$VERSION/g" \ | ||||||
"$output_file" | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
--- | ||
|
||
- hosts: all | ||
|
||
tasks: | ||
- name: basic-example | ||
debug: | ||
msg: You are running a basic example | ||
|
||
- name: Print extra variable 'example' | ||
debug: | ||
msg: "Example value is {{ example }}" | ||
l50 marked this conversation as resolved.
Show resolved
Hide resolved
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to change any of that, even if you just want to run
bin/bundle-playbook
, you don't have to build it.Changing these will prevent the build for other platforms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your feedback. I understand that changing these variables might prevent the build for other platforms. My intention was to avoid hardcoding the paths, as it could cause issues in different environments. However, I understand that it might not be the best solution for all cases.
Would you have any suggestions on how to make these paths more flexible without affecting other platforms? I am open to any ideas that could help us find a more suitable solution.