Closed Merlz closed 4 years ago
Perhaps this is some unintended side effect of https://github.com/gruntwork-io/fetch/releases/tag/v0.3.6? Out of curiosity, does it work with v0.3.5?
Confirmed it works on v0.3.5
Thx for checking! Def sounds like some bug was introduced in #55. @vedala Any chance you recognize what the issue is off the top of your head? If not, we'll have to dig in ourselves.
@Merlz Out of curiosity, was there something in 0.3.7 that you needed?
The --source-path
flag is required for pulling certain files from our own internal repos.
@brikis98 The error encountered by @Merlz is definitely due to changes I made in #55.
Based on a comment in gruntwork-install code, looks like gruntwork-install expects fetch
to accept empty value for --branch
option:
I feel the fix I made in PR #55 is not really needed. Working backwards, I feel the bug reported in issue #31, was not really a bug. If this is needed urgently, I can submit a PR that rolls back the changes. But, to be completely sure, I think we should review the fetch
's expected and actual behavior and then decide whether issue #31 should be considered a bug or not.
@vedala Thanks for looking into it! Yes, gruntwork-install
does expect empty values to work. That way, you can set --branch
, --tag
, etc to variables that might be empty, and so long as one isn't empty, it should still accept it. Otherwise, we'd have to include lots of conditional logic in the gruntwork-install
calls, which would prevent them from being simple one-liners. Perhaps the intended fix for #31 was that if all the values are empty, the error message should disambiguate between the arguments not being set, versus the arguments being set to empty values? @josh-padnick Is that what you had in mind?
@brikis98 It is likely I interpreted the bug incorrectly. Your suggestion sounds right, I will wait for confirmation @josh-padnick and submit a PR with fix.
@josh-padnick Could you chime in here?
You know, when I was looking through #31 myself, I had the same thought as @vedala: this isn't really an error. I'm sorry to create extra work for everyone, but I would be in favor of rolling back #55, especially if it's causing regressions.
@josh-padnick No problem. Thank you for confirming. I will roll back my changes made in #55 and send in a PR soon.
When using the latest version of gruntwork installer
v0.0.24
, it installs Fetchv0.3.2
as part of its install.If I proceed to install the latest version of Fetch
v0.3.7
and have additionalgruntwork-install --module-name XYZ --repo https://github.com/gruntwork-io/xxxx --tag vN.N.N
commands after installing Fetch, then it will fail with the error:==> ubuntu-ami: ERROR: You specified the --branch flag but did not provide any value.
The above only happens when using --module-name and not --binary-name, binaries like gruntkms, ssh-grunt install fine and then it will fail on installing a module.
As a workaround, I installed Fetch v0.3.7 last in the list of GW modules and then everything works as expected in building the packer image. I require that the latest fetch is installed on the AMI, which is why I'm not leaving v0.3.2 in place.
This is my
install-gruntwork-modules.sh
that is called from the packer json.If the
install_fetch ${FETCH_VERSION}
is called directly afterinstall_bash_commons ${BASH_COMMONS_VERSION}
and prior to other modules, it will fail to install any subsequent modules, but if the install function is called last, afterinstall_stateful_server_packages ${MODULE_STATEFUL_SERVER_VERSION}
then it works.Here is a full error output: