chocolatey / cChoco

Community resource to manage Chocolatey
Apache License 2.0
153 stars 99 forks source link

cChocoPackageInstall 'Source' cannot be null or empty #96

Closed jrdnr closed 6 years ago

jrdnr commented 6 years ago

I was trying to implement a fix for #80, and ran into errors on Test-TargetResource, PowerShell DSC resource cChocoPackageInstall failed to execute Test-TargetResource functionality with error message: Cannot validate argument on parameter 'Source'. The argument is null or empty. Provide an argument that is not null or empty, and then try the command again.

It appears to be a bug in #86, Where the code to ensure 'Source' was only included if it was not null or empty was removed.

ferventcoder commented 6 years ago

Source needs to allow for an empty value, so the validation should be removed.

jrdnr commented 6 years ago

If I understand the way this works, the schema.psm1 works, I believe this is what actually builds the MOF, right? So we can allow for Source to have an empty value in either the schema.psm1, OR w/in the cChocoPackageInstall.psm1 file we can go through and clean up everywhere that it expects a value to be specified.

I would lean toward adding the code back into the cChocoPackageInstallerSet.schema.psm1 to ensure MOFs do not define properties with null/empty values.

ferventcoder commented 6 years ago

Chocolatey handles the checks. Source does not need to be passed to it, so there should not be any validation that forces it.

jrdnr commented 6 years ago

Its not actually validation, the errors are generated because the current GA code as is available in the powershell gallery etc. has error handling built into schema.psm1 to remove 'Source' from the property list if its null. With update #86 this was removed causing null properties to be written into the MOF. The new null values in the MOF are now breaking functions in the cChocoPackageInstall.psm1 file. I'd be happy to clean up the cChocoPackageInstall.psm1 so it will drop null/empty properties.

However writing empty properties to the MOF seems like a little messy. Can you specifically address your thought on having empty Properties specified in the MOF?

ferventcoder commented 6 years ago

I don't want messy, I want the source passed unless it is empty. I guess I misunderstood what you were saying here. #86 was incorrect in that it should have determined whether source should be passed and then pass it every time, not what it is doing where it is unconditionally passing source, even when empty. Does that make sense?

jrdnr commented 6 years ago

Yes, I believe I understand what you want. To close this bug [ValidateNotNullOrEmpty] flags for the Source param should be removed from functions in cChocoPackageInstall.psm1 as it could cause errors to be thrown even when there is already error handling in place elseware, and cChocoPackageInstallSet.schema.ps1 should be updated to only write defined parameters into the MOF file.