Deps/Build Deps Should Be Commented Out in "plan init"

[sirajrauff]

Currently running hab plan init in sample-node-app (after removing the existing habitat folder) will produce the following:

...

# Optional.
# An array of package dependencies needed at runtime. You can refer to packages
# at three levels of specificity: `origin/package`, `origin/package/version`, or
# `origin/package/version/release`.
pkg_deps=(core/glibc)

# Optional.
# An array of the package dependencies needed only at build time.
pkg_build_deps=(core/make core/gcc)

...

# Optional.
# An array of paths, relative to the final install of the software, where
# libraries can be found. Used to populate LD_FLAGS and LD_RUN_PATH for
# software that depends on your package.
# pkg_lib_dirs=(lib)

# Optional.
# An array of paths, relative to the final install of the software, where
# headers can be found. Used to populate CFLAGS for software that depends on
# your package.
# pkg_include_dirs=(include)

Unlike the other "optional" plan variables, pkg_deps and pkg_build_deps are not commented out. See template_plan.sh#L129 vs template_plan.sh#L155.

Perhaps this departure was a conscious choice, however, this is doubly strange when scaffolding is detected; the expectation being this should be the responsibility of scaffolding, especially considering in pre-0.80 hab plan init would produce vastly different plans for scaffolding detected vs none.

[dmccown]

Thank you for submitting this issue! This looks like an oversight when we were making some changes to how we template the plan init. Let's comment the two optional dependency parameters just as we comment out the others.

[predominant]

@dmccown @nellshamrell

After reviewing the triage video from this week. I'm with @smacfarlane on the importance of these variables. But again, perhaps I am tainted by core plans contributions.

I feel that even if these can be empty, they should be specified. The feedback could be changed from hab-plan-build to ensure these are present and valid variables, before proceeding with the build, ensuring a quick feedback cycle for the developer.

I understand that they can be empty, and currently defaults will suffice, so commenting them out works. However, I feel that the importance of these is enough that a change in behaviour to require these in all plans would be beneficial in educating the user, and being explicit about dependencies for all plans (Defining no deps is as important as defining deps).

[bdangit]

Is the default of deps empty? Meaning if someone doesn’t have these variables is it empty?

[predominant]

Yep.

https://github.com/habitat-sh/habitat/blob/master/components/plan-build/bin/hab-plan-build.sh#L375-L377

[sirajrauff]

@predominant Just wanted to note considering your comment above - I would understand if the variables are required, and as such are always initialized during hab plan init but there's two parts of this that were still unexpected to me:

1. They're pre-filled. When using this package I had actually successfully built & ran the package before noticing that the variables had been defined; going back to check on the deployed system, core/glibc, core/make, and core/gcc had been installed, and since adding an unnecessary dependency won't cause an error this can easily be missed.
2. The use of scaffolding here; hab plan init correctly detected nodejs. Even if the functionality is changed to include these variables by default, should there be different behaviour for scaffolding? I believe this would make sense with your suggestion of validating this in hab-plan-build, as it would then verify that the scaffolding provides such values if the user has not?

[predominant]

@sirajrauff

1. Agreed, init with default values that don't consider what you're building isn't ideal. They should be generated empty for a plain plan, with comments showing a common example.
2. I can't speak to scaffolding much. I was still under the impression we were removing support for it at some point.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We value your input and contribution. Please leave a comment if this issue still affects you.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We value your input and contribution. Please leave a comment if this issue still affects you.

[stale[bot]]

This issue has been automatically closed after being stale for 400 days. We still value your input and contribution. Please re-open the issue if desired and leave a comment with details.

[sirajrauff]

Still an issue.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We value your input and contribution. Please leave a comment if this issue still affects you.

[stale[bot]]

This issue has been automatically closed after being stale for 400 days. We still value your input and contribution. Please re-open the issue if desired and leave a comment with details.