If go-core.bash is sourced in a function, go-script breaks

nkakouros commented 6 years ago

Due diligence

[x] I am familiar with the "Reporting issues" section of CONTRIBUTING.md
[x] I have searched the existing issues and haven't found one that matches what I'm reporting or requesting now.
[x] If this is a bug report, I have searched the Bash changelog for information that may explain or provide clues to the behavior I'm observing and reference it in the body of the report.

Framework, Bash, and operating system version information

_GO_CORE_VERSION:         v1.7.0
BASH_VERSION:             4.4.19(1)-release
OSTYPE:                   linux-gnu
_GO_PLATFORM_ID:          arch
_GO_PLATFORM_VERSION_ID:

Description

If the go-core.bash file is sourced in a function and not in the main body of the intro script of a project, then many variables are not set properly. For instance:

load_libs() {
  . "${0%/*}/go-core.bash" "scripts"
}

load_libs
@go "$@"

will fail since variables like _GO_USE_MODULES and _GO_CORE_DIR will be undefined.

In this case, the solution is easy, just declare the missing variables as global, not only as exported.

Should this be considered a use case go-script-bash should cater for?

mbland commented 6 years ago

I'm not sure. On the one hand, I'm inclined to wonder why anyone would want to encapsulate sourcing the framework in a function, other than perhaps for documentation reasons or for bundling together the import of multiple frameworks. (Possible workaround: create a libs.sh that sources all the frameworks, then source that file in one line in your main script.)

On the other, I was originally strict about limiting the scope of framework-wide variables (e.g. using declare -r), but over time started relaxing the strict scoping of such variables when the restrictions started causing problems in newer modules.

By default, I'm not inclined to change the current declarations. But I'm open to arguments presenting a compelling use case, since I don't think the changes would be that difficult or dangerous (especially given how thoroughly everything's tested).

nkakouros commented 6 years ago

Maybe declaring all these variables as global is just too messy. The workaround you suggest allows for sufficient tidying up of the code. So, issue resolved for me!

mbland / go-script-bash