Type fix for application statements/execution_file

[zhubonan]

This is a great package for building CLI tools!

But, I have received this error when building an application (on Julia 1.6.1):

┌ Info: application options: 
│   options.application =
│    ComoniconOptions.Application(;
│        filter_stdlibs = false,
│        precompile = ComoniconOptions.Precompile(;
│            execution_file = ["test/runtest.jl"],
│        ),
└    )
ERROR: LoadError: TypeError: in keyword argument precompile_statements_file, expected Union{String, Vector{String}}, got a value of type Vector{Union{Nothing, String}}
Stacktrace:
 [1] build_application(m::Module, options::ComoniconOptions.Comonicon)
   @ ComoniconBuilder ~/.julia/packages/ComoniconBuilder/I9tvS/src/build.jl:59
 [2] install(m::Module, options::ComoniconOptions.Comonicon)
   @ ComoniconBuilder ~/.julia/packages/ComoniconBuilder/I9tvS/src/install.jl:66
 [3] #install#1

I think the problem is that the following lines:

ComoniconBuilder.jl/src/build.jl

Lines 56 to 58 in 22e532e

	exec_file = map(x -> pkgdir(m, x), options.application.precompile.execution_file)
	stmt_file = map(x -> pkgdir(m, x), options.application.precompile.statements_file)

results an Array that can contain nothing. The nothing element should be filtered out instead. It works about replacing them with:

The change in this PR should make sure the type is Vector{String} instead of Vector{Union{Nothing, String}} in the same way it is for the system image function.

One more thing, if any of the files listed is missing the error would be raised inside the array comprehension.
Perhaps it is better to explicitly check for the existence of nothing and warn its implication (e.g. one of the files do not exist) to the user?

[codecov]

Codecov Report

Merging #3 (50a1144) into master (22e532e) will increase coverage by 1.01%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master       #3      +/-   ##
==========================================
+ Coverage   39.77%   40.79%   +1.01%     
==========================================
  Files           4        4              
  Lines         269      277       +8     
==========================================
+ Hits          107      113       +6     
- Misses        162      164       +2     

Impacted Files	Coverage Δ
src/build.jl	40.93% <0.00%> (+1.67%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 22e532e...50a1144. Read the comment docs.

[Roger-luo]

thanks! good catch, I think it should instead return a String[] if no file is specified instead of adding type assertions? we don't have any type inference problem here

[zhubonan]

I see, then it is probably better to do array comprehension directly with String[] so the type is always Vector{String}.

[Roger-luo]

thanks! could you also put a test case for this?

[Roger-luo]

just realize github has this annoying CI approve thing, great job! let's just wait for CI to pass!

[zhubonan]

seems there is an issue while building the app against the latest nightly build (segfault...). @Roger-luo Maybe the build test should be limited to the latest release for now?

[Roger-luo]

Yeah let's ignore the nightly for now

[Roger-luo]

LGTM thanks

[zhubonan]

No problem!
Seems that the CI still shows the failed state - I was not able to get my head around GitHub action 😂 I will leave that to you 😏
Maybe worth making a bug fix release?