Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Add a params.pp file #59

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Add a params.pp file #59

wants to merge 5 commits into from

Conversation

buzzdeee
Copy link

in order to allow OS specific differences to be set
here. It's included in init.pp archive defined type, and make
sure with an order, that it is instantiated in the catalog
before the defined type is executed.

The order of evaluation in the catalog is ensured by the
include line in init.pp, and the following two lines,
setting the order.

The defined variables specific for OpenBSD are:
$tarcmd string, defining a shell command to run tar
$digest_types hash, defining the supported hash types, and their
shell commands

The $tarcmd is used in extract.pp, the $digest_types is used
in download.pp

The only extra ::osfamily is OpenBSD, the rest is the values
taken in the default: case of the case statement.

The change in download.pp, uses the keys() function from stdlib.
Because of that, stdlib is added as dependency to metadata.json,
and .fixtures.yml. I chose an arbitrary version as minimal version
not too old, and not too recent. Otherwise, stdlib 3.X should
also work.

Added OpenBSD to the metadata.json, in order to get the specs
covering it too.

Let me know if there is something wrong, or needs change/enhancement etc.

@buzzdeee
Copy link
Author

A little addition, introducing the $binary_indicator in params.pp, since the hash commands on OpenBSD don't distinguish between binary and text files.

@raphink
Copy link
Contributor

raphink commented Aug 18, 2016

Sorry for the very long delay. Could you rebase this on master please?

here, and include it in init.pp archive defined type, and make
sure with an order, that it is instantiated in the catalog
before the defined type is executed.

The order of evaluation in the catalog is ensured by the
include line in init.pp, and the following two lines,
setting the order.

The defined variables specific for OpenBSD are:
$tarcmd string, defining a shell command to run tar
$digest_types hash, defining the supported hash types, and their
                    shell commands

The $tarcmd is used in extract.pp, the $digest_types is used
in download.pp

The only extra ::osfamily is OpenBSD, the rest is the values
taken in the default: case of the case statement.

The change in download.pp, uses the keys() function from stdlib.
Because of that, stdlib is added as dependency to metadata.json,
and .fixtures.yml. I chose an arbitrary version as minimal version
not too old, and not too recent. Otherwise, stdlib 3.X should
also work.

Added OpenBSD to the metadata.json, in order to get the specs
covering it too.
@buzzdeee
Copy link
Author

buzzdeee commented Aug 30, 2016

nevermind, rebased ;)

Had to add a case statement for Solaris now to the params.pp, where I set the gtar command as in OpenBSD, and the supported hashes as in the default section of the case statement.
Then I did some additional change/tweak to extract.pp to take the solaris case into account,
and keep the tar_command parameter available as it was before.

Let me know if that's ok, or something should be changed, or maybe squashed.

@buzzdeee buzzdeee force-pushed the master branch 2 times, most recently from 11dc7b2 to d75c797 Compare August 30, 2016 09:50
The hash commands on OpenBSD don't distinguish between binary
and text files, so they don't handle the '*' in front of the
file name to be checked.

additionally update metadata.json for more current OpenBSD versions
@buzzdeee buzzdeee force-pushed the master branch 2 times, most recently from 4231b94 to 88fb2cf Compare August 30, 2016 10:30
) {

require ::archive::params
Copy link
Contributor

Choose a reason for hiding this comment

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

There's no resources in params, so include will do just the same as require.

Copy link
Author

Choose a reason for hiding this comment

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

I'm not exactly sure about that one. Does an include enforce order of evaluation?

the require ensures ::archive::params is evaluated before the extract.pp is evaluated, so that the
values of ::archive::params::FOOBAR are available, when used later in the defined type, does the
include enforce the same?

Copy link
Contributor

Choose a reason for hiding this comment

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

You're mixing up order of parsing and order of evaluation.

Parsing is done on the master side, always top-down.

Evaluation is done on the agent, based on the catalog, and following the dependency tree based on either implicit or explicit dependencies between resources.

The params class has no resources at all, only variables, so there's no need for evaluation ordering. Variables will be available if they have been parsed before, which is true if you included the class already.

@buzzdeee
Copy link
Author

buzzdeee commented Sep 1, 2016

I changed the require to includes, as well as add a test for the parameter and the variable, and used the fail() function. I hope that is as it was meant. Otherwise let me know.

In case everything is fine, squash?

if $root_dir {
$extract_dir = "${target}/${root_dir}"
} else {
$extract_dir = "${target}/${name}"
}

if ! $tar_command and ! $::archive::params::tarcmd {
fail("${module_name}: parameter \$tar_command not specified and \$::archive::params::tarcmd not found")
Copy link
Contributor

Choose a reason for hiding this comment

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

If you include archive::params on line 47, there's no reason why you would get in a fail situation here. However, if you use $::archive::params::tarcmd as the default value for $tar_command, then you can check that $tar_command is indeed defined here and fail otherwise, and then you don't need the $real_tar_command variable below.

@@ -44,7 +44,7 @@
$tar_command=$::archive::params::tarcmd,
) {

include ::archive::params
require ::archive::params
Copy link
Contributor

Choose a reason for hiding this comment

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

No, really. include is all you need. If it fails, it must be another reason.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants